Skip to content

Commit 73fbd7b

Browse files
abidhabidh
committed
Handle review comments.
Main change is that we also take care of the case when only line table information is requested.
1 parent e038383 commit 73fbd7b

File tree

2 files changed

+75
-58
lines changed

2 files changed

+75
-58
lines changed

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

Lines changed: 70 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ bool debugInfoIsAlreadySet(mlir::Location loc) {
109109
// getTargetEntryUniqueInfo and
110110
// TargetRegionEntryInfo::getTargetRegionEntryFnName to generate the same name.
111111
// But even if there was a slight mismatch, it is not a problem because this
112-
// name is artifical and not important to debug experience.
112+
// name is artificial and not important to debug experience.
113113
mlir::StringAttr getTargetFunctionName(mlir::MLIRContext *context,
114114
mlir::Location Loc,
115115
llvm::StringRef parentName) {
@@ -477,13 +477,81 @@ void AddDebugInfoPass::handleFuncOp(mlir::func::FuncOp funcOp,
477477
line - 1, false);
478478
}
479479

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

@@ -541,63 +609,7 @@ void AddDebugInfoPass::handleFuncOp(mlir::func::FuncOp funcOp,
541609
funcName, fullName, funcFileAttr, line, line, subprogramFlags,
542610
subTypeAttr, entities, /*annotations=*/{});
543611
funcOp->setLoc(builder.getFusedLoc({l}, spAttr));
544-
545-
/* When we process the DeclareOp inside the OpenMP target region, all the
546-
variables get the DISubprogram of the parent function of the target op as
547-
the scope. In the codegen (to llvm ir), OpenMP target op results in the
548-
creation of a separate function. As the variables in the debug info have
549-
the DISubprogram of the parent function as the scope, the variables
550-
need to be updated at codegen time to avoid verification failures.
551-
552-
This updating after the fact becomes more and more difficult when types
553-
are dependent on local variables like in the case of variable size arrays
554-
or string. We not only have to generate new variables but also new types.
555-
We can avoid this problem by generating a DISubprogramAttr here for the
556-
target op and make sure that all the variables inside the target region
557-
get the correct scope in the first place. */
558-
funcOp.walk([&](mlir::omp::TargetOp targetOp) {
559-
unsigned line = getLineFromLoc(targetOp.getLoc());
560-
mlir::StringAttr Name =
561-
getTargetFunctionName(context, targetOp.getLoc(), funcOp.getName());
562-
mlir::LLVM::DISubprogramFlags flags =
563-
mlir::LLVM::DISubprogramFlags::Definition |
564-
mlir::LLVM::DISubprogramFlags::LocalToUnit;
565-
if (isOptimized)
566-
flags = flags | mlir::LLVM::DISubprogramFlags::Optimized;
567-
568-
mlir::DistinctAttr recId =
569-
mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
570-
mlir::DistinctAttr Id =
571-
mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
572-
llvm::SmallVector<mlir::LLVM::DITypeAttr> types;
573-
types.push_back(mlir::LLVM::DINullTypeAttr::get(context));
574-
mlir::LLVM::DISubroutineTypeAttr spTy =
575-
mlir::LLVM::DISubroutineTypeAttr::get(context, CC, types);
576-
auto spAttr = mlir::LLVM::DISubprogramAttr::get(
577-
context, recId, /*isRecSelf=*/true, Id, compilationUnit, Scope, Name,
578-
Name, funcFileAttr, line, line, flags, spTy, /*retainedNodes=*/{},
579-
/*annotations=*/{});
580-
581-
// Make sure that information about the imported modules in copied from the
582-
// parent function.
583-
llvm::SmallVector<mlir::LLVM::DINodeAttr> OpEntities;
584-
for (mlir::LLVM::DINodeAttr N : entities) {
585-
if (auto entity = mlir::dyn_cast<mlir::LLVM::DIImportedEntityAttr>(N)) {
586-
auto importedEntity = mlir::LLVM::DIImportedEntityAttr::get(
587-
context, llvm::dwarf::DW_TAG_imported_module, spAttr,
588-
entity.getEntity(), fileAttr, /*line=*/1, /*name=*/nullptr,
589-
/*elements*/ {});
590-
OpEntities.push_back(importedEntity);
591-
}
592-
}
593-
594-
Id = mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
595-
spAttr = mlir::LLVM::DISubprogramAttr::get(
596-
context, recId, /*isRecSelf=*/false, Id, compilationUnit, Scope, Name,
597-
Name, funcFileAttr, line, line, flags, spTy, OpEntities,
598-
/*annotations=*/{});
599-
targetOp->setLoc(builder.getFusedLoc({targetOp.getLoc()}, spAttr));
600-
});
612+
addTargetOpDISP(false, entities);
601613

602614
funcOp.walk([&](fir::cg::XDeclareOp declOp) {
603615
mlir::LLVM::DISubprogramAttr spTy = spAttr;

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
// 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
23

34
module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
45
func.func @_QQmain() attributes {fir.bindc_name = "test"} {
@@ -33,3 +34,7 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
3334
// CHECK: #llvm.di_local_variable<scope = #[[SP]], name = "y"{{.*}}line = 3, type = #[[TY]]>
3435
// CHECK: #llvm.di_local_variable<scope = #[[SP1]], name = "x"{{.*}}line = 7, type = #[[TY]]>
3536
// 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

0 commit comments

Comments
 (0)