[Mlir-commits] [mlir] 9e08db7 - [OpenMPIRBuilder] Don't drop debug info for target region. (#80692)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Wed Sep 4 02:16:20 PDT 2024
Author: Abid Qadeer
Date: 2024-09-04T10:16:14+01:00
New Revision: 9e08db796b7fc1aa21ec0a0c16a0213229e02010
URL: https://github.com/llvm/llvm-project/commit/9e08db796b7fc1aa21ec0a0c16a0213229e02010
DIFF: https://github.com/llvm/llvm-project/commit/9e08db796b7fc1aa21ec0a0c16a0213229e02010.diff
LOG: [OpenMPIRBuilder] Don't drop debug info for target region. (#80692)
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.
There are places in the OpenMPIRBuilder which changes insertion point but
don't update the debug location accordingly. They cause issue when debug info
is enabled. I have fixed a few that I observed to cause issue. But there may be
more and a systematic cleanup may be required.
With this change in place, I can set source line breakpoint in target
region and run to them in debugger.
Added:
flang/test/Integration/debug-target-region-vars.f90
mlir/test/Target/LLVMIR/omptarget-debug.mlir
mlir/test/Target/LLVMIR/omptarget-debug2.mlir
Modified:
flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
llvm/unittests/Frontend/OpenMPIRBuilderTest.cpp
Removed:
################################################################################
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"{{.*}})
diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
index 532313a31fc132..027b927fa64246 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"
@@ -4328,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());
@@ -6584,13 +6586,45 @@ 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);
// Save insert point.
- auto OldInsertPoint = Builder.saveIP();
+ 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()) {
+ if (DISubprogram *SP = ParentFunc->getSubprogram()) {
+ DICompileUnit *CU = SP->getUnit();
+ DIBuilder DB(*M, true, CU);
+ DebugLoc DL = Builder.getCurrentDebugLocation();
+ 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()));
+ }
+ }
+ }
// Generate the region into the function.
BasicBlock *EntryBB = BasicBlock::Create(Builder.getContext(), "entry", Func);
@@ -6697,9 +6731,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;
}
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
diff --git a/mlir/test/Target/LLVMIR/omptarget-debug.mlir b/mlir/test/Target/LLVMIR/omptarget-debug.mlir
new file mode 100644
index 00000000000000..76a853249caca3
--- /dev/null
+++ b/mlir/test/Target/LLVMIR/omptarget-debug.mlir
@@ -0,0 +1,29 @@
+// RUN: mlir-translate -mlir-to-llvmir %s | FileCheck %s
+
+module attributes {omp.is_target_device = true} {
+ 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]])
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]])
More information about the Mlir-commits
mailing list