Skip to content

Commit b4d7e29

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 576842c commit b4d7e29

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
@@ -110,7 +110,7 @@ bool debugInfoIsAlreadySet(mlir::Location loc) {
110110
// getTargetEntryUniqueInfo and
111111
// TargetRegionEntryInfo::getTargetRegionEntryFnName to generate the same name.
112112
// But even if there was a slight mismatch, it is not a problem because this
113-
// name is artifical and not important to debug experience.
113+
// name is artificial and not important to debug experience.
114114
mlir::StringAttr getTargetFunctionName(mlir::MLIRContext *context,
115115
mlir::Location Loc,
116116
llvm::StringRef parentName) {
@@ -478,13 +478,81 @@ void AddDebugInfoPass::handleFuncOp(mlir::func::FuncOp funcOp,
478478
line - 1, false);
479479
}
480480

481+
auto addTargetOpDISP = [&](bool lineTableOnly,
482+
const llvm::SmallVector<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+
mlir::LLVM::DISubroutineTypeAttr spTy =
511+
mlir::LLVM::DISubroutineTypeAttr::get(context, CC, types);
512+
if (lineTableOnly) {
513+
auto spAttr = mlir::LLVM::DISubprogramAttr::get(
514+
context, id, compilationUnit, Scope, name, name, funcFileAttr, line,
515+
line, flags, spTy, /*retainedNodes=*/{}, /*annotations=*/{});
516+
targetOp->setLoc(builder.getFusedLoc({targetOp.getLoc()}, spAttr));
517+
return;
518+
}
519+
mlir::DistinctAttr recId =
520+
mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
521+
auto spAttr = mlir::LLVM::DISubprogramAttr::get(
522+
context, recId, /*isRecSelf=*/true, id, compilationUnit, Scope, name,
523+
name, funcFileAttr, line, line, flags, spTy, /*retainedNodes=*/{},
524+
/*annotations=*/{});
525+
526+
// Make sure that information about the imported modules is copied in the
527+
// new function.
528+
llvm::SmallVector<mlir::LLVM::DINodeAttr> opEntities;
529+
for (mlir::LLVM::DINodeAttr N : entities) {
530+
if (auto entity = mlir::dyn_cast<mlir::LLVM::DIImportedEntityAttr>(N)) {
531+
auto importedEntity = mlir::LLVM::DIImportedEntityAttr::get(
532+
context, llvm::dwarf::DW_TAG_imported_module, spAttr,
533+
entity.getEntity(), fileAttr, /*line=*/1, /*name=*/nullptr,
534+
/*elements*/ {});
535+
opEntities.push_back(importedEntity);
536+
}
537+
}
538+
539+
id = mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
540+
spAttr = mlir::LLVM::DISubprogramAttr::get(
541+
context, recId, /*isRecSelf=*/false, id, compilationUnit, Scope, name,
542+
name, funcFileAttr, line, line, flags, spTy, opEntities,
543+
/*annotations=*/{});
544+
targetOp->setLoc(builder.getFusedLoc({targetOp.getLoc()}, spAttr));
545+
});
546+
};
547+
481548
// Don't process variables if user asked for line tables only.
482549
if (debugLevel == mlir::LLVM::DIEmissionKind::LineTablesOnly) {
483550
auto spAttr = mlir::LLVM::DISubprogramAttr::get(
484551
context, id, compilationUnit, Scope, funcName, fullName, funcFileAttr,
485552
line, line, subprogramFlags, subTypeAttr, /*retainedNodes=*/{},
486553
/*annotations=*/{});
487554
funcOp->setLoc(builder.getFusedLoc({l}, spAttr));
555+
addTargetOpDISP(true, {});
488556
return;
489557
}
490558

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

603615
funcOp.walk([&](fir::cg::XDeclareOp declOp) {
604616
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)