[flang-commits] [flang] [llvm] [mlir] [OpenMPIRBuilder] Don't drop debug info for target region. (PR #80692)
Abid Qadeer via flang-commits
flang-commits at lists.llvm.org
Tue Sep 3 03:46:42 PDT 2024
https://github.com/abidh updated https://github.com/llvm/llvm-project/pull/80692
>From 31472fe341190a150488c423510f370210b910dd Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Fri, 2 Feb 2024 12:16:52 -0600
Subject: [PATCH 01/10] [OpenMPIRBuilder] Don't drop debug info for target
region.
When an outlined function is generated for omp target region,
a corresponding DISubprogram was not being generated. This resulted in
all the debug information for the target region being dropped.
This commit adds DISubprogram for the outlined function if there is one
available for the parent function. It also updates the current debug
location so that the right scope is used for the entries in the outlined
function.
With this change in place, I can set source line breakpoint in target
region and run to them in debugger.
---
flang/test/Lower/OpenMP/target-debug.f90 | 18 ++++++++++
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 36 +++++++++++++++++--
.../Frontend/OpenMPIRBuilderTest.cpp | 4 +++
3 files changed, 56 insertions(+), 2 deletions(-)
create mode 100644 flang/test/Lower/OpenMP/target-debug.f90
diff --git a/flang/test/Lower/OpenMP/target-debug.f90 b/flang/test/Lower/OpenMP/target-debug.f90
new file mode 100644
index 00000000000000..c9843eba391d7c
--- /dev/null
+++ b/flang/test/Lower/OpenMP/target-debug.f90
@@ -0,0 +1,18 @@
+!RUN: %flang_fc1 -triple amdgcn-amd-amdhsa %s -debug-info-kind=line-tables-only -fopenmp -fopenmp-is-target-device -emit-llvm -o - | FileCheck %s
+program test
+implicit none
+
+ integer(kind = 4) :: a, b, c, d
+
+ !$omp target map(tofrom: a, b, c, d)
+ a = a + 1
+ ! CHECK: !DILocation(line: [[@LINE-1]]
+ b = a + 2
+ ! CHECK: !DILocation(line: [[@LINE-1]]
+ c = a + 3
+ ! CHECK: !DILocation(line: [[@LINE-1]]
+ d = a + 4
+ ! CHECK: !DILocation(line: [[@LINE-1]]
+ !$omp end target
+
+end program test
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 532313a31fc132..03b013ffdba10b 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -32,6 +32,7 @@
#include "llvm/IR/CallingConv.h"
#include "llvm/IR/Constant.h"
#include "llvm/IR/Constants.h"
+#include "llvm/IR/DIBuilder.h"
#include "llvm/IR/DebugInfoMetadata.h"
#include "llvm/IR/DerivedTypes.h"
#include "llvm/IR/Function.h"
@@ -6584,10 +6585,41 @@ static Function *createOutlinedFunction(
ParameterTypes.push_back(Arg->getType());
}
+ auto BB = Builder.GetInsertBlock();
+ auto M = BB->getModule();
auto FuncType = FunctionType::get(Builder.getVoidTy(), ParameterTypes,
/*isVarArg*/ false);
- auto Func = Function::Create(FuncType, GlobalValue::InternalLinkage, FuncName,
- Builder.GetInsertBlock()->getModule());
+ auto Func =
+ Function::Create(FuncType, GlobalValue::InternalLinkage, FuncName, M);
+
+ // If there's a DISubprogram associated with current function, then
+ // generate one for the outlined function.
+ if (Function *parentFunc = BB->getParent()) {
+ if (DISubprogram *SP = parentFunc->getSubprogram()) {
+ DICompileUnit *CU = SP->getUnit();
+ DIBuilder DB(*M, true, CU);
+ DebugLoc DL = Builder.getCurrentDebugLocation();
+ // TODO: We are using nullopt for arguments at the moment. This will need
+ // to be updated when debug data is being generated for variables.
+ DISubroutineType *Ty =
+ DB.createSubroutineType(DB.getOrCreateTypeArray(std::nullopt));
+ DISubprogram::DISPFlags SPFlags = DISubprogram::SPFlagDefinition |
+ DISubprogram::SPFlagOptimized |
+ DISubprogram::SPFlagLocalToUnit;
+
+ DISubprogram *OutlinedSP = DB.createFunction(
+ CU, FuncName, FuncName, SP->getFile(), DL.getLine(), Ty, DL.getLine(),
+ DINode::DIFlags::FlagArtificial, SPFlags);
+
+ // Attach subprogram to the function.
+ Func->setSubprogram(OutlinedSP);
+ // Update the CurrentDebugLocation in the builder so that right scope
+ // is used for things inside outlined function.
+ Builder.SetCurrentDebugLocation(
+ DILocation::get(Func->getContext(), DL.getLine(), DL.getCol(),
+ OutlinedSP, DL.getInlinedAt()));
+ }
+ }
// Save insert point.
auto OldInsertPoint = Builder.saveIP();
diff --git a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
index 6207792f9f0d08..c92a3ff2e7ba6c 100644
--- a/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
@@ -6006,6 +6006,10 @@ TEST_F(OpenMPIRBuilderTest, TargetRegion) {
BasicBlock *FallbackBlock = Branch->getSuccessor(0);
Iter = FallbackBlock->rbegin();
CallInst *FCall = dyn_cast<CallInst>(&*(++Iter));
+ // 'F' has a dummy DISubprogram which causes OutlinedFunc to also
+ // have a DISubprogram. In this case, the call to OutlinedFunc needs
+ // to have a debug loc, otherwise verifier will complain.
+ FCall->setDebugLoc(DL);
EXPECT_NE(FCall, nullptr);
// Check that the correct aguments are passed in
>From 9978dc38e54fa49bcc505099d9b211137e5c4fdd Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Wed, 7 Feb 2024 10:07:11 -0600
Subject: [PATCH 02/10] Move the test into MLIR.
The original f90 ->llvmir test has been removed.
I have added an MLIR -> llvmir test as per review comments.
---
flang/test/Lower/OpenMP/target-debug.f90 | 18 -----
mlir/test/Target/LLVMIR/omptarget-debug.mlir | 72 ++++++++++++++++++++
2 files changed, 72 insertions(+), 18 deletions(-)
delete mode 100644 flang/test/Lower/OpenMP/target-debug.f90
create mode 100644 mlir/test/Target/LLVMIR/omptarget-debug.mlir
diff --git a/flang/test/Lower/OpenMP/target-debug.f90 b/flang/test/Lower/OpenMP/target-debug.f90
deleted file mode 100644
index c9843eba391d7c..00000000000000
--- a/flang/test/Lower/OpenMP/target-debug.f90
+++ /dev/null
@@ -1,18 +0,0 @@
-!RUN: %flang_fc1 -triple amdgcn-amd-amdhsa %s -debug-info-kind=line-tables-only -fopenmp -fopenmp-is-target-device -emit-llvm -o - | FileCheck %s
-program test
-implicit none
-
- integer(kind = 4) :: a, b, c, d
-
- !$omp target map(tofrom: a, b, c, d)
- a = a + 1
- ! CHECK: !DILocation(line: [[@LINE-1]]
- b = a + 2
- ! CHECK: !DILocation(line: [[@LINE-1]]
- c = a + 3
- ! CHECK: !DILocation(line: [[@LINE-1]]
- d = a + 4
- ! CHECK: !DILocation(line: [[@LINE-1]]
- !$omp end target
-
-end program test
diff --git a/mlir/test/Target/LLVMIR/omptarget-debug.mlir b/mlir/test/Target/LLVMIR/omptarget-debug.mlir
new file mode 100644
index 00000000000000..4d061eb25082be
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/omptarget-debug.mlir
@@ -0,0 +1,72 @@
+// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
+
+module attributes {omp.is_target_device = true} {
+ llvm.func @_QQmain() {
+ %0 = llvm.mlir.constant(1 : i64) : i64
+ %1 = llvm.alloca %0 x i32 : (i64) -> !llvm.ptr<5>
+ %2 = llvm.addrspacecast %1 : !llvm.ptr<5> to !llvm.ptr
+ %3 = llvm.mlir.constant(1 : i64) : i64
+ %4 = llvm.alloca %3 x i32 : (i64) -> !llvm.ptr<5>
+ %5 = llvm.addrspacecast %4 : !llvm.ptr<5> to !llvm.ptr
+ %6 = llvm.mlir.constant(1 : i64) : i64
+ %7 = llvm.alloca %6 x i32 : (i64) -> !llvm.ptr<5>
+ %8 = llvm.addrspacecast %7 : !llvm.ptr<5> to !llvm.ptr
+ %9 = omp.map_info var_ptr(%2 : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = ""}
+ %10 = omp.map_info var_ptr(%5 : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = ""}
+ omp.target map_entries(%9 -> %arg0, %10 -> %arg1 : !llvm.ptr, !llvm.ptr) {
+ ^bb0(%arg0: !llvm.ptr, %arg1: !llvm.ptr):
+ %12 = llvm.mlir.constant(2 : i32) : i32
+ %13 = llvm.mlir.constant(1 : i32) : i32
+ %14 = llvm.load %arg0 : !llvm.ptr -> i32 loc(#loc2)
+ %15 = llvm.add %14, %13 : i32 loc(#loc2)
+ llvm.store %15, %arg0 : i32, !llvm.ptr loc(#loc2)
+ %16 = llvm.load %arg0 : !llvm.ptr -> i32 loc(#loc3)
+ %17 = llvm.add %16, %12 : i32 loc(#loc3)
+ llvm.store %17, %arg1 : i32, !llvm.ptr loc(#loc3)
+ omp.terminator
+ }
+ %11 = omp.map_info var_ptr(%8 : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = ""}
+ omp.target map_entries(%11 -> %arg0 : !llvm.ptr) {
+ ^bb0(%arg0: !llvm.ptr):
+ %12 = llvm.mlir.constant(1 : i32) : i32
+ omp.parallel {
+ %13 = llvm.load %arg0 : !llvm.ptr -> i32 loc(#loc4)
+ %14 = llvm.add %13, %12 : i32 loc(#loc4)
+ llvm.store %14, %arg0 : i32, !llvm.ptr loc(#loc4)
+ omp.terminator
+ }
+ omp.terminator
+ }
+ llvm.return
+ } loc(#loc5)
+} loc(#loc)
+#di_basic_type = #llvm.di_basic_type<tag = DW_TAG_base_type, name = "void", encoding = DW_ATE_address>
+#di_file = #llvm.di_file<"target.f90" in "">
+#loc = loc("target.f90":0:0)
+#loc1 = loc("target.f90":1:1)
+#loc2 = loc("target.f90":9:3)
+#loc3 = loc("target.f90":10:3)
+#loc4 = loc("target.f90":14:3)
+#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_normal, types = #di_basic_type, #di_basic_type>
+#di_subprogram = #llvm.di_subprogram<id = distinct[1]<>, compileUnit = #di_compile_unit, scope = #di_file, name = "_QQmain", linkageName = "_QQmain", file = #di_file, line = 1, scopeLine = 1, subprogramFlags = "Definition|Optimized", type = #di_subroutine_type>
+#loc5 = loc(fused<#di_subprogram>[#loc1])
+
+// 8: !$omp target map(tofrom: a, b)
+// 9: a = a + 1
+//10: b = a + 2
+//11: !$omp end target
+
+//13: !$omp target parallel map(tofrom: a, b)
+//14: c = c + 1
+//15: !$omp end target parallel
+
+// CHECK-DAG: [[FILE:.*]] = !DIFile(filename: "target.f90", directory: "")
+// CHECK-DAG: [[CU:.*]] = distinct !DICompileUnit(language: DW_LANG_Fortran95, file: [[FILE]], {{.*}})
+
+// CHECK: [[SP1:.*]] = distinct !DISubprogram(name: "__omp_offloading_{{.*}}", {{.*}}, unit: [[CU]])
+// CHECK-DAG: !DILocation(line: 9, column: 3, scope: [[SP1]])
+// CHECK-DAG: !DILocation(line: 10, column: 3, scope: [[SP1]])
+
+// CHECK: [[SP2:.*]] = distinct !DISubprogram(name: "__omp_offloading_{{.*}}omp_par", {{.*}}, unit: [[CU]])
+// CHECK: !DILocation(line: 14, column: 3, scope: [[SP2]])
>From da3ac09fdefe6475a47268a9b24e9676a5151bd5 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Wed, 7 Feb 2024 10:08:08 -0600
Subject: [PATCH 03/10] Correct the case on variable name.
---
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 03b013ffdba10b..cb8438ff1c199b 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -6594,8 +6594,8 @@ static Function *createOutlinedFunction(
// If there's a DISubprogram associated with current function, then
// generate one for the outlined function.
- if (Function *parentFunc = BB->getParent()) {
- if (DISubprogram *SP = parentFunc->getSubprogram()) {
+ if (Function *ParentFunc = BB->getParent()) {
+ if (DISubprogram *SP = ParentFunc->getSubprogram()) {
DICompileUnit *CU = SP->getUnit();
DIBuilder DB(*M, true, CU);
DebugLoc DL = Builder.getCurrentDebugLocation();
>From d3585a865f6add9c6d97e8852214ff5728cc4702 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Mon, 8 Jul 2024 16:22:20 +0100
Subject: [PATCH 04/10] Minimize testcase as per review comments.
---
mlir/test/Target/LLVMIR/omptarget-debug.mlir | 57 ++++----------------
1 file changed, 10 insertions(+), 47 deletions(-)
diff --git a/mlir/test/Target/LLVMIR/omptarget-debug.mlir b/mlir/test/Target/LLVMIR/omptarget-debug.mlir
index 4d061eb25082be..824df55704a6c2 100644
--- a/mlir/test/Target/LLVMIR/omptarget-debug.mlir
+++ b/mlir/test/Target/LLVMIR/omptarget-debug.mlir
@@ -5,68 +5,31 @@ module attributes {omp.is_target_device = true} {
%0 = llvm.mlir.constant(1 : i64) : i64
%1 = llvm.alloca %0 x i32 : (i64) -> !llvm.ptr<5>
%2 = llvm.addrspacecast %1 : !llvm.ptr<5> to !llvm.ptr
- %3 = llvm.mlir.constant(1 : i64) : i64
- %4 = llvm.alloca %3 x i32 : (i64) -> !llvm.ptr<5>
- %5 = llvm.addrspacecast %4 : !llvm.ptr<5> to !llvm.ptr
- %6 = llvm.mlir.constant(1 : i64) : i64
- %7 = llvm.alloca %6 x i32 : (i64) -> !llvm.ptr<5>
- %8 = llvm.addrspacecast %7 : !llvm.ptr<5> to !llvm.ptr
- %9 = omp.map_info var_ptr(%2 : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = ""}
- %10 = omp.map_info var_ptr(%5 : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = ""}
- omp.target map_entries(%9 -> %arg0, %10 -> %arg1 : !llvm.ptr, !llvm.ptr) {
+ %9 = omp.map.info var_ptr(%2 : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = ""}
+ omp.target map_entries(%9 -> %arg0 : !llvm.ptr) {
^bb0(%arg0: !llvm.ptr, %arg1: !llvm.ptr):
- %12 = llvm.mlir.constant(2 : i32) : i32
%13 = llvm.mlir.constant(1 : i32) : i32
%14 = llvm.load %arg0 : !llvm.ptr -> i32 loc(#loc2)
%15 = llvm.add %14, %13 : i32 loc(#loc2)
llvm.store %15, %arg0 : i32, !llvm.ptr loc(#loc2)
- %16 = llvm.load %arg0 : !llvm.ptr -> i32 loc(#loc3)
- %17 = llvm.add %16, %12 : i32 loc(#loc3)
- llvm.store %17, %arg1 : i32, !llvm.ptr loc(#loc3)
- omp.terminator
- }
- %11 = omp.map_info var_ptr(%8 : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = ""}
- omp.target map_entries(%11 -> %arg0 : !llvm.ptr) {
- ^bb0(%arg0: !llvm.ptr):
- %12 = llvm.mlir.constant(1 : i32) : i32
- omp.parallel {
- %13 = llvm.load %arg0 : !llvm.ptr -> i32 loc(#loc4)
- %14 = llvm.add %13, %12 : i32 loc(#loc4)
- llvm.store %14, %arg0 : i32, !llvm.ptr loc(#loc4)
- omp.terminator
- }
omp.terminator
}
llvm.return
} loc(#loc5)
-} loc(#loc)
+}
#di_basic_type = #llvm.di_basic_type<tag = DW_TAG_base_type, name = "void", encoding = DW_ATE_address>
#di_file = #llvm.di_file<"target.f90" in "">
-#loc = loc("target.f90":0:0)
#loc1 = loc("target.f90":1:1)
-#loc2 = loc("target.f90":9:3)
-#loc3 = loc("target.f90":10:3)
-#loc4 = loc("target.f90":14:3)
+#loc2 = loc("target.f90":46:3)
+
#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_normal, types = #di_basic_type, #di_basic_type>
#di_subprogram = #llvm.di_subprogram<id = distinct[1]<>, compileUnit = #di_compile_unit, scope = #di_file, name = "_QQmain", linkageName = "_QQmain", file = #di_file, line = 1, scopeLine = 1, subprogramFlags = "Definition|Optimized", type = #di_subroutine_type>
#loc5 = loc(fused<#di_subprogram>[#loc1])
-// 8: !$omp target map(tofrom: a, b)
-// 9: a = a + 1
-//10: b = a + 2
-//11: !$omp end target
-
-//13: !$omp target parallel map(tofrom: a, b)
-//14: c = c + 1
-//15: !$omp end target parallel
-
-// CHECK-DAG: [[FILE:.*]] = !DIFile(filename: "target.f90", directory: "")
-// CHECK-DAG: [[CU:.*]] = distinct !DICompileUnit(language: DW_LANG_Fortran95, file: [[FILE]], {{.*}})
-
-// CHECK: [[SP1:.*]] = distinct !DISubprogram(name: "__omp_offloading_{{.*}}", {{.*}}, unit: [[CU]])
-// CHECK-DAG: !DILocation(line: 9, column: 3, scope: [[SP1]])
-// CHECK-DAG: !DILocation(line: 10, column: 3, scope: [[SP1]])
+// 45: !$omp target map(tofrom: a)
+// 46: a = a + 1
+// 47: !$omp end target
-// CHECK: [[SP2:.*]] = distinct !DISubprogram(name: "__omp_offloading_{{.*}}omp_par", {{.*}}, unit: [[CU]])
-// CHECK: !DILocation(line: 14, column: 3, scope: [[SP2]])
+// CHECK: [[SP:.*]] = distinct !DISubprogram(name: "__omp_offloading_{{.*}}"{{.*}})
+// CHECK-DAG: !DILocation(line: 46, column: 3, scope: [[SP]])
>From f1b4e5496f669cde44a1aa1100f3f98dfbfce26c Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Tue, 9 Jul 2024 13:53:19 +0100
Subject: [PATCH 05/10] Don't generate debug info for variable in target
region.
These variables currently fail the llvm verifier tests as the scope
inside the variable is still point to the parent function of the
target region.
---
.../lib/Optimizer/Transforms/AddDebugInfo.cpp | 6 +++-
.../Integration/debug-target-region-vars.f90 | 28 +++++++++++++++++++
2 files changed, 33 insertions(+), 1 deletion(-)
create mode 100644 flang/test/Integration/debug-target-region-vars.f90
diff --git a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
index 46fc40b714aac7..576e65ba6ecc50 100644
--- a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
+++ b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
@@ -309,7 +309,11 @@ void AddDebugInfoPass::handleFuncOp(mlir::func::FuncOp funcOp,
return;
funcOp.walk([&](fir::cg::XDeclareOp declOp) {
- handleDeclareOp(declOp, fileAttr, spAttr, typeGen, symbolTable);
+ // FIXME: We currently dont handle variables that are not in the entry
+ // blocks of the fuctions. These may be variable or arguments used in the
+ // OpenMP target regions.
+ if (&funcOp.front() == declOp->getBlock())
+ handleDeclareOp(declOp, fileAttr, spAttr, typeGen, symbolTable);
});
}
diff --git a/flang/test/Integration/debug-target-region-vars.f90 b/flang/test/Integration/debug-target-region-vars.f90
new file mode 100644
index 00000000000000..a57afb301d9b7b
--- /dev/null
+++ b/flang/test/Integration/debug-target-region-vars.f90
@@ -0,0 +1,28 @@
+! RUN: %flang_fc1 -fopenmp -emit-llvm -debug-info-kind=standalone %s -o - | FileCheck %s
+
+! Test that variables inside OpenMP target region don't cause build failure.
+subroutine test1
+ implicit none
+ real, allocatable :: xyz(:)
+ integer :: i
+
+ !$omp target simd map(from:xyz)
+ do i = 1, size(xyz)
+ xyz(i) = 5.0 * xyz(i)
+ end do
+end subroutine
+
+subroutine test2 (xyz)
+ integer :: i
+ integer :: xyz(:)
+
+ !$omp target map(from:xyz)
+ !$omp do private(xyz)
+ do i = 1, 10
+ xyz(i) = i
+ end do
+ !$omp end target
+end subroutine
+
+!CHECK: DISubprogram(name: "test1"{{.*}})
+!CHECK: DISubprogram(name: "test2"{{.*}})
>From 4d1507af8825c4fc417c8a4f57b326abf28d1cf2 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Fri, 23 Aug 2024 09:49:28 +0100
Subject: [PATCH 06/10] Improve test case.
---
mlir/test/Target/LLVMIR/omptarget-debug.mlir | 38 +++++++++-----------
1 file changed, 16 insertions(+), 22 deletions(-)
diff --git a/mlir/test/Target/LLVMIR/omptarget-debug.mlir b/mlir/test/Target/LLVMIR/omptarget-debug.mlir
index 824df55704a6c2..76a853249caca3 100644
--- a/mlir/test/Target/LLVMIR/omptarget-debug.mlir
+++ b/mlir/test/Target/LLVMIR/omptarget-debug.mlir
@@ -2,34 +2,28 @@
module attributes {omp.is_target_device = true} {
llvm.func @_QQmain() {
- %0 = llvm.mlir.constant(1 : i64) : i64
- %1 = llvm.alloca %0 x i32 : (i64) -> !llvm.ptr<5>
- %2 = llvm.addrspacecast %1 : !llvm.ptr<5> to !llvm.ptr
- %9 = omp.map.info var_ptr(%2 : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = ""}
+ %0 = llvm.mlir.constant(1 : i32) : i32
+ %1 = llvm.alloca %0 x i32 : (i32) -> !llvm.ptr
+ %9 = omp.map.info var_ptr(%1 : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = ""}
omp.target map_entries(%9 -> %arg0 : !llvm.ptr) {
- ^bb0(%arg0: !llvm.ptr, %arg1: !llvm.ptr):
+ ^bb0(%arg0: !llvm.ptr):
%13 = llvm.mlir.constant(1 : i32) : i32
- %14 = llvm.load %arg0 : !llvm.ptr -> i32 loc(#loc2)
- %15 = llvm.add %14, %13 : i32 loc(#loc2)
- llvm.store %15, %arg0 : i32, !llvm.ptr loc(#loc2)
+ llvm.store %13, %arg0 : i32, !llvm.ptr loc(#loc2)
omp.terminator
}
llvm.return
- } loc(#loc5)
+ } loc(#loc3)
}
-#di_basic_type = #llvm.di_basic_type<tag = DW_TAG_base_type, name = "void", encoding = DW_ATE_address>
-#di_file = #llvm.di_file<"target.f90" in "">
+#file = #llvm.di_file<"target.f90" in "">
+#cu = #llvm.di_compile_unit<id = distinct[0]<>,
+ sourceLanguage = DW_LANG_Fortran95, file = #file, isOptimized = false,
+ emissionKind = LineTablesOnly>
+#sp_ty = #llvm.di_subroutine_type<callingConvention = DW_CC_normal>
+#sp = #llvm.di_subprogram<id = distinct[1]<>, compileUnit = #cu, scope = #file,
+ name = "_QQmain", file = #file, subprogramFlags = "Definition", type = #sp_ty>
#loc1 = loc("target.f90":1:1)
#loc2 = loc("target.f90":46:3)
+#loc3 = loc(fused<#sp>[#loc1])
-#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_normal, types = #di_basic_type, #di_basic_type>
-#di_subprogram = #llvm.di_subprogram<id = distinct[1]<>, compileUnit = #di_compile_unit, scope = #di_file, name = "_QQmain", linkageName = "_QQmain", file = #di_file, line = 1, scopeLine = 1, subprogramFlags = "Definition|Optimized", type = #di_subroutine_type>
-#loc5 = loc(fused<#di_subprogram>[#loc1])
-
-// 45: !$omp target map(tofrom: a)
-// 46: a = a + 1
-// 47: !$omp end target
-
-// CHECK: [[SP:.*]] = distinct !DISubprogram(name: "__omp_offloading_{{.*}}"{{.*}})
-// CHECK-DAG: !DILocation(line: 46, column: 3, scope: [[SP]])
+// CHECK-DAG: ![[SP:.*]] = {{.*}}!DISubprogram(name: "__omp_offloading_{{.*}}"{{.*}})
+// CHECK-DAG: !DILocation(line: 46, column: 3, scope: ![[SP]])
>From 9370c298d15adc100e8d33409113d431430faeb4 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Fri, 23 Aug 2024 17:26:22 +0100
Subject: [PATCH 07/10] Use InsertPointGuard instead of manual save/restore.
It has the extra benefit of saving/restoring the debug location which otherwise could be lost.
---
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 8 ++------
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index cb8438ff1c199b..88639bd889b5ed 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -6592,6 +6592,8 @@ static Function *createOutlinedFunction(
auto Func =
Function::Create(FuncType, GlobalValue::InternalLinkage, FuncName, M);
+ // Save insert point.
+ IRBuilder<>::InsertPointGuard IPG(Builder);
// If there's a DISubprogram associated with current function, then
// generate one for the outlined function.
if (Function *ParentFunc = BB->getParent()) {
@@ -6621,9 +6623,6 @@ static Function *createOutlinedFunction(
}
}
- // Save insert point.
- auto OldInsertPoint = Builder.saveIP();
-
// Generate the region into the function.
BasicBlock *EntryBB = BasicBlock::Create(Builder.getContext(), "entry", Func);
Builder.SetInsertPoint(EntryBB);
@@ -6729,9 +6728,6 @@ static Function *createOutlinedFunction(
for (auto Deferred : DeferredReplacement)
ReplaceValue(std::get<0>(Deferred), std::get<1>(Deferred), Func);
- // Restore insert point.
- Builder.restoreIP(OldInsertPoint);
-
return Func;
}
>From a4b6b77b0def697d1afaecc3abef36a38783a2ef Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Fri, 23 Aug 2024 18:16:57 +0100
Subject: [PATCH 08/10] Check DebugLoc is valid before using it.
---
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 40 ++++++++++++-----------
1 file changed, 21 insertions(+), 19 deletions(-)
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 88639bd889b5ed..c33bfad6568019 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -6601,25 +6601,27 @@ static Function *createOutlinedFunction(
DICompileUnit *CU = SP->getUnit();
DIBuilder DB(*M, true, CU);
DebugLoc DL = Builder.getCurrentDebugLocation();
- // TODO: We are using nullopt for arguments at the moment. This will need
- // to be updated when debug data is being generated for variables.
- DISubroutineType *Ty =
- DB.createSubroutineType(DB.getOrCreateTypeArray(std::nullopt));
- DISubprogram::DISPFlags SPFlags = DISubprogram::SPFlagDefinition |
- DISubprogram::SPFlagOptimized |
- DISubprogram::SPFlagLocalToUnit;
-
- DISubprogram *OutlinedSP = DB.createFunction(
- CU, FuncName, FuncName, SP->getFile(), DL.getLine(), Ty, DL.getLine(),
- DINode::DIFlags::FlagArtificial, SPFlags);
-
- // Attach subprogram to the function.
- Func->setSubprogram(OutlinedSP);
- // Update the CurrentDebugLocation in the builder so that right scope
- // is used for things inside outlined function.
- Builder.SetCurrentDebugLocation(
- DILocation::get(Func->getContext(), DL.getLine(), DL.getCol(),
- OutlinedSP, DL.getInlinedAt()));
+ if (DL) {
+ // TODO: We are using nullopt for arguments at the moment. This will
+ // need to be updated when debug data is being generated for variables.
+ DISubroutineType *Ty =
+ DB.createSubroutineType(DB.getOrCreateTypeArray(std::nullopt));
+ DISubprogram::DISPFlags SPFlags = DISubprogram::SPFlagDefinition |
+ DISubprogram::SPFlagOptimized |
+ DISubprogram::SPFlagLocalToUnit;
+
+ DISubprogram *OutlinedSP = DB.createFunction(
+ CU, FuncName, FuncName, SP->getFile(), DL.getLine(), Ty,
+ DL.getLine(), DINode::DIFlags::FlagArtificial, SPFlags);
+
+ // Attach subprogram to the function.
+ Func->setSubprogram(OutlinedSP);
+ // Update the CurrentDebugLocation in the builder so that right scope
+ // is used for things inside outlined function.
+ Builder.SetCurrentDebugLocation(
+ DILocation::get(Func->getContext(), DL.getLine(), DL.getCol(),
+ OutlinedSP, DL.getInlinedAt()));
+ }
}
}
>From beda51c64670d6c95e1c2325bf161a7e0de5ce29 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Wed, 28 Aug 2024 17:06:07 +0100
Subject: [PATCH 09/10] Add testcase with is_target_device = false
---
mlir/test/Target/LLVMIR/omptarget-debug2.mlir | 31 +++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100644 mlir/test/Target/LLVMIR/omptarget-debug2.mlir
diff --git a/mlir/test/Target/LLVMIR/omptarget-debug2.mlir b/mlir/test/Target/LLVMIR/omptarget-debug2.mlir
new file mode 100644
index 00000000000000..ee19cc31e5c6b4
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/omptarget-debug2.mlir
@@ -0,0 +1,31 @@
+// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
+
+// Same test as omptarget-debug.mlir but with is_target_device = false.
+// Somehow test with omp.target don't work with -split-input-file.
+module attributes {omp.is_target_device = false} {
+ llvm.func @_QQmain() {
+ %0 = llvm.mlir.constant(1 : i32) : i32
+ %1 = llvm.alloca %0 x i32 : (i32) -> !llvm.ptr
+ %9 = omp.map.info var_ptr(%1 : !llvm.ptr, i32) map_clauses(tofrom) capture(ByRef) -> !llvm.ptr {name = ""}
+ omp.target map_entries(%9 -> %arg0 : !llvm.ptr) {
+ ^bb0(%arg0: !llvm.ptr):
+ %13 = llvm.mlir.constant(1 : i32) : i32
+ llvm.store %13, %arg0 : i32, !llvm.ptr loc(#loc2)
+ omp.terminator
+ }
+ llvm.return
+ } loc(#loc3)
+}
+#file = #llvm.di_file<"target.f90" in "">
+#cu = #llvm.di_compile_unit<id = distinct[0]<>,
+ sourceLanguage = DW_LANG_Fortran95, file = #file, isOptimized = false,
+ emissionKind = LineTablesOnly>
+#sp_ty = #llvm.di_subroutine_type<callingConvention = DW_CC_normal>
+#sp = #llvm.di_subprogram<id = distinct[1]<>, compileUnit = #cu, scope = #file,
+ name = "_QQmain", file = #file, subprogramFlags = "Definition", type = #sp_ty>
+#loc1 = loc("target.f90":1:1)
+#loc2 = loc("target.f90":46:3)
+#loc3 = loc(fused<#sp>[#loc1])
+
+// CHECK-DAG: ![[SP:.*]] = {{.*}}!DISubprogram(name: "__omp_offloading_{{.*}}"{{.*}})
+// CHECK-DAG: !DILocation(line: 46, column: 3, scope: ![[SP]])
>From bab81853aa5fd6c2ad6c0dcd93dea1146ea1c03e Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Tue, 3 Sep 2024 11:45:54 +0100
Subject: [PATCH 10/10] Avoid using stale debug location.
---
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index c33bfad6568019..027b927fa64246 100644
--- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
+++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
@@ -4329,6 +4329,7 @@ workshareLoopTargetCallback(OpenMPIRBuilder *OMPIRBuilder,
// That's why make an unconditional branch from loop preheader to loop
// exit block
Builder.restoreIP({Preheader, Preheader->end()});
+ Builder.SetCurrentDebugLocation(Preheader->getTerminator()->getDebugLoc());
Preheader->getTerminator()->eraseFromParent();
Builder.CreateBr(CLI->getExit());
More information about the flang-commits
mailing list