Skip to content

Reland [mlir][OpenMP] Use correct debug location with link clause. #145889

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 26, 2025

Conversation

abidh
Copy link
Contributor

@abidh abidh commented Jun 26, 2025

#145026 was reverted because it failed a sanitizer test. That issue has been fixed in #145883.

@llvmbot
Copy link
Member

llvmbot commented Jun 26, 2025

@llvm/pr-subscribers-mlir-openmp

@llvm/pr-subscribers-mlir-llvm

Author: Abid Qadeer (abidh)

Changes

This was reverted because it failed a sanitizer test. I am fixing the sanitizer issue in #145883.


Full diff: https://github.com/llvm/llvm-project/pull/145889.diff

2 Files Affected:

  • (modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (+2)
  • (added) mlir/test/Target/LLVMIR/omptarget-debug-map-link-loc.mlir (+39)
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 23140f22555a5..3806db3ceab25 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -4831,6 +4831,7 @@ handleDeclareTargetMapVar(MapInfoData &mapData,
                           llvm::IRBuilderBase &builder, llvm::Function *func) {
   assert(moduleTranslation.getOpenMPBuilder()->Config.isTargetDevice() &&
          "function only supported for target device codegen");
+  llvm::IRBuilderBase::InsertPointGuard guard(builder);
   for (size_t i = 0; i < mapData.MapClause.size(); ++i) {
     // In the case of declare target mapped variables, the basePointer is
     // the reference pointer generated by the convertDeclareTargetAttr
@@ -4865,6 +4866,7 @@ handleDeclareTargetMapVar(MapInfoData &mapData,
       for (llvm::User *user : userVec) {
         if (auto *insn = dyn_cast<llvm::Instruction>(user)) {
           if (insn->getFunction() == func) {
+            builder.SetCurrentDebugLocation(insn->getDebugLoc());
             auto *load = builder.CreateLoad(mapData.BasePointers[i]->getType(),
                                             mapData.BasePointers[i]);
             load->moveBefore(insn->getIterator());
diff --git a/mlir/test/Target/LLVMIR/omptarget-debug-map-link-loc.mlir b/mlir/test/Target/LLVMIR/omptarget-debug-map-link-loc.mlir
new file mode 100644
index 0000000000000..a41766e9c1852
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/omptarget-debug-map-link-loc.mlir
@@ -0,0 +1,39 @@
+// RUN: mlir-translate -mlir-to-llvmir %s
+
+module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<"dlti.alloca_memory_space", 5 : ui32>>, llvm.target_triple = "amdgcn-amd-amdhsa", omp.is_target_device = true} {
+  llvm.mlir.global external @_QMtest_0Esp() {addr_space = 0 : i32, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (link)>} : i32 {
+    %0 = llvm.mlir.constant(1 : i32) : i32
+    llvm.return %0 : i32
+  }
+  llvm.func @_QQmain() {
+    %0 = llvm.mlir.constant(1 : i64) : i64
+    %1 = llvm.alloca %0 x i32 : (i64) -> !llvm.ptr<5> loc(#loc2)
+    %2 = llvm.addrspacecast %1 : !llvm.ptr<5> to !llvm.ptr loc(#loc2)
+    %6 = llvm.mlir.addressof @_QMtest_0Esp : !llvm.ptr loc(#loc1)
+    %7 = omp.map.info var_ptr(%2 : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr loc(#loc3)
+    %8 = omp.map.info var_ptr(%6 : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr loc(#loc3)
+    omp.target map_entries(%7 -> %arg0, %8 -> %arg1 : !llvm.ptr, !llvm.ptr) {
+      %16 = llvm.load %arg1 : !llvm.ptr -> i32 loc(#loc5)
+      llvm.store %16, %arg0 : i32, !llvm.ptr loc(#loc5)
+      omp.terminator loc(#loc5)
+    } loc(#loc5)
+    llvm.return loc(#loc6)
+  } loc(#loc15)
+}
+#di_file = #llvm.di_file<"target7.f90" in "">
+#di_null_type = #llvm.di_null_type
+#di_compile_unit = #llvm.di_compile_unit<id = distinct[0]<>,
+ sourceLanguage = DW_LANG_Fortran95, file = #di_file, producer = "flang",
+ isOptimized = false, emissionKind = LineTablesOnly>
+#di_subroutine_type = #llvm.di_subroutine_type<
+  callingConvention = DW_CC_program, types = #di_null_type>
+#di_subprogram = #llvm.di_subprogram<id = distinct[1]<>,
+  compileUnit = #di_compile_unit, scope = #di_file, name = "main",
+  file = #di_file, subprogramFlags = "Definition|MainSubprogram",
+  type = #di_subroutine_type>
+#loc1 = loc("test.f90":3:18)
+#loc2 = loc("test.f90":7:7)
+#loc3 = loc("test.f90":9:18)
+#loc5 = loc("test.f90":11:7)
+#loc6 = loc("test.f90":12:7)
+#loc15 = loc(fused<#di_subprogram>[#loc2])

@llvmbot
Copy link
Member

llvmbot commented Jun 26, 2025

@llvm/pr-subscribers-mlir

Author: Abid Qadeer (abidh)

Changes

This was reverted because it failed a sanitizer test. I am fixing the sanitizer issue in #145883.


Full diff: https://github.com/llvm/llvm-project/pull/145889.diff

2 Files Affected:

  • (modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (+2)
  • (added) mlir/test/Target/LLVMIR/omptarget-debug-map-link-loc.mlir (+39)
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 23140f22555a5..3806db3ceab25 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -4831,6 +4831,7 @@ handleDeclareTargetMapVar(MapInfoData &mapData,
                           llvm::IRBuilderBase &builder, llvm::Function *func) {
   assert(moduleTranslation.getOpenMPBuilder()->Config.isTargetDevice() &&
          "function only supported for target device codegen");
+  llvm::IRBuilderBase::InsertPointGuard guard(builder);
   for (size_t i = 0; i < mapData.MapClause.size(); ++i) {
     // In the case of declare target mapped variables, the basePointer is
     // the reference pointer generated by the convertDeclareTargetAttr
@@ -4865,6 +4866,7 @@ handleDeclareTargetMapVar(MapInfoData &mapData,
       for (llvm::User *user : userVec) {
         if (auto *insn = dyn_cast<llvm::Instruction>(user)) {
           if (insn->getFunction() == func) {
+            builder.SetCurrentDebugLocation(insn->getDebugLoc());
             auto *load = builder.CreateLoad(mapData.BasePointers[i]->getType(),
                                             mapData.BasePointers[i]);
             load->moveBefore(insn->getIterator());
diff --git a/mlir/test/Target/LLVMIR/omptarget-debug-map-link-loc.mlir b/mlir/test/Target/LLVMIR/omptarget-debug-map-link-loc.mlir
new file mode 100644
index 0000000000000..a41766e9c1852
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/omptarget-debug-map-link-loc.mlir
@@ -0,0 +1,39 @@
+// RUN: mlir-translate -mlir-to-llvmir %s
+
+module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<"dlti.alloca_memory_space", 5 : ui32>>, llvm.target_triple = "amdgcn-amd-amdhsa", omp.is_target_device = true} {
+  llvm.mlir.global external @_QMtest_0Esp() {addr_space = 0 : i32, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (link)>} : i32 {
+    %0 = llvm.mlir.constant(1 : i32) : i32
+    llvm.return %0 : i32
+  }
+  llvm.func @_QQmain() {
+    %0 = llvm.mlir.constant(1 : i64) : i64
+    %1 = llvm.alloca %0 x i32 : (i64) -> !llvm.ptr<5> loc(#loc2)
+    %2 = llvm.addrspacecast %1 : !llvm.ptr<5> to !llvm.ptr loc(#loc2)
+    %6 = llvm.mlir.addressof @_QMtest_0Esp : !llvm.ptr loc(#loc1)
+    %7 = omp.map.info var_ptr(%2 : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr loc(#loc3)
+    %8 = omp.map.info var_ptr(%6 : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr loc(#loc3)
+    omp.target map_entries(%7 -> %arg0, %8 -> %arg1 : !llvm.ptr, !llvm.ptr) {
+      %16 = llvm.load %arg1 : !llvm.ptr -> i32 loc(#loc5)
+      llvm.store %16, %arg0 : i32, !llvm.ptr loc(#loc5)
+      omp.terminator loc(#loc5)
+    } loc(#loc5)
+    llvm.return loc(#loc6)
+  } loc(#loc15)
+}
+#di_file = #llvm.di_file<"target7.f90" in "">
+#di_null_type = #llvm.di_null_type
+#di_compile_unit = #llvm.di_compile_unit<id = distinct[0]<>,
+ sourceLanguage = DW_LANG_Fortran95, file = #di_file, producer = "flang",
+ isOptimized = false, emissionKind = LineTablesOnly>
+#di_subroutine_type = #llvm.di_subroutine_type<
+  callingConvention = DW_CC_program, types = #di_null_type>
+#di_subprogram = #llvm.di_subprogram<id = distinct[1]<>,
+  compileUnit = #di_compile_unit, scope = #di_file, name = "main",
+  file = #di_file, subprogramFlags = "Definition|MainSubprogram",
+  type = #di_subroutine_type>
+#loc1 = loc("test.f90":3:18)
+#loc2 = loc("test.f90":7:7)
+#loc3 = loc("test.f90":9:18)
+#loc5 = loc("test.f90":11:7)
+#loc6 = loc("test.f90":12:7)
+#loc15 = loc(fused<#di_subprogram>[#loc2])

@llvmbot
Copy link
Member

llvmbot commented Jun 26, 2025

@llvm/pr-subscribers-flang-openmp

Author: Abid Qadeer (abidh)

Changes

This was reverted because it failed a sanitizer test. I am fixing the sanitizer issue in #145883.


Full diff: https://github.com/llvm/llvm-project/pull/145889.diff

2 Files Affected:

  • (modified) mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp (+2)
  • (added) mlir/test/Target/LLVMIR/omptarget-debug-map-link-loc.mlir (+39)
diff --git a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 23140f22555a5..3806db3ceab25 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -4831,6 +4831,7 @@ handleDeclareTargetMapVar(MapInfoData &mapData,
                           llvm::IRBuilderBase &builder, llvm::Function *func) {
   assert(moduleTranslation.getOpenMPBuilder()->Config.isTargetDevice() &&
          "function only supported for target device codegen");
+  llvm::IRBuilderBase::InsertPointGuard guard(builder);
   for (size_t i = 0; i < mapData.MapClause.size(); ++i) {
     // In the case of declare target mapped variables, the basePointer is
     // the reference pointer generated by the convertDeclareTargetAttr
@@ -4865,6 +4866,7 @@ handleDeclareTargetMapVar(MapInfoData &mapData,
       for (llvm::User *user : userVec) {
         if (auto *insn = dyn_cast<llvm::Instruction>(user)) {
           if (insn->getFunction() == func) {
+            builder.SetCurrentDebugLocation(insn->getDebugLoc());
             auto *load = builder.CreateLoad(mapData.BasePointers[i]->getType(),
                                             mapData.BasePointers[i]);
             load->moveBefore(insn->getIterator());
diff --git a/mlir/test/Target/LLVMIR/omptarget-debug-map-link-loc.mlir b/mlir/test/Target/LLVMIR/omptarget-debug-map-link-loc.mlir
new file mode 100644
index 0000000000000..a41766e9c1852
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/omptarget-debug-map-link-loc.mlir
@@ -0,0 +1,39 @@
+// RUN: mlir-translate -mlir-to-llvmir %s
+
+module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<"dlti.alloca_memory_space", 5 : ui32>>, llvm.target_triple = "amdgcn-amd-amdhsa", omp.is_target_device = true} {
+  llvm.mlir.global external @_QMtest_0Esp() {addr_space = 0 : i32, omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (link)>} : i32 {
+    %0 = llvm.mlir.constant(1 : i32) : i32
+    llvm.return %0 : i32
+  }
+  llvm.func @_QQmain() {
+    %0 = llvm.mlir.constant(1 : i64) : i64
+    %1 = llvm.alloca %0 x i32 : (i64) -> !llvm.ptr<5> loc(#loc2)
+    %2 = llvm.addrspacecast %1 : !llvm.ptr<5> to !llvm.ptr loc(#loc2)
+    %6 = llvm.mlir.addressof @_QMtest_0Esp : !llvm.ptr loc(#loc1)
+    %7 = omp.map.info var_ptr(%2 : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr loc(#loc3)
+    %8 = omp.map.info var_ptr(%6 : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr loc(#loc3)
+    omp.target map_entries(%7 -> %arg0, %8 -> %arg1 : !llvm.ptr, !llvm.ptr) {
+      %16 = llvm.load %arg1 : !llvm.ptr -> i32 loc(#loc5)
+      llvm.store %16, %arg0 : i32, !llvm.ptr loc(#loc5)
+      omp.terminator loc(#loc5)
+    } loc(#loc5)
+    llvm.return loc(#loc6)
+  } loc(#loc15)
+}
+#di_file = #llvm.di_file<"target7.f90" in "">
+#di_null_type = #llvm.di_null_type
+#di_compile_unit = #llvm.di_compile_unit<id = distinct[0]<>,
+ sourceLanguage = DW_LANG_Fortran95, file = #di_file, producer = "flang",
+ isOptimized = false, emissionKind = LineTablesOnly>
+#di_subroutine_type = #llvm.di_subroutine_type<
+  callingConvention = DW_CC_program, types = #di_null_type>
+#di_subprogram = #llvm.di_subprogram<id = distinct[1]<>,
+  compileUnit = #di_compile_unit, scope = #di_file, name = "main",
+  file = #di_file, subprogramFlags = "Definition|MainSubprogram",
+  type = #di_subroutine_type>
+#loc1 = loc("test.f90":3:18)
+#loc2 = loc("test.f90":7:7)
+#loc3 = loc("test.f90":9:18)
+#loc5 = loc("test.f90":11:7)
+#loc6 = loc("test.f90":12:7)
+#loc15 = loc(fused<#di_subprogram>[#loc2])

Please see the following program.

module test_0
    INTEGER :: sp = 1
!$omp declare target link(sp)
end module test_0

program main
use test_0
integer :: new_len

!$omp target map(tofrom:new_len) map(tofrom:sp)
    new_len = sp
!$omp end target

  print *, new_len
  print *, sp
end program

flang -g -O0 -fopenmp --offload-arch=gfx1100

will fail the compilation with the following error:

dbg attachment points at wrong subprogram for function

The reason is that with the link clause on !$omp declare target, an
extra load instruction is inserted. But the debug location was not
updated before insertion which cuased it to have an invalid debug
location.
@abidh abidh merged commit 232c292 into llvm:main Jun 26, 2025
7 checks passed
anthonyhatran pushed a commit to anthonyhatran/llvm-project that referenced this pull request Jun 26, 2025
…lvm#145889)

llvm#145026 was reverted because it
failed a sanitizer test. That issue has been fixed in
llvm#145883.
rlavaee pushed a commit to rlavaee/llvm-project that referenced this pull request Jul 1, 2025
…lvm#145889)

llvm#145026 was reverted because it
failed a sanitizer test. That issue has been fixed in
llvm#145883.
@abidh abidh deleted the link_loc2 branch July 2, 2025 11:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants