[flang][debug] Add fake use ops for dynamic array dimension variables#200061
Open
timsmith78 wants to merge 2 commits into
Open
[flang][debug] Add fake use ops for dynamic array dimension variables#200061timsmith78 wants to merge 2 commits into
timsmith78 wants to merge 2 commits into
Conversation
In cases where the upper or lower bounds of a dynamic array are not explicitly referenced in code, flang can optimize away the internal variables that represent these values. This causes missing values to appear in the debugger when examining the dynamic array's type. Adding an llvm.fake.use op for each bound preserves it for use by a debugger, similar to the fix for llvm#185432. Resolves llvm#119474
llvmorg-github-actions
Bot
added
flang
|
@llvm/pr-subscribers-flang-fir-hlfir Author: Tim (timsmith78) ChangesIn cases where the upper or lower bounds of a dynamic array are not explicitly referenced in code, flang can optimize away the internal variables that represent these values. This causes missing values to appear in the debugger when examining the dynamic array's type. Adding an llvm.fake.use op for each bound preserves it for use by a debugger, similar to the fix for #185432. Resolves #119474 Full diff: https://github.com/llvm/llvm-project/pull/200061.diff 4 Files Affected:
diff --git a/flang/include/flang/Optimizer/Transforms/Passes.td b/flang/include/flang/Optimizer/Transforms/Passes.td
index d9072e7aab4f7..8c082fb073451 100644
--- a/flang/include/flang/Optimizer/Transforms/Passes.td
+++ b/flang/include/flang/Optimizer/Transforms/Passes.td
@@ -267,9 +267,9 @@ def AddDebugInfo : Pass<"add-debug-info", "mlir::ModuleOp"> {
Option<"dwarfDebugFlags", "dwarf-debug-flags",
"std::string", /*default=*/"std::string{}",
"Command-line flags to append to DWARF producer">,
- Option<"emitFakeUseForArguments", "emit-fake-use-for-arguments",
+ Option<"emitFakeUseForDebugVars", "emit-fake-use-for-debug-vars",
"bool", /*default=*/"false",
- "Emit fake use for function arguments to extend their lifetime">
+ "Emit fake use for debug variables to extend their lifetime">
];
}
diff --git a/flang/lib/Optimizer/Passes/Pipelines.cpp b/flang/lib/Optimizer/Passes/Pipelines.cpp
index 2b80da308a7d4..8c3528b588a18 100644
--- a/flang/lib/Optimizer/Passes/Pipelines.cpp
+++ b/flang/lib/Optimizer/Passes/Pipelines.cpp
@@ -113,7 +113,7 @@ void addDebugInfoPass(mlir::PassManager &pm,
options.dwarfVersion = config.DwarfVersion;
options.splitDwarfFile = config.SplitDwarfFile;
options.dwarfDebugFlags = config.DwarfDebugFlags;
- options.emitFakeUseForArguments =
+ options.emitFakeUseForDebugVars =
(config.OptLevel == llvm::OptimizationLevel::O0) &&
!disableArgumentFakeUse;
addPassConditionally(pm, disableDebugInfo,
diff --git a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
index e0b570a908521..5aaf66baf9c9d 100644
--- a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
+++ b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
@@ -307,7 +307,7 @@ void AddDebugInfoPass::handleLocalVariable(Op declOp, llvm::StringRef name,
if (dummyScope && declOp.getDummyScope() == dummyScope) {
if (auto argNoOpt = declOp.getDummyArgNo()) {
argNo = *argNoOpt;
- if (emitFakeUseForArguments) {
+ if (emitFakeUseForDebugVars) {
if constexpr (std::is_same_v<Op, fir::cg::XDeclareOp>) {
if (auto funcOp =
declOp->template getParentOfType<mlir::func::FuncOp>()) {
@@ -331,6 +331,60 @@ void AddDebugInfoPass::handleLocalVariable(Op declOp, llvm::StringRef name,
auto tyAttr =
typeGen.convertType(typeToConvert, fileAttr, scopeAttr, typeGenDeclOp);
+ // Check if tyAttr represents a dynamic Fortran array. If so, create
+ // FakeUseOps to preserve the automatic variables that represent the lower
+ // bound, count, and stride.
+ if (emitFakeUseForDebugVars) {
+ if (auto arrayTy =
+ mlir::dyn_cast<mlir::LLVM::DICompositeTypeAttr>(tyAttr)) {
+ if (arrayTy.getTag() == llvm::dwarf::DW_TAG_array_type) {
+ bool isDynamic = llvm::any_of(
+ arrayTy.getElements(), [](mlir::Attribute element) {
+ auto subrange =
+ mlir::dyn_cast<mlir::LLVM::DISubrangeAttr>(element);
+ if (!subrange)
+ return false;
+ return static_cast<bool>(
+ mlir::dyn_cast_if_present<
+ mlir::LLVM::DILocalVariableAttr>(subrange.getCount()) ||
+ mlir::dyn_cast_if_present<
+ mlir::LLVM::DILocalVariableAttr>(
+ subrange.getLowerBound()) ||
+ mlir::dyn_cast_if_present<
+ mlir::LLVM::DILocalVariableAttr>(
+ subrange.getStride()));
+ });
+ if (isDynamic) {
+ if constexpr (std::is_same_v<Op, fir::cg::XDeclareOp>) {
+ if (auto funcOp =
+ declOp->template getParentOfType<mlir::func::FuncOp>()) {
+ if (declOp->getBlock() == &funcOp.getBody().front()) {
+ for (mlir::Block &block : funcOp.getBody()) {
+ if (auto returnOp = mlir::dyn_cast<mlir::func::ReturnOp>(
+ block.getTerminator())) {
+ mlir::OpBuilder::InsertionGuard guard(builder);
+ builder.setInsertionPoint(returnOp);
+ // Preserve count if it is a variable and not a constant.
+ // Count is represented by shape in the declOp.
+ for (auto val : declOp.getShape())
+ if (!fir::getIntIfConstant(val))
+ fir::FakeUseOp::create(builder, declOp.getLoc(), val);
+ // Preserve lower bound if it is a variable and not a
+ // constant.
+ // Lower bound is represented by shift in the declOp.
+ for (auto val : declOp.getShift())
+ if (!fir::getIntIfConstant(val))
+ fir::FakeUseOp::create(builder, declOp.getLoc(), val);
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+
auto localVarAttr = mlir::LLVM::DILocalVariableAttr::get(
context, scopeAttr, mlir::StringAttr::get(context, name), fileAttr,
getLineFromLoc(declOp.getLoc()), argNo, /* alignInBits*/ 0, tyAttr,
diff --git a/flang/test/Transforms/debug-fake-use.fir b/flang/test/Transforms/debug-fake-use.fir
index d86cc9b3afee4..e0aea80adf845 100644
--- a/flang/test/Transforms/debug-fake-use.fir
+++ b/flang/test/Transforms/debug-fake-use.fir
@@ -1,5 +1,5 @@
-// RUN: fir-opt --add-debug-info="emit-fake-use-for-arguments=true" %s | FileCheck %s --check-prefix=FAKE-USE
-// RUN: fir-opt --add-debug-info="emit-fake-use-for-arguments=false" %s | FileCheck %s --check-prefix=NO-FAKE-USE
+// RUN: fir-opt --add-debug-info="emit-fake-use-for-debug-vars=true" %s | FileCheck %s --check-prefix=FAKE-USE
+// RUN: fir-opt --add-debug-info="emit-fake-use-for-debug-vars=false" %s | FileCheck %s --check-prefix=NO-FAKE-USE
// FAKE-USE-LABEL: func.func @test_
// FAKE-USE: %[[UNDEF:.*]] = fir.undefined !fir.dscope
@@ -31,6 +31,19 @@
// NO-FAKE-USE-NOT: fir.fake_use
// NO-FAKE-USE: return
+// FAKE-USE-LABEL: func.func @test_dynamic_array
+// FAKE-USE: fircg.ext_declare %arg0(%[[COUNT:.*]]) origin %[[LB:.*]] dummy_scope
+// FAKE-USE: fir.call @foo() : () -> ()
+// FAKE-USE: fir.fake_use %[[COUNT]]
+// FAKE-USE: fir.fake_use %[[LB]]
+// FAKE-USE: return
+
+// NO-FAKE-USE-LABEL: func.func @test_dynamic_array
+// NO-FAKE-USE: fircg.ext_declare %arg0(%{{.*}}) origin %{{.*}} dummy_scope
+// NO-FAKE-USE: fir.call @foo() : () -> ()
+// NO-FAKE-USE-NOT: fir.fake_use
+// NO-FAKE-USE: return
+
#loc1 = loc("debug-fake-use.f90":1:1)
#loc3 = loc("debug-fake-use.f90":3:14)
#loc4 = loc("debug-fake-use.f90":4:14)
@@ -53,4 +66,20 @@ module attributes {dlti.dl_spec = #dlti.dl_spec<i128 = dense<128> : vector<2xi64
fir.call @foo() : () -> ()
return loc(#loc5)
} loc(#loc1)
+
+ func.func @test_dynamic_array(%arg0: !fir.ref<!fir.array<?xi32>> {fir.bindc_name = "arr"} loc("debug-fake-use.f90":1:1), %arg1: !fir.ref<i32> {fir.bindc_name = "n"} loc("debug-fake-use.f90":1:1), %arg2: !fir.ref<i32> {fir.bindc_name = "lb"} loc("debug-fake-use.f90":1:1)) attributes {fir.internal_name = "_QPtest_dynamic_array"} {
+ %c0 = arith.constant 0 : index
+ %0 = fir.undefined !fir.dscope loc(#loc1)
+ %1 = fircg.ext_declare %arg1 dummy_scope %0 arg 2 {uniq_name = "_QFtest_dynamic_arrayEn"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32> loc(#loc3)
+ %2 = fircg.ext_declare %arg2 dummy_scope %0 arg 3 {uniq_name = "_QFtest_dynamic_arrayElb"} : (!fir.ref<i32>, !fir.dscope) -> !fir.ref<i32> loc(#loc4)
+ %3 = fir.load %1 : !fir.ref<i32>
+ %4 = fir.convert %3 : (i32) -> index
+ %5 = arith.cmpi sgt, %4, %c0 : index
+ %6 = arith.select %5, %4, %c0 : index
+ %7 = fir.load %2 : !fir.ref<i32>
+ %8 = fir.convert %7 : (i32) -> index
+ %9 = fircg.ext_declare %arg0(%6) origin %8 dummy_scope %0 arg 1 {uniq_name = "_QFtest_dynamic_arrayEarr"} : (!fir.ref<!fir.array<?xi32>>, index, index, !fir.dscope) -> !fir.ref<!fir.array<?xi32>> loc(#loc3)
+ fir.call @foo() : () -> ()
+ return loc(#loc5)
+ } loc(#loc1)
} loc(#loc)
|
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions cpp -- flang/lib/Optimizer/Passes/Pipelines.cpp flang/lib/Optimizer/Transforms/AddDebugInfo.cpp --diff_from_common_commit
View the diff from clang-format here.diff --git a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
index 5aaf66baf..338235b29 100644
--- a/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
+++ b/flang/lib/Optimizer/Transforms/AddDebugInfo.cpp
@@ -338,20 +338,18 @@ void AddDebugInfoPass::handleLocalVariable(Op declOp, llvm::StringRef name,
if (auto arrayTy =
mlir::dyn_cast<mlir::LLVM::DICompositeTypeAttr>(tyAttr)) {
if (arrayTy.getTag() == llvm::dwarf::DW_TAG_array_type) {
- bool isDynamic = llvm::any_of(
- arrayTy.getElements(), [](mlir::Attribute element) {
+ bool isDynamic =
+ llvm::any_of(arrayTy.getElements(), [](mlir::Attribute element) {
auto subrange =
mlir::dyn_cast<mlir::LLVM::DISubrangeAttr>(element);
if (!subrange)
return false;
return static_cast<bool>(
- mlir::dyn_cast_if_present<
- mlir::LLVM::DILocalVariableAttr>(subrange.getCount()) ||
- mlir::dyn_cast_if_present<
- mlir::LLVM::DILocalVariableAttr>(
+ mlir::dyn_cast_if_present<mlir::LLVM::DILocalVariableAttr>(
+ subrange.getCount()) ||
+ mlir::dyn_cast_if_present<mlir::LLVM::DILocalVariableAttr>(
subrange.getLowerBound()) ||
- mlir::dyn_cast_if_present<
- mlir::LLVM::DILocalVariableAttr>(
+ mlir::dyn_cast_if_present<mlir::LLVM::DILocalVariableAttr>(
subrange.getStride()));
});
if (isDynamic) {
|
Contributor
Author
|
I accidentally clicked the "Update branch" button thinking it would rebase this branch on top of main, but it performed a merge instead. Should I delete this branch and push a new one that I've rebased? I don't want to mess up the commit history. I'd fix the issue clang-format cited at the same time... |
abidh
reviewed
Jun 1, 2026
| declOp->template getParentOfType<mlir::func::FuncOp>()) { | ||
| if (declOp->getBlock() == &funcOp.getBody().front()) { | ||
| for (mlir::Block &block : funcOp.getBody()) { | ||
| if (auto returnOp = mlir::dyn_cast<mlir::func::ReturnOp>( |
Contributor
There was a problem hiding this comment.
The code to insert fake use ops here largely duplicates the existing code for dummy arguments. I wonder if we can move that code to a helper and then use that in both places. The getIntIfConstant check added here would also be safe to apply in the argument case.
| if (auto arrayTy = | ||
| mlir::dyn_cast<mlir::LLVM::DICompositeTypeAttr>(tyAttr)) { | ||
| if (arrayTy.getTag() == llvm::dwarf::DW_TAG_array_type) { | ||
| bool isDynamic = llvm::any_of( |
Contributor
There was a problem hiding this comment.
Suggested change
| bool isDynamic = llvm::any_of( | |
| bool isDynamic = | |
| llvm::any_of(declOp.getShape(), | |
| [](mlir::Value v) { return !fir::getIntIfConstant(v); }) || | |
| llvm::any_of(declOp.getShift(), | |
| [](mlir::Value v) { return !fir::getIntIfConstant(v); }); |
Do you think something like this will work better? It asks declOp directly if it has non constant shape or size and does not depend on the translated type. Also I don't think we generate an artificial variable for stride so the stride check in your patch was probably redundant.
Contributor
There was a problem hiding this comment.
What is actually the point of isDynamic? It does not look like it is saving much processing time compared to directly going through the declOp below even if there is nothing to do, and it is adding compilation time when there are fake use to emit.
I would suggest removing it.
| %9 = fircg.ext_declare %arg0(%6) origin %8 dummy_scope %0 arg 1 {uniq_name = "_QFtest_dynamic_arrayEarr"} : (!fir.ref<!fir.array<?xi32>>, index, index, !fir.dscope) -> !fir.ref<!fir.array<?xi32>> loc(#loc3) | ||
| fir.call @foo() : () -> () | ||
| return loc(#loc5) | ||
| } loc(#loc1) |
Contributor
There was a problem hiding this comment.
I think we can increase the test coverage a bit.
- Not specific to your case but I think having a function with multiple returns would be a useful test.
- Multi-dimensional array and/or having variable size with constant lower bound.
jeanPerier
reviewed
Jun 2, 2026
Contributor
jeanPerier
left a comment
jeanPerier
left a comment
There was a problem hiding this comment.
Thanks, makes sense to me to emit this.
Should this also be emitted for character length when relevant?
| if (auto arrayTy = | ||
| mlir::dyn_cast<mlir::LLVM::DICompositeTypeAttr>(tyAttr)) { | ||
| if (arrayTy.getTag() == llvm::dwarf::DW_TAG_array_type) { | ||
| bool isDynamic = llvm::any_of( |
Contributor
There was a problem hiding this comment.
What is actually the point of isDynamic? It does not look like it is saving much processing time compared to directly going through the declOp below even if there is nothing to do, and it is adding compilation time when there are fake use to emit.
I would suggest removing it.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
In cases where the upper or lower bounds of a dynamic array are not explicitly referenced in code, flang can optimize away the internal variables that represent these values. This causes missing values to appear in the debugger when examining the dynamic array's type. Adding an llvm.fake.use op for each bound preserves it for use by a debugger, similar to the fix for #185432.
Resolves #119474