[flang-commits] [flang] [flang][PFT-to-MLIR] lazily allocate label-target blocks in branch lowering (PR #210950)
via flang-commits
flang-commits at lists.llvm.org
Tue Jul 21 04:14:49 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-openmp
Author: Kareem Ergawy (ergawy)
<details>
<summary>Changes</summary>
Problem
-------
Under -mmlir --wrap-unstructured-constructs-in-execute-region, branch lowering can hit a null Evaluation::block for a branch target. For example, genMultiwayBranch trips
Bridge.cpp: Assertion `block && "missing multiway branch block"' failed
and genFIR(SelectCaseStmt) trips
Bridge.cpp: Assertion `e->block && "missing CaseStmt block"' failed
The wrap machinery decides an unstructured DO/IF is wrappable and therefore stops propagating its isUnstructured flag to the enclosing construct. The enclosing construct's isUnstructured stays false, so the top-level createEmptyBlocks does not recurse into its body. If that enclosing construct's lowering then takes over the body itself instead of routing through genFIR(DoConstruct)/genFIR(IfConstruct) (where wrapUnstructuredConstruct would have allocated nested blocks), branch targets in the body are never given an .block, and the consumers assert.
Solution
--------
Make the branch consumers responsible for the blocks they need instead of trusting a caller to have populated them. A new helper getOrCreateEvalBlock(Evaluation&) returns eval.block, lazily allocating one in the current region (guarded by mlir::OpBuilder::InsertionGuard so the builder's position is preserved) when it has not been created. genMultiwayBranch, genConstructExitBranch, and genFIR(SelectCaseStmt) route every label / default / case target through it, so any path that skips up-front block allocation resolves itself on demand.
Co-authored-by: Claude Opus 4.7 <noreply@<!-- -->anthropic.com>
---
Full diff: https://github.com/llvm/llvm-project/pull/210950.diff
3 Files Affected:
- (modified) flang/lib/Lower/Bridge.cpp (+35-15)
- (added) flang/test/Lower/OpenMP/wsloop-computed-goto.f90 (+25)
- (added) flang/test/Lower/OpenMP/wsloop-select-case.f90 (+35)
``````````diff
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 7cde3ae492e14..26e3f2b532155 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -1782,10 +1782,29 @@ class FirConverter : public Fortran::lower::AbstractConverter {
return false;
}
+ /// Return \p eval.block, lazily allocating it in the current region if
+ /// it hasn't been created yet. Branch consumers that jump to a
+ /// labeled or exit target must go through this helper rather than
+ /// reading eval.block directly, so they don't rely on a caller having
+ /// pre-allocated the block. Some parent lowerings take over a
+ /// construct's body without invoking the wrap machinery that would
+ /// otherwise create nested blocks up front; in those cases the
+ /// consumer that needs the landing pad creates it on demand.
+ mlir::Block *getOrCreateEvalBlock(Fortran::lower::pft::Evaluation &eval) {
+ if (!eval.block) {
+ // Preserve the builder's insertion point: createBlock moves it to
+ // the newly created block, but we only want to give the eval a
+ // landing pad — the branch instruction still needs to be emitted
+ // at the current position.
+ mlir::OpBuilder::InsertionGuard guard(*builder);
+ eval.block = builder->createBlock(&builder->getRegion());
+ }
+ return eval.block;
+ }
+
/// Generate a branch to \p targetEval after generating on-exit code for
/// any enclosing construct scopes that are exited by taking the branch.
- void
- genConstructExitBranch(const Fortran::lower::pft::Evaluation &targetEval) {
+ void genConstructExitBranch(Fortran::lower::pft::Evaluation &targetEval) {
Fortran::lower::pft::Evaluation *activeAncestor =
getActiveAncestor(targetEval);
for (auto it = activeConstructStack.rbegin(),
@@ -1795,7 +1814,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
break;
it->stmtCtx.finalizeAndKeep();
}
- genBranch(targetEval.block);
+ genBranch(getOrCreateEvalBlock(targetEval));
}
/// A construct contains nested evaluations. Some of these evaluations
@@ -1862,13 +1881,14 @@ class FirConverter : public Fortran::lower::AbstractConverter {
void genMultiwayBranch(mlir::Value selector,
llvm::SmallVector<int64_t> valueList,
llvm::SmallVector<Fortran::parser::Label> labelList,
- const Fortran::lower::pft::Evaluation &defaultEval,
+ Fortran::lower::pft::Evaluation &defaultEval,
mlir::Block *errorBlock = nullptr) {
bool inArithmeticIfContext = valueList.empty();
assert(((inArithmeticIfContext && labelList.size() == 2) ||
(valueList.size() && labelList.size() == valueList.size())) &&
"mismatched multiway branch targets");
- mlir::Block *defaultBlock = errorBlock ? errorBlock : defaultEval.block;
+ mlir::Block *defaultBlock =
+ errorBlock ? errorBlock : getOrCreateEvalBlock(defaultEval);
bool defaultHasExitCode = !errorBlock && hasExitCode(defaultEval);
bool hasAnyExitCode = defaultHasExitCode;
if (!hasAnyExitCode)
@@ -1884,10 +1904,9 @@ class FirConverter : public Fortran::lower::AbstractConverter {
// Generate a SelectOp.
llvm::SmallVector<mlir::Block *> blockList;
for (auto label : labelList) {
- mlir::Block *block =
- label ? evalOfLabel(label).block : defaultEval.block;
- assert(block && "missing multiway branch block");
- blockList.push_back(block);
+ Fortran::lower::pft::Evaluation &targetEval =
+ label ? evalOfLabel(label) : defaultEval;
+ blockList.push_back(getOrCreateEvalBlock(targetEval));
}
blockList.push_back(defaultBlock);
if (valueList[branchCount - 1] == 0) // Swap IO ERR and default blocks.
@@ -1934,7 +1953,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
lastBranch && !defaultHasExitCode
? defaultBlock
: builder->getBlock()->splitBlock(builder->getInsertionPoint());
- const Fortran::lower::pft::Evaluation &targetEval =
+ Fortran::lower::pft::Evaluation &targetEval =
label.value() ? evalOfLabel(label.value()) : defaultEval;
if (hasExitCode(targetEval)) {
mlir::Block *jumpBlock =
@@ -1943,7 +1962,7 @@ class FirConverter : public Fortran::lower::AbstractConverter {
startBlock(jumpBlock);
genConstructExitBranch(targetEval);
} else {
- genConditionalBranch(cond, targetEval.block, nextBlock);
+ genConditionalBranch(cond, getOrCreateEvalBlock(targetEval), nextBlock);
}
if (!lastBranch) {
startBlock(nextBlock);
@@ -4182,7 +4201,8 @@ class FirConverter : public Fortran::lower::AbstractConverter {
llvm::SmallVector<mlir::Attribute> attrList;
llvm::SmallVector<mlir::Value> valueList;
llvm::SmallVector<mlir::Block *> blockList;
- mlir::Block *defaultBlock = parentConstruct->constructExit->block;
+ mlir::Block *defaultBlock =
+ getOrCreateEvalBlock(*parentConstruct->constructExit);
using CaseValue = Fortran::parser::Scalar<Fortran::parser::ConstantExpr>;
auto addValue = [&](const CaseValue &caseValue) {
const Fortran::lower::SomeExpr *expr =
@@ -4200,19 +4220,19 @@ class FirConverter : public Fortran::lower::AbstractConverter {
for (Fortran::lower::pft::Evaluation *e = eval.controlSuccessor; e;
e = e->controlSuccessor) {
const auto &caseStmt = e->getIf<Fortran::parser::CaseStmt>();
- assert(e->block && "missing CaseStmt block");
+ mlir::Block *caseBlock = getOrCreateEvalBlock(*e);
const auto &caseSelector =
std::get<Fortran::parser::CaseSelector>(caseStmt->t);
const auto *caseValueRangeList =
std::get_if<std::list<Fortran::parser::CaseValueRange>>(
&caseSelector.u);
if (!caseValueRangeList) {
- defaultBlock = e->block;
+ defaultBlock = caseBlock;
continue;
}
for (const Fortran::parser::CaseValueRange &caseValueRange :
*caseValueRangeList) {
- blockList.push_back(e->block);
+ blockList.push_back(caseBlock);
if (const auto *caseValue = std::get_if<CaseValue>(&caseValueRange.u)) {
attrList.push_back(fir::PointIntervalAttr::get(context));
addValue(*caseValue);
diff --git a/flang/test/Lower/OpenMP/wsloop-computed-goto.f90 b/flang/test/Lower/OpenMP/wsloop-computed-goto.f90
new file mode 100644
index 0000000000000..289b5d126cf95
--- /dev/null
+++ b/flang/test/Lower/OpenMP/wsloop-computed-goto.f90
@@ -0,0 +1,25 @@
+! RUN: bbc -emit-hlfir -fopenmp --wrap-unstructured-constructs-in-execute-region %s -o - | FileCheck %s
+
+! An unstructured DO whose body has a computed GO TO targeting a label
+! inside the loop. The DO is wrappable, so its isUnstructured no longer
+! propagates to the enclosing OpenMPConstruct — the top-level
+! createEmptyBlocks therefore does not recurse into the DO's body, and the
+! label 17 ContinueStmt has no pre-allocated block when the OMP loop-nest
+! lowering later reaches the ComputedGotoStmt. genMultiwayBranch (and
+! genConstructExitBranch) now allocate label-target blocks on demand.
+subroutine s(ii1)
+ integer ii1
+!$omp do
+ do ii1 = 0, 1
+ go to (17), ii1
+17 continue
+ end do
+end subroutine
+
+! CHECK-LABEL: func.func @_QPs
+! CHECK: omp.wsloop
+! CHECK: omp.loop_nest
+! CHECK: fir.load
+! CHECK: fir.select %{{[0-9]+}} : i32 [1, ^[[TARGET:bb[0-9]+]], unit, ^[[TARGET]]]
+! CHECK: ^[[TARGET]]:
+! CHECK: omp.yield
diff --git a/flang/test/Lower/OpenMP/wsloop-select-case.f90 b/flang/test/Lower/OpenMP/wsloop-select-case.f90
new file mode 100644
index 0000000000000..a1703c9b37c91
--- /dev/null
+++ b/flang/test/Lower/OpenMP/wsloop-select-case.f90
@@ -0,0 +1,35 @@
+! RUN: bbc -emit-hlfir -fopenmp --wrap-unstructured-constructs-in-execute-region %s -o - | FileCheck %s
+
+! An unstructured SELECT CASE inside an OpenMP DO body. With the wrap
+! flag on, the enclosing DoConstruct is wrappable, so its isUnstructured
+! no longer propagates to the OpenMPConstruct — the top-level
+! createEmptyBlocks does not recurse into the DO's body, and the
+! CaseStmt / constructExit blocks the SelectCase lowering wants are not
+! pre-allocated. genFIR(SelectCaseStmt) now goes through
+! getOrCreateEvalBlock, which allocates the missing blocks on demand.
+subroutine s(a, k)
+ integer a, k
+!$omp do
+ do k = 1, 2
+ select case (k)
+ case (1)
+ a = 10
+ case (2)
+ a = 20
+ case default
+ a = 30
+ end select
+ end do
+end subroutine
+
+! CHECK-LABEL: func.func @_QPs
+! CHECK: omp.wsloop
+! CHECK: omp.loop_nest
+! CHECK: fir.load
+! CHECK: fir.select_case %{{[0-9]+}} : i32 [#fir.point, %{{.*}}, ^[[C1:bb[0-9]+]], #fir.point, %{{.*}}, ^[[C2:bb[0-9]+]], unit, ^[[CDFLT:bb[0-9]+]]]
+! CHECK: ^[[C1]]:
+! CHECK: hlfir.assign
+! CHECK: ^[[C2]]:
+! CHECK: hlfir.assign
+! CHECK: ^[[CDFLT]]:
+! CHECK: hlfir.assign
``````````
</details>
https://github.com/llvm/llvm-project/pull/210950
More information about the flang-commits
mailing list