[flang-commits] [flang] [flang][debug] Fix issues with local variables. (PR #98661)
Abid Qadeer via flang-commits
flang-commits at lists.llvm.org
Tue Jul 16 03:09:27 PDT 2024
https://github.com/abidh updated https://github.com/llvm/llvm-project/pull/98661
>From e03ce79a5658cf9b54166a85e3f8655a8678ec58 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Tue, 9 Jul 2024 18:38:49 +0100
Subject: [PATCH 1/2] [flang][debug] Fix issues with local variables.
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 as a global variable.
---
.../lib/Optimizer/Transforms/AddDebugInfo.cpp | 36 +++++++++----
.../Integration/debug-fixed-array-type-2.f90 | 3 --
.../debug-local-global-storage-1.fir | 52 +++++++++++++++++++
3 files changed, 78 insertions(+), 13 deletions(-)
create mode 100644 flang/test/Transforms/debug-local-global-storage-1.fir
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{{.*}}>
>From 5a1d911a16a6dae06d61873407fba391d9612405 Mon Sep 17 00:00:00 2001
From: Abid Qadeer <haqadeer at amd.com>
Date: Tue, 16 Jul 2024 11:08:22 +0100
Subject: [PATCH 2/2] Handle review comments.
1. Replace module.lookupSymbol with symbolTable->lookup for performance.
2. Move check that debug info is already processed in a separate function.
---
.../lib/Optimizer/Transforms/AddDebugInfo.cpp | 32 ++++++++++++-------
1 file changed, 21 insertions(+), 11 deletions(-)
diff --git a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
index 2e7c3007b7980..8bb24fb6c8078 100644
--- a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
+++ b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
@@ -49,7 +49,8 @@ class AddDebugInfoPass : public fir::impl::AddDebugInfoBase<AddDebugInfoPass> {
void handleDeclareOp(fir::cg::XDeclareOp declOp,
mlir::LLVM::DIFileAttr fileAttr,
mlir::LLVM::DIScopeAttr scopeAttr,
- fir::DebugTypeGenerator &typeGen);
+ fir::DebugTypeGenerator &typeGen,
+ mlir::SymbolTable *symbolTable);
public:
AddDebugInfoPass(fir::AddDebugInfoOptions options) : Base(options) {}
@@ -63,7 +64,8 @@ class AddDebugInfoPass : public fir::impl::AddDebugInfoBase<AddDebugInfoPass> {
mlir::LLVM::DIScopeAttr scope, unsigned line, bool decl);
void handleGlobalOp(fir::GlobalOp glocalOp, mlir::LLVM::DIFileAttr fileAttr,
- mlir::LLVM::DIScopeAttr scope);
+ mlir::LLVM::DIScopeAttr scope,
+ mlir::SymbolTable *symbolTable);
};
static uint32_t getLineFromLoc(mlir::Location loc) {
@@ -73,14 +75,20 @@ static uint32_t getLineFromLoc(mlir::Location loc) {
return line;
}
+bool debugInfoIsAlreadySet(mlir::Location loc) {
+ if (mlir::isa<mlir::FusedLoc>(loc))
+ return true;
+ return false;
+}
+
} // namespace
void AddDebugInfoPass::handleDeclareOp(fir::cg::XDeclareOp declOp,
mlir::LLVM::DIFileAttr fileAttr,
mlir::LLVM::DIScopeAttr scopeAttr,
- fir::DebugTypeGenerator &typeGen) {
+ fir::DebugTypeGenerator &typeGen,
+ mlir::SymbolTable *symbolTable) {
mlir::MLIRContext *context = &getContext();
- mlir::ModuleOp module = getOperation();
mlir::OpBuilder builder(context);
auto result = fir::NameUniquer::deconstruct(declOp.getUniqName());
@@ -88,8 +96,8 @@ void AddDebugInfoPass::handleDeclareOp(fir::cg::XDeclareOp declOp,
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);
+ if (auto global = symbolTable->lookup<fir::GlobalOp>(declOp.getUniqName())) {
+ handleGlobalOp(global, fileAttr, scopeAttr, symbolTable);
return;
}
@@ -145,8 +153,9 @@ 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()))
+ mlir::LLVM::DIScopeAttr scope,
+ mlir::SymbolTable *symbolTable) {
+ if (debugInfoIsAlreadySet(globalOp.getLoc()))
return;
mlir::ModuleOp module = getOperation();
mlir::MLIRContext *context = &getContext();
@@ -198,6 +207,7 @@ void AddDebugInfoPass::handleGlobalOp(fir::GlobalOp globalOp,
void AddDebugInfoPass::runOnOperation() {
mlir::ModuleOp module = getOperation();
mlir::MLIRContext *context = &getContext();
+ mlir::SymbolTable symbolTable(module);
mlir::OpBuilder builder(context);
llvm::StringRef fileName;
std::string filePath;
@@ -238,7 +248,7 @@ void AddDebugInfoPass::runOnOperation() {
mlir::Location l = funcOp->getLoc();
// If fused location has already been created then nothing to do
// Otherwise, create a fused location.
- if (mlir::dyn_cast<mlir::FusedLoc>(l))
+ if (debugInfoIsAlreadySet(l))
return;
unsigned int CC = (funcOp.getName() == fir::NameUniquer::doProgramEntry())
@@ -303,14 +313,14 @@ void AddDebugInfoPass::runOnOperation() {
return;
funcOp.walk([&](fir::cg::XDeclareOp declOp) {
- handleDeclareOp(declOp, fileAttr, spAttr, typeGen);
+ handleDeclareOp(declOp, fileAttr, spAttr, typeGen, &symbolTable);
});
});
// 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);
+ handleGlobalOp(globalOp, fileAttr, cuAttr, &symbolTable);
}
}
More information about the flang-commits
mailing list