Skip to content

Commit e476d57

Browse files
abidhabidh
committed
[flang][debug] Generate DISubprogramAttr for omp::TargetOp.
There are DeclareOp present for the variables mapped into target region. That allow us to generate debug information for them. Bu the TargetOp is still part of parent function and those variables get the parent function's DISubprogram as a scope. In OMPIRBuilder, a new function is created for the TargetOp. We also create a new DISubprogram for it. All the variables that were in the target region now have to be updated to have the correct scope. This after the fact updating of debug information becomes very difficult in certain cases. Take the example of variable arrays. The type of those arrays depend on the artificial DILocalVariable(s) which hold the size(s) of the array. This new function will now require that we generate the new variable and and new types. Similar issue exist for character type variables too. To avoid this after the fact updating, this PR generates a DISubprogramAttr for the TargetOp while generating the debug info in flang. This help us avoid updating later. This commit is flang side of the change.
1 parent 90e20d4 commit e476d57

File tree

3 files changed

+209
-1
lines changed

3 files changed

+209
-1
lines changed

‎flang/lib/Optimizer/Transforms/AddDebugInfo.cpp‎

Lines changed: 116 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
#include "llvm/BinaryFormat/Dwarf.h"
3636
#include "llvm/Support/Debug.h"
3737
#include "llvm/Support/FileSystem.h"
38+
#include "llvm/Support/FormatVariadic.h"
3839
#include "llvm/Support/Path.h"
3940
#include "llvm/Support/raw_ostream.h"
4041

@@ -104,6 +105,37 @@ bool debugInfoIsAlreadySet(mlir::Location loc) {
104105
return false;
105106
}
106107

108+
// Generates the name for the artificial DISubprogram that we are going to
109+
// generate for omp::TargetOp. Its logic is borrowed from
110+
// getTargetEntryUniqueInfo and
111+
// TargetRegionEntryInfo::getTargetRegionEntryFnName to generate the same name.
112+
// But even if there was a slight mismatch, it is not a problem because this
113+
// name is artificial and not important to debug experience.
114+
mlir::StringAttr getTargetFunctionName(mlir::MLIRContext *context,
115+
mlir::Location Loc,
116+
llvm::StringRef parentName) {
117+
auto fileLoc = Loc->findInstanceOf<mlir::FileLineColLoc>();
118+
119+
assert(fileLoc && "No file found from location");
120+
llvm::StringRef fileName = fileLoc.getFilename().getValue();
121+
122+
llvm::sys::fs::UniqueID id;
123+
uint64_t line = fileLoc.getLine();
124+
size_t fileId;
125+
size_t deviceId;
126+
if (auto ec = llvm::sys::fs::getUniqueID(fileName, id)) {
127+
fileId = llvm::hash_value(fileName.str());
128+
deviceId = 0xdeadf17e;
129+
} else {
130+
fileId = id.getFile();
131+
deviceId = id.getDevice();
132+
}
133+
return mlir::StringAttr::get(
134+
context,
135+
std::string(llvm::formatv("__omp_offloading_{0:x-}_{1:x-}_{2}_l{3}",
136+
deviceId, fileId, parentName, line)));
137+
}
138+
107139
} // namespace
108140

109141
bool AddDebugInfoPass::createCommonBlockGlobal(
@@ -446,13 +478,87 @@ void AddDebugInfoPass::handleFuncOp(mlir::func::FuncOp funcOp,
446478
line - 1, false);
447479
}
448480

481+
auto addTargetOpDISP = [&](bool lineTableOnly,
482+
llvm::ArrayRef<mlir::LLVM::DINodeAttr> entities) {
483+
// When we process the DeclareOp inside the OpenMP target region, all the
484+
// variables get the DISubprogram of the parent function of the target op as
485+
// the scope. In the codegen (to llvm ir), OpenMP target op results in the
486+
// creation of a separate function. As the variables in the debug info have
487+
// the DISubprogram of the parent function as the scope, the variables
488+
// need to be updated at codegen time to avoid verification failures.
489+
490+
// This updating after the fact becomes more and more difficult when types
491+
// are dependent on local variables like in the case of variable size arrays
492+
// or string. We not only have to generate new variables but also new types.
493+
// We can avoid this problem by generating a DISubprogramAttr here for the
494+
// target op and make sure that all the variables inside the target region
495+
// get the correct scope in the first place.
496+
funcOp.walk([&](mlir::omp::TargetOp targetOp) {
497+
unsigned line = getLineFromLoc(targetOp.getLoc());
498+
mlir::StringAttr name =
499+
getTargetFunctionName(context, targetOp.getLoc(), funcOp.getName());
500+
mlir::LLVM::DISubprogramFlags flags =
501+
mlir::LLVM::DISubprogramFlags::Definition |
502+
mlir::LLVM::DISubprogramFlags::LocalToUnit;
503+
if (isOptimized)
504+
flags = flags | mlir::LLVM::DISubprogramFlags::Optimized;
505+
506+
mlir::DistinctAttr id =
507+
mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
508+
llvm::SmallVector<mlir::LLVM::DITypeAttr> types;
509+
types.push_back(mlir::LLVM::DINullTypeAttr::get(context));
510+
for (auto arg : targetOp.getRegion().getArguments()) {
511+
auto tyAttr = typeGen.convertType(fir::unwrapRefType(arg.getType()),
512+
fileAttr, cuAttr, /*declOp=*/nullptr);
513+
types.push_back(tyAttr);
514+
}
515+
CC = llvm::dwarf::getCallingConvention("DW_CC_normal");
516+
mlir::LLVM::DISubroutineTypeAttr spTy =
517+
mlir::LLVM::DISubroutineTypeAttr::get(context, CC, types);
518+
if (lineTableOnly) {
519+
auto spAttr = mlir::LLVM::DISubprogramAttr::get(
520+
context, id, compilationUnit, Scope, name, name, funcFileAttr, line,
521+
line, flags, spTy, /*retainedNodes=*/{}, /*annotations=*/{});
522+
targetOp->setLoc(builder.getFusedLoc({targetOp.getLoc()}, spAttr));
523+
return;
524+
}
525+
mlir::DistinctAttr recId =
526+
mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
527+
auto spAttr = mlir::LLVM::DISubprogramAttr::get(
528+
context, recId, /*isRecSelf=*/true, id, compilationUnit, Scope, name,
529+
name, funcFileAttr, line, line, flags, spTy, /*retainedNodes=*/{},
530+
/*annotations=*/{});
531+
532+
// Make sure that information about the imported modules is copied in the
533+
// new function.
534+
llvm::SmallVector<mlir::LLVM::DINodeAttr> opEntities;
535+
for (mlir::LLVM::DINodeAttr N : entities) {
536+
if (auto entity = mlir::dyn_cast<mlir::LLVM::DIImportedEntityAttr>(N)) {
537+
auto importedEntity = mlir::LLVM::DIImportedEntityAttr::get(
538+
context, llvm::dwarf::DW_TAG_imported_module, spAttr,
539+
entity.getEntity(), fileAttr, /*line=*/1, /*name=*/nullptr,
540+
/*elements*/ {});
541+
opEntities.push_back(importedEntity);
542+
}
543+
}
544+
545+
id = mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
546+
spAttr = mlir::LLVM::DISubprogramAttr::get(
547+
context, recId, /*isRecSelf=*/false, id, compilationUnit, Scope, name,
548+
name, funcFileAttr, line, line, flags, spTy, opEntities,
549+
/*annotations=*/{});
550+
targetOp->setLoc(builder.getFusedLoc({targetOp.getLoc()}, spAttr));
551+
});
552+
};
553+
449554
// Don't process variables if user asked for line tables only.
450555
if (debugLevel == mlir::LLVM::DIEmissionKind::LineTablesOnly) {
451556
auto spAttr = mlir::LLVM::DISubprogramAttr::get(
452557
context, id, compilationUnit, Scope, funcName, fullName, funcFileAttr,
453558
line, line, subprogramFlags, subTypeAttr, /*retainedNodes=*/{},
454559
/*annotations=*/{});
455560
funcOp->setLoc(builder.getFusedLoc({l}, spAttr));
561+
addTargetOpDISP(/*lineTableOnly=*/true, /*entities=*/{});
456562
return;
457563
}
458564

@@ -510,9 +616,18 @@ void AddDebugInfoPass::handleFuncOp(mlir::func::FuncOp funcOp,
510616
funcName, fullName, funcFileAttr, line, line, subprogramFlags,
511617
subTypeAttr, entities, /*annotations=*/{});
512618
funcOp->setLoc(builder.getFusedLoc({l}, spAttr));
619+
addTargetOpDISP(/*lineTableOnly=*/false, entities);
513620

514621
funcOp.walk([&](fir::cg::XDeclareOp declOp) {
515-
handleDeclareOp(declOp, fileAttr, spAttr, typeGen, symbolTable);
622+
mlir::LLVM::DISubprogramAttr spTy = spAttr;
623+
if (auto tOp = declOp->getParentOfType<mlir::omp::TargetOp>()) {
624+
if (auto fusedLoc = llvm::dyn_cast<mlir::FusedLoc>(tOp.getLoc())) {
625+
if (auto sp = llvm::dyn_cast<mlir::LLVM::DISubprogramAttr>(
626+
fusedLoc.getMetadata()))
627+
spTy = sp;
628+
}
629+
}
630+
handleDeclareOp(declOp, fileAttr, spTy, typeGen, symbolTable);
516631
});
517632
// commonBlockMap ensures that we don't create multiple DICommonBlockAttr of
518633
// the same name in one function. But it is ok (rather required) to create

‎flang/test/Transforms/debug-omp-target-op-1.fir‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s | FileCheck %s
2+
// RUN: fir-opt --add-debug-info="debug-level=LineTablesOnly" --mlir-print-debuginfo %s | FileCheck %s --check-prefix=LINETABLE
3+
4+
module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
5+
func.func @_QQmain() attributes {fir.bindc_name = "test"} {
6+
%c13_i32 = arith.constant 13 : i32
7+
%c12_i32 = arith.constant 12 : i32
8+
%c6_i32 = arith.constant 6 : i32
9+
%c1_i32 = arith.constant 1 : i32
10+
%c5_i32 = arith.constant 5 : i32
11+
%0 = fir.alloca i32 {bindc_name = "x", uniq_name = "_QFEx"} loc(#loc1)
12+
%1 = fircg.ext_declare %0 {uniq_name = "_QFEx"} : (!fir.ref<i32>) -> !fir.ref<i32> loc(#loc1)
13+
%2 = fir.alloca i32 {bindc_name = "y", uniq_name = "_QFEy"} loc(#loc2)
14+
%3 = fircg.ext_declare %2 {uniq_name = "_QFEy"} : (!fir.ref<i32>) -> !fir.ref<i32> loc(#loc2)
15+
%4 = omp.map.info var_ptr(%1 : !fir.ref<i32>, i32) map_clauses(tofrom) capture(ByRef) -> !fir.ref<i32> {name = "x"}
16+
%5 = omp.map.info var_ptr(%3 : !fir.ref<i32>, i32) map_clauses(tofrom) capture(ByRef) -> !fir.ref<i32> {name = "y"}
17+
omp.target map_entries(%4 -> %arg0, %5 -> %arg1 : !fir.ref<i32>, !fir.ref<i32>) {
18+
%16 = fircg.ext_declare %arg0 {uniq_name = "_QFEx"} : (!fir.ref<i32>) -> !fir.ref<i32> loc(#loc3)
19+
%17 = fircg.ext_declare %arg1 {uniq_name = "_QFEy"} : (!fir.ref<i32>) -> !fir.ref<i32> loc(#loc4)
20+
omp.terminator
21+
} loc(#loc5)
22+
return
23+
}
24+
}
25+
#loc1 = loc("test.f90":1:1)
26+
#loc2 = loc("test.f90":3:1)
27+
#loc3 = loc("test.f90":7:1)
28+
#loc4 = loc("test.f90":8:1)
29+
#loc5 = loc("test.f90":6:1)
30+
31+
// CHECK: #[[SP:.*]] = #llvm.di_subprogram<{{.*}}name = "test"{{.*}}>
32+
// CHECK: #[[SP1:.*]] = #llvm.di_subprogram<{{.*}}name = "__omp_offloading_{{.*}}_QQmain_l6"{{.*}}line = 6{{.*}}subprogramFlags = "LocalToUnit|Definition"{{.*}}>
33+
// CHECK: #llvm.di_local_variable<scope = #[[SP]], name = "x"{{.*}}line = 1, type = #[[TY:.*]]>
34+
// CHECK: #llvm.di_local_variable<scope = #[[SP]], name = "y"{{.*}}line = 3, type = #[[TY]]>
35+
// CHECK: #llvm.di_local_variable<scope = #[[SP1]], name = "x"{{.*}}line = 7, type = #[[TY]]>
36+
// CHECK: #llvm.di_local_variable<scope = #[[SP1]], name = "y"{{.*}}line = 8, type = #[[TY]]>
37+
38+
// LINETABLE: #[[SP:.*]] = #llvm.di_subprogram<{{.*}}name = "test"{{.*}}>
39+
// LINETABLE: #[[SP1:.*]] = #llvm.di_subprogram<{{.*}}name = "__omp_offloading_{{.*}}_QQmain_l6"{{.*}}line = 6{{.*}}subprogramFlags = "LocalToUnit|Definition"{{.*}}>
40+
// LINETABLE-NOT: #llvm.di_local_variable

‎flang/test/Transforms/debug-omp-target-op-2.fir‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
// RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s | FileCheck %s
2+
3+
module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
4+
func.func @fn_(%arg0: !fir.ref<!fir.array<?x?xi32>> {fir.bindc_name = "b"}, %arg1: !fir.ref<i32> {fir.bindc_name = "c"}, %arg2: !fir.ref<i32> {fir.bindc_name = "d"}) {
5+
%c1 = arith.constant 1 : index
6+
%c0 = arith.constant 0 : index
7+
%0 = fir.alloca i32
8+
%1 = fir.alloca i32
9+
%2 = fir.undefined !fir.dscope
10+
%3 = fircg.ext_declare %arg1 dummy_scope %2 {uniq_name = "_QFfnEc"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32> loc(#loc2)
11+
%4 = fircg.ext_declare %arg2 dummy_scope %2 {uniq_name = "_QFfnEd"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32> loc(#loc3)
12+
%5 = fir.load %3 : !fir.ref<i32>
13+
%6 = fir.convert %5 : (i32) -> index
14+
%9 = fir.load %4 : !fir.ref<i32>
15+
%10 = fir.convert %9 : (i32) -> index
16+
%15 = fircg.ext_declare %arg0(%6, %10) dummy_scope %2 {uniq_name = "_QFfnEb"} : (!fir.ref<!fir.array<?x?xi32>>, index, index, !fir.dscope) -> !fir.ref<!fir.array<?x?xi32>> loc(#loc4)
17+
%16 = fircg.ext_embox %15(%6, %10) : (!fir.ref<!fir.array<?x?xi32>>, index, index) -> !fir.box<!fir.array<?x?xi32>>
18+
%17:3 = fir.box_dims %16, %c0 : (!fir.box<!fir.array<?x?xi32>>, index) -> (index, index, index)
19+
%18 = arith.subi %17#1, %c1 : index
20+
%19 = omp.map.bounds lower_bound(%c0 : index) upper_bound(%18 : index) extent(%17#1 : index) stride(%17#2 : index) start_idx(%c1 : index) {stride_in_bytes = true}
21+
%20 = arith.muli %17#2, %17#1 : index
22+
%21:3 = fir.box_dims %16, %c1 : (!fir.box<!fir.array<?x?xi32>>, index) -> (index, index, index)
23+
%22 = arith.subi %21#1, %c1 : index
24+
%23 = omp.map.bounds lower_bound(%c0 : index) upper_bound(%22 : index) extent(%21#1 : index) stride(%20 : index) start_idx(%c1 : index) {stride_in_bytes = true}
25+
%24 = omp.map.info var_ptr(%15 : !fir.ref<!fir.array<?x?xi32>>, i32) map_clauses(tofrom) capture(ByRef) bounds(%19, %23) -> !fir.ref<!fir.array<?x?xi32>> {name = "b"}
26+
%25 = omp.map.info var_ptr(%1 : !fir.ref<i32>, i32) map_clauses(implicit, exit_release_or_enter_alloc) capture(ByCopy) -> !fir.ref<i32> {name = ""}
27+
%26 = omp.map.info var_ptr(%0 : !fir.ref<i32>, i32) map_clauses(implicit, exit_release_or_enter_alloc) capture(ByCopy) -> !fir.ref<i32> {name = ""}
28+
omp.target map_entries(%24 -> %arg3, %25 -> %arg4, %26 -> %arg5 : !fir.ref<!fir.array<?x?xi32>>, !fir.ref<i32>, !fir.ref<i32>) {
29+
%27 = fir.load %arg5 : !fir.ref<i32>
30+
%28 = fir.load %arg4 : !fir.ref<i32>
31+
%29 = fir.convert %27 : (i32) -> index
32+
%31 = fir.convert %28 : (i32) -> index
33+
%37 = fircg.ext_declare %arg3(%29, %31) {uniq_name = "_QFfnEb"} : (!fir.ref<!fir.array<?x?xi32>>, index, index) -> !fir.ref<!fir.array<?x?xi32>> loc(#loc5)
34+
omp.terminator
35+
} loc(#loc6)
36+
return
37+
} loc(#loc7)
38+
}
39+
#loc1 = loc("test.f90":1:1)
40+
#loc2 = loc("test.f90":3:1)
41+
#loc3 = loc("test.f90":7:1)
42+
#loc4 = loc("test.f90":8:1)
43+
#loc5 = loc("test.f90":6:1)
44+
#loc6 = loc("test.f90":16:1)
45+
#loc7 = loc("test.f90":26:1)
46+
47+
48+
// Test that variable size arrays inside target regions get their own
49+
// compiler generated variables for size.
50+
51+
// CHECK: #[[SP:.*]] = #llvm.di_subprogram<{{.*}}name = "__omp_offloading_{{.*}}_fn__l16"{{.*}}>
52+
// CHECK: #llvm.di_local_variable<scope = #[[SP]], name = "._QFfnEb1"{{.*}}>
53+
// CHECK: #llvm.di_local_variable<scope = #[[SP]], name = "._QFfnEb2"{{.*}}>

0 commit comments

Comments
 (0)