[flang][debug] Fix issues with local variables. #98661
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR fixes 2 similar issues. 1. As reported in llvm#97476, flang generated executable has inconsistent behavior regarding values of the local array variables. 2. Variable with save attribute would not show up in debugger. The reason behind is same for both cases. If a local variable has storage which extends beyond function lifetime, the way to represent it in the debug info is through a global variable whose scope is limited to the function. This is what is used for static local variable in C. Previously local array worked in cases they were on stack. But will not show up if they had a global storage. To fix this, if we can get a corresponding GlobalOp for a variable while processing DeclareOp, we treat it as a global variable.
abidh
requested review from
tblah,
kiranchandramohan,
jeanPerier,
dpalermo,
pawosm-arm,
jinisusan and
kiranktp
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir Author: Abid Qadeer (abidh) ChangesThis PR fixes 2 similar issues.
The reason behind is same for both cases. If a local variable has storage which extends beyond function lifetime, the way to represent it in the debug info is through a global variable whose scope is limited to the function. This is what is used for static local variable in C. Previously local array worked in cases they were on stack. But will not show up if they had a global storage. To fix this, if we can get a corresponding With this fix in place, all the issues described in #97476 go away. It also fixes a lot of fails in GDB's fortran testsuite. Fixes #97476. Full diff: https://github.com/llvm/llvm-project/pull/98661.diff 3 Files Affected:
diff --git a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
index 10c71d3fc9551..2e7c3007b7980 100644
--- a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
+++ b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
@@ -80,12 +80,19 @@ void AddDebugInfoPass::handleDeclareOp(fir::cg::XDeclareOp declOp,
mlir::LLVM::DIScopeAttr scopeAttr,
fir::DebugTypeGenerator &typeGen) {
mlir::MLIRContext *context = &getContext();
+ mlir::ModuleOp module = getOperation();
mlir::OpBuilder builder(context);
auto result = fir::NameUniquer::deconstruct(declOp.getUniqName());
if (result.first != fir::NameUniquer::NameKind::VARIABLE)
return;
+ // If this DeclareOp actually represents a global then treat it as such.
+ if (auto global = module.lookupSymbol<fir::GlobalOp>(declOp.getUniqName())) {
+ handleGlobalOp(global, fileAttr, scopeAttr);
+ return;
+ }
+
// Only accept local variables.
if (result.second.procs.empty())
return;
@@ -139,6 +146,8 @@ mlir::LLVM::DIModuleAttr AddDebugInfoPass::getOrCreateModuleAttr(
void AddDebugInfoPass::handleGlobalOp(fir::GlobalOp globalOp,
mlir::LLVM::DIFileAttr fileAttr,
mlir::LLVM::DIScopeAttr scope) {
+ if (mlir::isa<mlir::FusedLoc>(globalOp.getLoc()))
+ return;
mlir::ModuleOp module = getOperation();
mlir::MLIRContext *context = &getContext();
fir::DebugTypeGenerator typeGen(module);
@@ -163,12 +172,19 @@ void AddDebugInfoPass::handleGlobalOp(fir::GlobalOp globalOp,
// 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.modules.empty())
- return;
+ if (result.second.procs.empty()) {
+ // Only look for module if this variable is not part of a function.
+ if (result.second.modules.empty())
+ return;
- scope = getOrCreateModuleAttr(result.second.modules[0], fileAttr, scope,
- line - 1, !globalOp.isInitialized());
+ // 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, globalOp.getLoc());
auto gvAttr = mlir::LLVM::DIGlobalVariableAttr::get(
@@ -218,12 +234,6 @@ void AddDebugInfoPass::runOnOperation() {
llvm::dwarf::getLanguage("DW_LANG_Fortran95"), fileAttr, producer,
isOptimized, debugLevel);
- if (debugLevel == mlir::LLVM::DIEmissionKind::Full) {
- // Process 'GlobalOp' only if full debug info is requested.
- for (auto globalOp : module.getOps<fir::GlobalOp>())
- handleGlobalOp(globalOp, fileAttr, cuAttr);
- }
-
module.walk([&](mlir::func::FuncOp funcOp) {
mlir::Location l = funcOp->getLoc();
// If fused location has already been created then nothing to do
@@ -296,6 +306,12 @@ void AddDebugInfoPass::runOnOperation() {
handleDeclareOp(declOp, fileAttr, spAttr, typeGen);
});
});
+ // Process any global which was not processed through DeclareOp.
+ if (debugLevel == mlir::LLVM::DIEmissionKind::Full) {
+ // Process 'GlobalOp' only if full debug info is requested.
+ for (auto globalOp : module.getOps<fir::GlobalOp>())
+ handleGlobalOp(globalOp, fileAttr, cuAttr);
+ }
}
std::unique_ptr<mlir::Pass>
diff --git a/flang/test/Integration/debug-fixed-array-type-2.f90 b/flang/test/Integration/debug-fixed-array-type-2.f90
index 315525442a5bc..b34413458ad8d 100644
--- a/flang/test/Integration/debug-fixed-array-type-2.f90
+++ b/flang/test/Integration/debug-fixed-array-type-2.f90
@@ -23,20 +23,17 @@ function fn1(a1, b1, c1) result (res)
! CHECK-DAG: ![[R1:.*]] = !DISubrange(count: 3, lowerBound: 1)
! CHECK-DAG: ![[SUB1:.*]] = !{![[R1]]}
! CHECK-DAG: ![[D1TY:.*]] = !DICompositeType(tag: DW_TAG_array_type, baseType: ![[INT]], elements: ![[SUB1]])
-! CHECK-DAG: !DILocalVariable(name: "d1"{{.*}}type: ![[D1TY]])
! CHECK-DAG: ![[R21:.*]] = !DISubrange(count: 2, lowerBound: 1)
! CHECK-DAG: ![[R22:.*]] = !DISubrange(count: 5, lowerBound: 1)
! CHECK-DAG: ![[SUB2:.*]] = !{![[R21]], ![[R22]]}
! CHECK-DAG: ![[D2TY:.*]] = !DICompositeType(tag: DW_TAG_array_type, baseType: ![[INT]], elements: ![[SUB2]])
-! CHECK-DAG: !DILocalVariable(name: "d2"{{.*}}type: ![[D2TY]])
! CHECK-DAG: ![[R31:.*]] = !DISubrange(count: 6, lowerBound: 1)
! CHECK-DAG: ![[R32:.*]] = !DISubrange(count: 8, lowerBound: 1)
! CHECK-DAG: ![[R33:.*]] = !DISubrange(count: 7, lowerBound: 1)
! CHECK-DAG: ![[SUB3:.*]] = !{![[R31]], ![[R32]], ![[R33]]}
! CHECK-DAG: ![[D3TY:.*]] = !DICompositeType(tag: DW_TAG_array_type, baseType: ![[REAL]], elements: ![[SUB3]])
-! CHECK-DAG: !DILocalVariable(name: "d3"{{.*}}type: ![[D3TY]])
! CHECK-DAG: !DILocalVariable(name: "a1", arg: 1{{.*}}type: ![[D1TY]])
! CHECK-DAG: !DILocalVariable(name: "b1", arg: 2{{.*}}type: ![[D2TY]])
diff --git a/flang/test/Transforms/debug-local-global-storage-1.fir b/flang/test/Transforms/debug-local-global-storage-1.fir
new file mode 100644
index 0000000000000..d9d8083a14709
--- /dev/null
+++ b/flang/test/Transforms/debug-local-global-storage-1.fir
@@ -0,0 +1,52 @@
+// RUN: fir-opt --add-debug-info --mlir-print-debuginfo %s | FileCheck %s
+
+module attributes {dlti.dl_spec = #dlti.dl_spec<#dlti.dl_entry<i64, dense<64> : vector<2xi64>>, #dlti.dl_entry<!llvm.ptr<272>, dense<64> : vector<4xi64>>, #dlti.dl_entry<!llvm.ptr<271>, dense<32> : vector<4xi64>>, #dlti.dl_entry<!llvm.ptr<270>, dense<32> : vector<4xi64>>, #dlti.dl_entry<f128, dense<128> : vector<2xi64>>, #dlti.dl_entry<f80, dense<128> : vector<2xi64>>, #dlti.dl_entry<i128, dense<128> : vector<2xi64>>, #dlti.dl_entry<i8, dense<8> : vector<2xi64>>, #dlti.dl_entry<!llvm.ptr, dense<64> : vector<4xi64>>, #dlti.dl_entry<i1, dense<8> : vector<2xi64>>, #dlti.dl_entry<f16, dense<16> : vector<2xi64>>, #dlti.dl_entry<f64, dense<64> : vector<2xi64>>, #dlti.dl_entry<i32, dense<32> : vector<2xi64>>, #dlti.dl_entry<i16, dense<16> : vector<2xi64>>, #dlti.dl_entry<"dlti.stack_alignment", 128 : i64>, #dlti.dl_entry<"dlti.endianness", "little">>, fir.defaultkind = "a1c4d8i4l4r4", fir.kindmap = "", llvm.data_layout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-i128:128-f80:128-n8:16:32:64-S128"} {
+ func.func @_QMexamplePmod_sub() {
+ %c2 = arith.constant 2 : index
+ %1 = fir.address_of(@_QMexampleEmod_arr) : !fir.ref<!fir.array<2x2xi32>>
+ %2 = fircg.ext_declare %1(%c2, %c2) {uniq_name = "_QMexampleEmod_arr"} : (!fir.ref<!fir.array<2x2xi32>>, index, index) -> !fir.ref<!fir.array<2x2xi32>> loc(#loc4)
+ %3 = fir.address_of(@_QMexampleFmod_subEss) : !fir.ref<i32>
+ %4 = fircg.ext_declare %3 {uniq_name = "_QMexampleFmod_subEss"} : (!fir.ref<i32>) -> !fir.ref<i32> loc(#loc5)
+ return
+ } loc(#loc6)
+ func.func @_QQmain() attributes {fir.bindc_name = "test"} {
+ %c3 = arith.constant 3 : index
+ %c4 = arith.constant 4 : index
+ %1 = fir.address_of(@_QFEarr) : !fir.ref<!fir.array<3x4xi32>>
+ %2 = fircg.ext_declare %1(%c3, %c4) {uniq_name = "_QFEarr"} : (!fir.ref<!fir.array<3x4xi32>>, index, index) -> !fir.ref<!fir.array<3x4xi32>> loc(#loc2)
+ %3 = fir.address_of(@_QFEs) : !fir.ref<i32>
+ %4 = fircg.ext_declare %3 {uniq_name = "_QFEs"} : (!fir.ref<i32>) -> !fir.ref<i32> loc(#loc3)
+ return
+ } loc(#loc1)
+ fir.global @_QMexampleEmod_arr : !fir.array<2x2xi32> {
+ %0 = fir.zero_bits !fir.array<2x2xi32>
+ fir.has_value %0 : !fir.array<2x2xi32>
+ } loc(#loc4)
+ fir.global internal @_QMexampleFmod_subEss : i32 {
+ %c2_i32 = arith.constant 2 : i32
+ fir.has_value %c2_i32 : i32
+ } loc(#loc5)
+ fir.global internal @_QFEarr : !fir.array<3x4xi32> {
+ %0 = fir.zero_bits !fir.array<3x4xi32>
+ fir.has_value %0 : !fir.array<3x4xi32>
+ } loc(#loc2)
+ fir.global internal @_QFEs : i32 {
+ %c2_i32 = arith.constant 2 : i32
+ fir.has_value %c2_i32 : i32
+ } loc(#loc3)
+}
+#loc1 = loc("test.f90":21:1)
+#loc2 = loc("test.f90":22:1)
+#loc3 = loc("test.f90":23:1)
+#loc4 = loc("test.f90":5:1)
+#loc5 = loc("test.f90":12:1)
+#loc6 = loc("test.f90":10:1)
+
+// 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: #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{{.*}}>
+// CHECK-DAG: #llvm.di_global_variable<scope = #[[MOD]], name = "mod_arr"{{.*}}line = 5{{.*}}>
|
jeanPerier
reviewed
Jul 15, 2024
Comment on lines
91
to
92
| if (auto global = module.lookupSymbol<fir::GlobalOp>(declOp.getUniqName())) { | ||
| handleGlobalOp(global, fileAttr, scopeAttr); |
There was a problem hiding this comment.
module lookups are linear with the number of symbol inside the module.
So this will introduce a "number of variables" * "number of types + number of global + number of function" pseudo quadratic behavior that is noticeable on real applications.
I advise sharing a const mlir::SymbolTable that would be initialized in in runOnOperation only once (ideally, it would be done in pass initialization, but I am not sure how it can be done with the initialize hook from Pass.h that only provide the mlir::Context).
There was a problem hiding this comment.
Thanks. That is very useful thing to know. I have updated the patch.
Comment on lines
149
to
150
| if (mlir::isa<mlir::FusedLoc>(globalOp.getLoc())) | ||
| return; |
There was a problem hiding this comment.
Could you share that into a debugInfoIsAlreadySet(globalOp.getLoc()) static helper so that there it is updated too when #95862 lands.
1. Replace module.lookupSymbol with symbolTable->lookup for performance. 2. Move check that debug info is already processed in a separate function.
sgundapa
pushed a commit
to sgundapa/upstream_effort
that referenced
this pull request
Jul 23, 2024
This PR fixes 2 similar issues. 1. As reported in llvm#97476, flang generated executable has inconsistent behavior regarding values of the local array variables. 2. Variable with save attribute would not show up in debugger. The reason behind is same for both cases. If a local variable has storage which extends beyond function lifetime, the way to represent it in the debug info is through a global variable whose scope is limited to the function. This is what is used for static local variable in C. Previously local array worked in cases they were on stack. But will not show up if they had a global storage. To fix this, if we can get a corresponding `GlobalOp` for a variable while processing `DeclareOp`, we treat it the variable as global with scope set appropriately. A new FIR test is added. A previous Integration test has been adjusted as to not expect local variables for local arrays. With this fix in place, all the issues described in llvm#97476 go away. It also fixes a lot of fails in GDB's fortran testsuite. Fixes llvm#97476.
yuxuanchen1997
pushed a commit
that referenced
this pull request
Jul 25, 2024
Summary: This PR fixes 2 similar issues. 1. As reported in #97476, flang generated executable has inconsistent behavior regarding values of the local array variables. 2. Variable with save attribute would not show up in debugger. The reason behind is same for both cases. If a local variable has storage which extends beyond function lifetime, the way to represent it in the debug info is through a global variable whose scope is limited to the function. This is what is used for static local variable in C. Previously local array worked in cases they were on stack. But will not show up if they had a global storage. To fix this, if we can get a corresponding `GlobalOp` for a variable while processing `DeclareOp`, we treat it the variable as global with scope set appropriately. A new FIR test is added. A previous Integration test has been adjusted as to not expect local variables for local arrays. With this fix in place, all the issues described in #97476 go away. It also fixes a lot of fails in GDB's fortran testsuite. Fixes #97476. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60250985
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes 2 similar issues.
The reason behind is same for both cases. If a local variable has storage which extends beyond function lifetime, the way to represent it in the debug info is through a global variable whose scope is limited to the function. This is what is used for static local variable in C. Previously local array worked in cases they were on stack. But will not show up if they had a global storage.
To fix this, if we can get a corresponding
GlobalOpfor a variable while processingDeclareOp, we treat it the variable as global with scope set appropriately. A new FIR test is added. A previous Integration test has been adjusted as to not expect local variables for local arrays.With this fix in place, all the issues described in #97476 go away. It also fixes a lot of fails in GDB's fortran testsuite.
Fixes #97476.