[flang-commits] [flang] [flang][debug] Handle 'used' module. (PR #107626)
via flang-commits
flang-commits at lists.llvm.org
Fri Sep 6 11:02:17 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-fir-hlfir
Author: Abid Qadeer (abidh)
<details>
<summary>Changes</summary>
As described in #<!-- -->98883, we have to qualify a module variable name in debugger to get its value. This PR tries to remove this limitation.
LLVM provides `DIImportedEntity` to handle such cases but the PR is made more complicated due to the following 2 issues.
1. The MLIR attributes are readonly and we have a circular dependency here. This has to be handled using the recursive interface provided by the MLIR. This requires us to first create a place holder `DISubprogramAttr` which is used in creating `DIImportedEntityAttr`. Later another `DISubprogramAttr` is created which replaces the place holder.
2. The flang IR does not provide any information about the 'used' module so this has to be extracted by doing a pass over the
`DeclareOp` in the function. This presents certain limitation as 'only' and module variable renaming may not be handled properly.
Due to the change in `DISubprogramAttr`, some tests also needed to be adjusted.
Fixes #<!-- -->98883.
---
Full diff: https://github.com/llvm/llvm-project/pull/107626.diff
7 Files Affected:
- (modified) flang/lib/Optimizer/Transforms/AddDebugInfo.cpp (+111-33)
- (modified) flang/test/Integration/debug-module-2.f90 (+1-1)
- (modified) flang/test/Transforms/debug-90683.fir (+1-1)
- (modified) flang/test/Transforms/debug-fn-info.fir (+3-3)
- (added) flang/test/Transforms/debug-imported-entity.fir (+30)
- (modified) flang/test/Transforms/debug-line-table-inc-file.fir (+2-2)
- (modified) flang/test/Transforms/debug-local-global-storage-1.fir (+1-1)
``````````diff
diff --git a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
index 576e65ba6ecc50..c0675a2e6092ea 100644
--- a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
+++ b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
@@ -72,6 +72,10 @@ class AddDebugInfoPass : public fir::impl::AddDebugInfoBase<AddDebugInfoPass> {
mlir::LLVM::DICompileUnitAttr cuAttr,
fir::DebugTypeGenerator &typeGen,
mlir::SymbolTable *symbolTable);
+ std::optional<mlir::LLVM::DIModuleAttr>
+ getModuleAttrFromGlobalOp(fir::GlobalOp globalOp,
+ mlir::LLVM::DIFileAttr fileAttr,
+ mlir::LLVM::DIScopeAttr scope);
};
bool debugInfoIsAlreadySet(mlir::Location loc) {
@@ -152,6 +156,44 @@ mlir::LLVM::DIModuleAttr AddDebugInfoPass::getOrCreateModuleAttr(
return modAttr;
}
+/// If globalOp represents a module variable, return a ModuleAttr that
+/// represents that module.
+std::optional<mlir::LLVM::DIModuleAttr>
+AddDebugInfoPass::getModuleAttrFromGlobalOp(fir::GlobalOp globalOp,
+ mlir::LLVM::DIFileAttr fileAttr,
+ mlir::LLVM::DIScopeAttr scope) {
+ mlir::MLIRContext *context = &getContext();
+ mlir::OpBuilder builder(context);
+
+ std::pair result = fir::NameUniquer::deconstruct(globalOp.getSymName());
+ // Only look for module if this variable is not part of a function.
+ if (!result.second.procs.empty() || result.second.modules.empty())
+ return std::nullopt;
+
+ // DWARF5 says following about the fortran modules:
+ // A Fortran 90 module may also be represented by a module entry
+ // (but no declaration attribute is warranted because Fortran has no concept
+ // of a corresponding module body).
+ // But in practice, compilers use declaration attribute with a module in cases
+ // where module was defined in another source file (only being used in this
+ // one). The isInitialized() seems to provide the right information
+ // but inverted. It is true where module is actually defined but false where
+ // it is used.
+ // FIXME: Currently we don't have the line number on which a module was
+ // declared. We are using a best guess of line - 1 where line is the source
+ // line of the first member of the module that we encounter.
+ unsigned line = getLineFromLoc(globalOp.getLoc());
+
+ mlir::LLVM::DISubprogramAttr sp =
+ mlir::dyn_cast_if_present<mlir::LLVM::DISubprogramAttr>(scope);
+ // Modules are generated at compile unit scope
+ if (sp)
+ scope = sp.getCompileUnit();
+
+ return getOrCreateModuleAttr(result.second.modules[0], fileAttr, scope,
+ line - 1, !globalOp.isInitialized());
+}
+
void AddDebugInfoPass::handleGlobalOp(fir::GlobalOp globalOp,
mlir::LLVM::DIFileAttr fileAttr,
mlir::LLVM::DIScopeAttr scope,
@@ -174,33 +216,11 @@ void AddDebugInfoPass::handleGlobalOp(fir::GlobalOp globalOp,
return;
unsigned line = getLineFromLoc(globalOp.getLoc());
+ std::optional<mlir::LLVM::DIModuleAttr> modOpt =
+ getModuleAttrFromGlobalOp(globalOp, fileAttr, scope);
+ if (modOpt)
+ scope = *modOpt;
- // DWARF5 says following about the fortran modules:
- // A Fortran 90 module may also be represented by a module entry
- // (but no declaration attribute is warranted because Fortran has no concept
- // of a corresponding module body).
- // But in practice, compilers use declaration attribute with a module in cases
- // where module was defined in another source file (only being used in this
- // one). The isInitialized() seems to provide the right information
- // but inverted. It is true where module is actually defined but false where
- // it is used.
- // FIXME: Currently we don't have the line number on which a module was
- // declared. We are using a best guess of line - 1 where line is the source
- // line of the first member of the module that we encounter.
-
- if (result.second.procs.empty()) {
- // Only look for module if this variable is not part of a function.
- if (result.second.modules.empty())
- return;
-
- // Modules are generated at compile unit scope
- if (mlir::LLVM::DISubprogramAttr sp =
- mlir::dyn_cast_if_present<mlir::LLVM::DISubprogramAttr>(scope))
- scope = sp.getCompileUnit();
-
- scope = getOrCreateModuleAttr(result.second.modules[0], fileAttr, scope,
- line - 1, !globalOp.isInitialized());
- }
mlir::LLVM::DITypeAttr diType =
typeGen.convertType(globalOp.getType(), fileAttr, scope, declOp);
auto gvAttr = mlir::LLVM::DIGlobalVariableAttr::get(
@@ -262,7 +282,7 @@ void AddDebugInfoPass::handleFuncOp(mlir::func::FuncOp funcOp,
mlir::LLVM::DIFileAttr::get(context, fileName, filePath);
// Only definitions need a distinct identifier and a compilation unit.
- mlir::DistinctAttr id;
+ mlir::DistinctAttr id, id2;
mlir::LLVM::DIScopeAttr Scope = fileAttr;
mlir::LLVM::DICompileUnitAttr compilationUnit;
mlir::LLVM::DISubprogramFlags subprogramFlags =
@@ -270,7 +290,10 @@ void AddDebugInfoPass::handleFuncOp(mlir::func::FuncOp funcOp,
if (isOptimized)
subprogramFlags = mlir::LLVM::DISubprogramFlags::Optimized;
if (!funcOp.isExternal()) {
+ // Place holder and final function have to have different IDs, otherwise
+ // translation code will reject one of them.
id = mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
+ id2 = mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
compilationUnit = cuAttr;
subprogramFlags =
subprogramFlags | mlir::LLVM::DISubprogramFlags::Definition;
@@ -299,14 +322,69 @@ void AddDebugInfoPass::handleFuncOp(mlir::func::FuncOp funcOp,
line - 1, false);
}
- auto spAttr = mlir::LLVM::DISubprogramAttr::get(
- context, id, compilationUnit, Scope, funcName, fullName, funcFileAttr,
- line, line, subprogramFlags, subTypeAttr, /*retainedNodes=*/{});
- funcOp->setLoc(builder.getFusedLoc({funcOp->getLoc()}, spAttr));
-
// Don't process variables if user asked for line tables only.
- if (debugLevel == mlir::LLVM::DIEmissionKind::LineTablesOnly)
+ if (debugLevel == mlir::LLVM::DIEmissionKind::LineTablesOnly) {
+ auto spAttr = mlir::LLVM::DISubprogramAttr::get(
+ context, id, compilationUnit, Scope, funcName, fullName, funcFileAttr,
+ line, line, subprogramFlags, subTypeAttr, /*retainedNodes=*/{});
+ funcOp->setLoc(builder.getFusedLoc({l}, spAttr));
return;
+ }
+
+ mlir::DistinctAttr recId =
+ mlir::DistinctAttr::create(mlir::UnitAttr::get(context));
+
+ // The debug attribute in MLIR are readonly once created. But in case of
+ // imported entities, we have a circular dependency. The
+ // DIImportedEntityAttr requires scope information (DISubprogramAttr in this
+ // case) and DISubprogramAttr requires the list of imported entities. The
+ // MLIR provides a way where a DISubprogramAttr an be created with a certain
+ // recID and be used in places like DIImportedEntityAttr. After that another
+ // DISubprogramAttr can be created with same recID but with list of entities
+ // now available. The MLIR translation code takes care of updating the
+ // references. Note that references will be updated only in the things that
+ // are part of DISubprogramAttr (like DIImportedEntityAttr) so we have to
+ // create the final DISubprogramAttr before we process local variables.
+ // Look at DIRecursiveTypeAttrInterface for more details.
+
+ auto spAttr = mlir::LLVM::DISubprogramAttr::get(
+ context, recId, /*isRecSelf=*/true, id, compilationUnit, Scope, funcName,
+ fullName, funcFileAttr, line, line, subprogramFlags, subTypeAttr,
+ /*retainedNodes=*/{});
+
+ // There is no direct information in the IR for any 'use' statement in the
+ // function. We have to extract that information from the DeclareOp. We do
+ // a pass on the DeclareOp and generate ModuleAttr and corresponding
+ // DIImportedEntityAttr for that module.
+ // FIXME: As we are depending on the variables to see which module is being
+ // 'used' in the function, there are certain limitations.
+ // For things like 'use mod1, only: v1', whole module will be brought into the
+ // namespace in the debug info. It is not a problem as such unless there is a
+ // clash of names.
+ // There is no information about module variable renaming
+ llvm::DenseSet<mlir::LLVM::DIImportedEntityAttr> importedModules;
+ funcOp.walk([&](fir::cg::XDeclareOp declOp) {
+ if (&funcOp.front() == declOp->getBlock())
+ if (auto global =
+ symbolTable->lookup<fir::GlobalOp>(declOp.getUniqName())) {
+ std::optional<mlir::LLVM::DIModuleAttr> modOpt =
+ getModuleAttrFromGlobalOp(global, fileAttr, cuAttr);
+ if (modOpt) {
+ auto importedEntity = mlir::LLVM::DIImportedEntityAttr::get(
+ context, llvm::dwarf::DW_TAG_imported_module, spAttr, *modOpt,
+ fileAttr, /*line=*/1, /*name=*/nullptr, /*elements*/ {});
+ importedModules.insert(importedEntity);
+ }
+ }
+ });
+ llvm::SmallVector<mlir::LLVM::DINodeAttr> entities(importedModules.begin(),
+ importedModules.end());
+ // We have the imported entities now. Generate the final DISubprogramAttr.
+ spAttr = mlir::LLVM::DISubprogramAttr::get(
+ context, recId, /*isRecSelf=*/false, id2, compilationUnit, Scope,
+ funcName, fullName, funcFileAttr, line, line, subprogramFlags,
+ subTypeAttr, entities);
+ funcOp->setLoc(builder.getFusedLoc({l}, spAttr));
funcOp.walk([&](fir::cg::XDeclareOp declOp) {
// FIXME: We currently dont handle variables that are not in the entry
diff --git a/flang/test/Integration/debug-module-2.f90 b/flang/test/Integration/debug-module-2.f90
index 60fccaa2a6c1f2..f07416c3ef3cc8 100644
--- a/flang/test/Integration/debug-module-2.f90
+++ b/flang/test/Integration/debug-module-2.f90
@@ -17,7 +17,7 @@ module helper
integer gli
contains
-!CHECK-DAG: !DISubprogram(name: "test", linkageName: "_QMhelperPtest", scope: ![[MOD]], file: ![[FILE2]], line: [[@LINE+1]]{{.*}}unit: ![[CU]])
+!CHECK-DAG: !DISubprogram(name: "test", linkageName: "_QMhelperPtest", scope: ![[MOD]], file: ![[FILE2]], line: [[@LINE+1]]{{.*}}unit: ![[CU]]{{.*}})
subroutine test()
glr = 12.34
gli = 67
diff --git a/flang/test/Transforms/debug-90683.fir b/flang/test/Transforms/debug-90683.fir
index cc6929c10411f8..a21332e3968a73 100644
--- a/flang/test/Transforms/debug-90683.fir
+++ b/flang/test/Transforms/debug-90683.fir
@@ -22,4 +22,4 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
// CHECK-DAG: #[[TY:.*]] = #llvm.di_basic_type<tag = DW_TAG_base_type, name = "real", sizeInBits = 64, encoding = DW_ATE_float>
// CHECK-DAG: #[[TY1:.*]] = #llvm.di_subroutine_type<callingConvention = DW_CC_normal, types = #[[TY]], #[[TY]], #[[TY]]>
-// CHECK-DAG: #{{.*}} = #llvm.di_subprogram<scope = #{{.*}}, name = "cabs", linkageName = "cabs", file = #{{.*}}, line = {{.*}}, scopeLine = {{.*}}, type = #[[TY1]]>
+// CHECK-DAG: #{{.*}} = #llvm.di_subprogram<{{.*}}name = "cabs", linkageName = "cabs"{{.*}}, type = #[[TY1]]>
diff --git a/flang/test/Transforms/debug-fn-info.fir b/flang/test/Transforms/debug-fn-info.fir
index f456e35d3dd702..5433e088a648d0 100644
--- a/flang/test/Transforms/debug-fn-info.fir
+++ b/flang/test/Transforms/debug-fn-info.fir
@@ -69,7 +69,7 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
// CHECK: #[[TY2:.*]] = #llvm.di_subroutine_type<callingConvention = DW_CC_normal, types = #[[INT4]], #[[INT8]], #[[REAL4]], #[[LOG4]]>
// Line numbers should match the number in corresponding loc entry.
-// CHECK: #llvm.di_subprogram<id = {{.*}}, compileUnit = {{.*}}, scope = {{.*}}, name = "_QQmain", linkageName = "_QQmain", file = {{.*}}, line = 15, scopeLine = 15, subprogramFlags = Definition, type = #[[TY0]]>
-// CHECK: #llvm.di_subprogram<id = {{.*}}, compileUnit = {{.*}}, scope = {{.*}}, name = "fn1", linkageName = "_QFPfn1", file = {{.*}}, line = 26, scopeLine = 26, subprogramFlags = Definition, type = #[[TY1]]>
-// CHECK: #llvm.di_subprogram<id = {{.*}}, compileUnit = {{.*}}, scope = {{.*}}, name = "fn2", linkageName = "_QFPfn2", file = {{.*}}, line = 43, scopeLine = 43, subprogramFlags = Definition, type = #[[TY2]]>
+// CHECK: #llvm.di_subprogram<{{.*}}name = "_QQmain", linkageName = "_QQmain", file = {{.*}}, line = 15, scopeLine = 15, subprogramFlags = Definition, type = #[[TY0]]>
+// CHECK: #llvm.di_subprogram<{{.*}}name = "fn1", linkageName = "_QFPfn1", file = {{.*}}, line = 26, scopeLine = 26, subprogramFlags = Definition, type = #[[TY1]]>
+// CHECK: #llvm.di_subprogram<{{.*}}name = "fn2", linkageName = "_QFPfn2", file = {{.*}}, line = 43, scopeLine = 43, subprogramFlags = Definition, type = #[[TY2]]>
diff --git a/flang/test/Transforms/debug-imported-entity.fir b/flang/test/Transforms/debug-imported-entity.fir
new file mode 100644
index 00000000000000..7be6531a703a89
--- /dev/null
+++ b/flang/test/Transforms/debug-imported-entity.fir
@@ -0,0 +1,30 @@
+// RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s | FileCheck %s
+
+
+module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
+ fir.global @_QMfooEv1 : i32 {
+ %0 = fir.zero_bits i32
+ fir.has_value %0 : i32
+ }
+ fir.global internal @_QFtestExyz : i32 {
+ %c12_i32 = arith.constant 12 : i32
+ fir.has_value %c12_i32 : i32
+ } loc(#loc4)
+ func.func @test() attributes {fir.bindc_name = "test"} {
+ %0 = fir.address_of(@_QMfooEv1) : !fir.ref<i32>
+ %1 = fircg.ext_declare %0 {uniq_name = "_QMfooEv1"} : (!fir.ref<i32>) -> !fir.ref<i32> loc(#loc1)
+ %4 = fir.address_of(@_QFtestExyz) : !fir.ref<i32>
+ %5 = fircg.ext_declare %4 {uniq_name = "_QFtestExyz"} : (!fir.ref<i32>) -> !fir.ref<i32> loc(#loc4)
+ return
+ } loc(#loc3)
+}
+#loc1 = loc("test.f90":2:14)
+#loc2 = loc("test.f90":6:1)
+#loc3 = loc("test.f90":10:1)
+#loc4 = loc("test.f90":13:1)
+
+// CHECK: #[[MOD:.+]] = #llvm.di_module<{{.*}}name = "foo"{{.*}}>
+// CHECK: #[[SP_REC:.+]] = #llvm.di_subprogram<recId = distinct[[[REC_ID:[0-9]+]]]<>, isRecSelf = true{{.*}}>
+// CHECK: #[[IMP_ENTITY:.+]] = #llvm.di_imported_entity<tag = DW_TAG_imported_module, scope = #[[SP_REC]], entity = #[[MOD]]{{.*}}>
+// CHECK: #[[SP:.+]] = #llvm.di_subprogram<recId = distinct[[[REC_ID]]]<>{{.*}}retainedNodes = #[[IMP_ENTITY]]>
+// CHECK: #llvm.di_global_variable<scope = #[[SP]], name = "xyz"{{.*}}>
diff --git a/flang/test/Transforms/debug-line-table-inc-file.fir b/flang/test/Transforms/debug-line-table-inc-file.fir
index 065039b59c5ae8..216cd5e016f2f8 100644
--- a/flang/test/Transforms/debug-line-table-inc-file.fir
+++ b/flang/test/Transforms/debug-line-table-inc-file.fir
@@ -31,7 +31,7 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<>} {
// CHECK: #[[LOC_INC_FILE:.*]] = loc("{{.*}}inc.f90":1:1)
// CHECK: #[[LOC_FILE:.*]] = loc("{{.*}}simple.f90":3:1)
// CHECK: #[[DI_CU:.*]] = #llvm.di_compile_unit<id = distinct[{{.*}}]<>, sourceLanguage = DW_LANG_Fortran95, file = #[[DI_FILE]], producer = "{{.*}}flang{{.*}}", isOptimized = false, emissionKind = LineTablesOnly>
-// CHECK: #[[DI_SP_INC:.*]] = #llvm.di_subprogram<id = distinct[{{.*}}]<>, compileUnit = #[[DI_CU]], scope = #[[DI_FILE]], name = "sinc", linkageName = "_QPsinc", file = #[[DI_INC_FILE]], {{.*}}>
-// CHECK: #[[DI_SP:.*]] = #llvm.di_subprogram<id = distinct[{{.*}}]<>, compileUnit = #[[DI_CU]], scope = #[[DI_FILE]], name = "_QQmain", linkageName = "_QQmain", file = #[[DI_FILE]], {{.*}}>
+// CHECK: #[[DI_SP_INC:.*]] = #llvm.di_subprogram<{{.*}}id = distinct[{{.*}}]<>, compileUnit = #[[DI_CU]], scope = #[[DI_FILE]], name = "sinc", linkageName = "_QPsinc", file = #[[DI_INC_FILE]], {{.*}}>
+// CHECK: #[[DI_SP:.*]] = #llvm.di_subprogram<{{.*}}id = distinct[{{.*}}]<>, compileUnit = #[[DI_CU]], scope = #[[DI_FILE]], name = "_QQmain", linkageName = "_QQmain", file = #[[DI_FILE]], {{.*}}>
// CHECK: #[[FUSED_LOC_INC_FILE]] = loc(fused<#[[DI_SP_INC]]>[#[[LOC_INC_FILE]]])
// CHECK: #[[FUSED_LOC_FILE]] = loc(fused<#[[DI_SP]]>[#[[LOC_FILE]]])
diff --git a/flang/test/Transforms/debug-local-global-storage-1.fir b/flang/test/Transforms/debug-local-global-storage-1.fir
index d9d8083a14709e..83a9055a6b8dc3 100644
--- a/flang/test/Transforms/debug-local-global-storage-1.fir
+++ b/flang/test/Transforms/debug-local-global-storage-1.fir
@@ -45,7 +45,7 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<i64, dense<64> :
// CHECK-DAG: #[[CU:.*]] = #llvm.di_compile_unit<{{.*}}>
// CHECK-DAG: #[[MOD:.*]] = #llvm.di_module<{{.*}}scope = #[[CU]]{{.*}}name = "example"{{.*}}>
// CHECK-DAG: #[[SP:.*]] = #llvm.di_subprogram<{{.*}}name = "_QQmain"{{.*}}>
-// CHECK-DAG: #[[MOD_SP:.*]] = #llvm.di_subprogram<{{.*}}name = "mod_sub"{{.*}}>
+// CHECK-DAG: #[[MOD_SP:.*]] = #llvm.di_subprogram<{{.*}}name = "mod_sub"{{.*}}retainedNodes = {{.*}}>
// CHECK-DAG: #llvm.di_global_variable<scope = #[[SP]], name = "arr"{{.*}}line = 22{{.*}}>
// CHECK-DAG: #llvm.di_global_variable<scope = #[[SP]], name = "s"{{.*}}line = 23{{.*}}>
// CHECK-DAG: #llvm.di_global_variable<scope = #[[MOD_SP]], name = "ss"{{.*}}line = 12{{.*}}>
``````````
</details>
https://github.com/llvm/llvm-project/pull/107626
More information about the flang-commits
mailing list