[flang-commits] [flang] [flang] Fix stack-arrays pass moving alloca across stackrestore scope (PR #184727)
via flang-commits
flang-commits at lists.llvm.org
Sat Mar 7 18:07:43 PST 2026
https://github.com/tmjbios updated https://github.com/llvm/llvm-project/pull/184727
>From d1b3d05e16437403fa9872448575b6673ec635bf Mon Sep 17 00:00:00 2001
From: "Ted M. Johnson" <tedmjohnson at protonmail.com>
Date: Wed, 4 Mar 2026 06:12:59 -0700
Subject: [PATCH 1/2] [flang] Fix stack-arrays pass moving alloca across
stackrestore scope
When a operand is shared between two fir.allocmem ops in different
stacksave/stackrestore scopes, findAllocaInsertionPoint() placed both
allocas at the operand definition site inside the first scope. The
first stackrestore then reclaimed both, leaving the second call with
a dangling pointer.
Add a check for intervening stackrestore ops between the last operand
and the allocmem. If one is found, fall back to the allocmem's own
location, matching the existing bail-out for cross-block operands.
---
.../lib/Optimizer/Transforms/StackArrays.cpp | 19 +++++++++++
flang/test/Transforms/stack-arrays-scope.f90 | 33 +++++++++++++++++++
2 files changed, 52 insertions(+)
create mode 100644 flang/test/Transforms/stack-arrays-scope.f90
diff --git a/flang/lib/Optimizer/Transforms/StackArrays.cpp b/flang/lib/Optimizer/Transforms/StackArrays.cpp
index 860149942a5c1..5875138216930 100644
--- a/flang/lib/Optimizer/Transforms/StackArrays.cpp
+++ b/flang/lib/Optimizer/Transforms/StackArrays.cpp
@@ -668,6 +668,25 @@ InsertionPoint AllocMemConversion::findAllocaInsertionPoint(
// there were value operands to the allocmem so insert after the last one
LLVM_DEBUG(llvm::dbgs()
<< "--Placing after last operand: " << *lastOperand << "\n");
+ // Check we aren't moving across a stackrestore scope boundary.
+ // The last operand may have been defined in an earlier stacksave/
+ // stackrestore scope (e.g. due to CSE merging identical size computations,
+ // or HLFIR-to-FIR lowering reusing a single size value across scopes).
+ // Placing the alloca at the operand's location would put it in the wrong
+ // scope, where it gets reclaimed before the allocmem's actual use.
+ // Fall back to the allocmem's own location, matching the "operand declared
+ // in a different block" bail-out logic above.
+ if (lastOperand->getBlock() == oldAlloc->getBlock()) {
+ for (mlir::Operation *op = lastOperand->getNextNode();
+ op && op != oldAlloc.getOperation(); op = op->getNextNode()) {
+ if (mlir::isa<mlir::LLVM::StackRestoreOp>(op)) {
+ LLVM_DEBUG(llvm::dbgs()
+ << "--stackrestore found between lastOperand and "
+ "allocmem, falling back to allocmem location\n");
+ return checkReturn(oldAlloc.getOperation());
+ }
+ }
+ }
// check we aren't moving out of an omp region
auto lastOpOmpRegion =
lastOperand->getParentOfType<mlir::omp::OutlineableOpenMPOpInterface>();
diff --git a/flang/test/Transforms/stack-arrays-scope.f90 b/flang/test/Transforms/stack-arrays-scope.f90
new file mode 100644
index 0000000000000..ba2536ce91001
--- /dev/null
+++ b/flang/test/Transforms/stack-arrays-scope.f90
@@ -0,0 +1,33 @@
+! Regression test for https://github.com/llvm/llvm-project/issues/178821
+
+! Verify that the stack-arrays pass does not move a stack allocation
+! for a temporary outside its stacksave/stackrestore scope when the
+! size operand is shared across scopes (e.g. due to ConvertHLFIRtoFIR
+! reusing a single size value, or CSE merging duplicates).
+
+! RUN: %flang_fc1 -emit-fir -fstack-arrays %s -o - \
+! RUN: | fir-opt --stack-arrays \
+! RUN: | FileCheck %s
+
+subroutine ss1(a)
+ character*(*) a(1)
+ character*1 t_s
+ call u(t_s() // '?' // a)
+ call u(t_s() // '?' // a)
+end subroutine
+
+! CHECK-LABEL: func.func @_QPss1
+!
+! First scope:
+! CHECK: llvm.intr.stacksave
+! CHECK: fir.alloca !fir.array<1x!fir.char<1,?>>
+! CHECK-SAME: {bindc_name = ".tmp.array"}
+! CHECK: fir.call @_QPu(
+! CHECK: llvm.intr.stackrestore
+!
+! Second scope (the second alloca must be AFTER the first stackrestore):
+! CHECK: llvm.intr.stacksave
+! CHECK: fir.alloca !fir.array<1x!fir.char<1,?>>
+! CHECK-SAME: {bindc_name = ".tmp.array"}
+! CHECK: fir.call @_QPu(
+! CHECK: llvm.intr.stackrestore
>From 36818ac761110d5ab691dae1e268ffe542c895c0 Mon Sep 17 00:00:00 2001
From: "Ted M. Johnson" <tedmjohnson at protonmail.com>
Date: Sat, 7 Mar 2026 18:52:48 -0700
Subject: [PATCH 2/2] [flang] Extend alloca placement check
Address review feedback: remove the same-block restriction on the
stackrestore scope check. Walk from the allocmem up the parent chain
to its ancestor in the last operand's block, then scan between them
for stackrestore ops - skipping regions, which have their own paired
stacksave/stackrestore. Bail out to the allocmem location when the
block CFG is too complex to resolve an ancestor.
---
.../lib/Optimizer/Transforms/StackArrays.cpp | 49 +++++++++++++------
1 file changed, 33 insertions(+), 16 deletions(-)
diff --git a/flang/lib/Optimizer/Transforms/StackArrays.cpp b/flang/lib/Optimizer/Transforms/StackArrays.cpp
index 5875138216930..3f49f60089dbd 100644
--- a/flang/lib/Optimizer/Transforms/StackArrays.cpp
+++ b/flang/lib/Optimizer/Transforms/StackArrays.cpp
@@ -665,33 +665,50 @@ InsertionPoint AllocMemConversion::findAllocaInsertionPoint(
}
if (lastOperand) {
- // there were value operands to the allocmem so insert after the last one
+ // There were value operands to the allocmem so insert after the last one
LLVM_DEBUG(llvm::dbgs()
<< "--Placing after last operand: " << *lastOperand << "\n");
// Check we aren't moving across a stackrestore scope boundary.
// The last operand may have been defined in an earlier stacksave/
- // stackrestore scope (e.g. due to CSE merging identical size computations,
- // or HLFIR-to-FIR lowering reusing a single size value across scopes).
- // Placing the alloca at the operand's location would put it in the wrong
- // scope, where it gets reclaimed before the allocmem's actual use.
- // Fall back to the allocmem's own location, matching the "operand declared
- // in a different block" bail-out logic above.
- if (lastOperand->getBlock() == oldAlloc->getBlock()) {
- for (mlir::Operation *op = lastOperand->getNextNode();
- op && op != oldAlloc.getOperation(); op = op->getNextNode()) {
- if (mlir::isa<mlir::LLVM::StackRestoreOp>(op)) {
- LLVM_DEBUG(llvm::dbgs()
- << "--stackrestore found between lastOperand and "
- "allocmem, falling back to allocmem location\n");
- return checkReturn(oldAlloc.getOperation());
- }
+ // stackrestore scope. Placing the alloca at the operand's location would
+ // put it in the wrong scope, causing it to get reclaimed before its
+ // actual use.
+ //
+ // To start, we find the ancestor of oldAlloc that resides in lastOperand's
+ // block. If oldAlloc is in the same block, this is oldAlloc itself.
+ // If oldAlloc is nested in a region, this is the enclosing op in
+ // lastOperand's block. If no such ancestor exists (e.g. the blocks are
+ // siblings), conservatively fall back to the allocmem's own location.
+ mlir::Operation *target = oldAlloc.getOperation();
+ while (target && target->getBlock() != lastOperand->getBlock())
+ target = target->getParentOp();
+ if (!target) {
+ LLVM_DEBUG(llvm::dbgs()
+ << "--Cannot find allocmem ancestor in lastOperand's "
+ "block, falling back to allocmem location\n");
+ return checkReturn(oldAlloc.getOperation());
+ }
+
+ // Walk from lastOperand to target in the same block, checking for
+ // stackrestore ops. We do not descend into regions of intervening
+ // operations; a stackrestore inside a region is expected to be paired
+ // with its own stacksave and does not affect the enclosing scope.
+ for (mlir::Operation *op = lastOperand->getNextNode(); op && op != target;
+ op = op->getNextNode()) {
+ if (mlir::isa<mlir::LLVM::StackRestoreOp>(op)) {
+ LLVM_DEBUG(llvm::dbgs()
+ << "--stackrestore found between lastOperand and "
+ "allocmem, falling back to allocmem location\n");
+ return checkReturn(oldAlloc.getOperation());
}
}
+
// check we aren't moving out of an omp region
auto lastOpOmpRegion =
lastOperand->getParentOfType<mlir::omp::OutlineableOpenMPOpInterface>();
if (lastOpOmpRegion == oldOmpRegion)
return checkReturn(lastOperand);
+
// Presumably this happened because the operands became ready before the
// start of this openmp region. (lastOpOmpRegion != oldOmpRegion) should
// imply that oldOmpRegion comes after lastOpOmpRegion.
More information about the flang-commits
mailing list