[flang-commits] [flang] [flang][NFC] Split the OpenACC construct lowering into two lanes (PR #227706)
Kareem Ergawy via flang-commits
flang-commits at lists.llvm.org
Wed Sep 30 06:19:18 PDT 2026
https://github.com/ergawy created https://github.com/llvm/llvm-project/pull/227706
genFIR(OpenACCConstruct) decided twice, in three places, whether the construct it lowers is structured, and reassigned the evaluation it works from halfway through: before the descent that evaluation is the construct, after it the loop the directive absorbs. Everything downstream had to know which one it was holding.
Give each form its own function and leave genFIR to choose between them. One lane allocates the exit selector, lowers the evaluations the construct holds, and emits the jump table; the other reads the collapse clauses, descends to the absorbed depth, and lowers what is inside it. The prologue and epilogue are short enough to state in both rather than share.
>From 5006230a6308dab10e87f8bbf35d235cc471fb9d Mon Sep 17 00:00:00 2001
From: ergawy <kareem.ergawy at gmail.com>
Date: Mon, 28 Sep 2026 12:00:29 -0700
Subject: [PATCH] [flang][NFC] Split the OpenACC construct lowering into two
lanes
genFIR(OpenACCConstruct) decided twice, in three places, whether the
construct it lowers is structured, and reassigned the evaluation it works
from halfway through: before the descent that evaluation is the construct,
after it the loop the directive absorbs. Everything downstream had to know
which one it was holding.
Give each form its own function and leave genFIR to choose between them.
One lane allocates the exit selector, lowers the evaluations the construct
holds, and emits the jump table; the other reads the collapse clauses,
descends to the absorbed depth, and lowers what is inside it. The prologue
and epilogue are short enough to state in both rather than share.
---
flang/lib/Lower/Bridge.cpp | 252 ++++++++++++++++++++++---------------
1 file changed, 150 insertions(+), 102 deletions(-)
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 8bd073a479e19..d4c0e547efe88 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -3800,31 +3800,100 @@ class FirConverter : public Fortran::lower::AbstractConverter {
dir.u);
}
- void genFIR(const Fortran::parser::OpenACCConstruct &acc) {
- setCurrentPositionAt(acc);
- Fortran::lower::clearCollapsedDoConstructs();
+ /// Branch out of the ACC region, if the construct is a loop whose lowering
+ /// asked for it. Shared by both lanes.
+ void genAccLoopEarlyExit(const Fortran::parser::OpenACCConstruct &acc,
+ mlir::Value exitCond) {
+ if (!std::holds_alternative<Fortran::parser::OpenACCLoopConstruct>(acc.u) ||
+ !exitCond)
+ return;
+ Fortran::lower::pft::FunctionLikeUnit *funit =
+ getEval().getOwningProcedure();
+ assert(funit && "not inside main program, function or subroutine");
+ mlir::Block *continueBlock =
+ builder->getBlock()->splitBlock(builder->getBlock()->end());
+ mlir::cf::CondBranchOp::create(*builder, toLocation(), exitCond,
+ funit->finalBlock, continueBlock);
+ builder->setInsertionPointToEnd(continueBlock);
+ }
+
+ /// Lower an ACC construct whose branching leaves the region.
+ ///
+ /// The directive absorbs no loop: the loop op carries no bounds and its
+ /// iteration is emitted as explicit control flow inside the region, so every
+ /// evaluation the construct holds is lowered as it stands. A GOTO out of the
+ /// region cannot branch across it either, so it stores an id into a selector
+ /// and leaves through the region's terminator; the jump table after the op
+ /// dispatches on that id.
+ void
+ genUnstructuredAccConstruct(const Fortran::parser::OpenACCConstruct &acc) {
mlir::OpBuilder::InsertPoint insertPt = builder->saveInsertionPoint();
// Cache constructs should not push/pop a scope because they need to update
// the symbol map for subsequent statements in the same loop body.
- bool isCacheConstruct =
+ const bool isCacheConstruct =
std::holds_alternative<Fortran::parser::OpenACCCacheConstruct>(acc.u);
-
if (!isCacheConstruct)
localSymbols.pushScope();
- // Allocate exit selector for GOTO jump table if the construct is
- // unstructured (may contain GOTOs that exit the ACC region).
- bool needsExitSelector = getEval().lowerAsUnstructured();
- if (needsExitSelector) {
- AccRegionExitInfo exitInfo;
- exitInfo.selector =
- builder->createTemporary(toLocation(), builder->getI32Type());
- mlir::Value zero = builder->createIntegerConstant(
- toLocation(), builder->getI32Type(), 0);
- fir::StoreOp::create(*builder, toLocation(), zero, exitInfo.selector);
- accRegionExitStack.push_back(std::move(exitInfo));
+
+ AccRegionExitInfo exitInfo;
+ exitInfo.selector =
+ builder->createTemporary(toLocation(), builder->getI32Type());
+ mlir::Value zero =
+ builder->createIntegerConstant(toLocation(), builder->getI32Type(), 0);
+ fir::StoreOp::create(*builder, toLocation(), zero, exitInfo.selector);
+ accRegionExitStack.push_back(std::move(exitInfo));
+
+ mlir::Value exitCond = genOpenACCConstruct(
+ *this, bridge.getSemanticsContext(), getEval(), acc, localSymbols);
+
+ if (getEval().hasNestedEvaluations())
+ for (Fortran::lower::pft::Evaluation &e :
+ getEval().getNestedEvaluations())
+ genFIR(e);
+
+ if (!isCacheConstruct)
+ localSymbols.popScope();
+ builder->restoreInsertionPoint(insertPt);
+
+ // Dispatch to each recorded GOTO target on its id, falling through to the
+ // code after the region.
+ AccRegionExitInfo regionExits = accRegionExitStack.pop_back_val();
+ if (!regionExits.exits.empty()) {
+ mlir::Location loc = toLocation();
+ mlir::Value sel =
+ fir::LoadOp::create(*builder, loc, regionExits.selector);
+ for (auto &[id, target] : regionExits.exits) {
+ mlir::Value idVal =
+ builder->createIntegerConstant(loc, builder->getI32Type(), id);
+ mlir::Value cmp = mlir::arith::CmpIOp::create(
+ *builder, loc, mlir::arith::CmpIPredicate::eq, sel, idVal);
+ mlir::Block *nextBlock =
+ builder->getBlock()->splitBlock(builder->getBlock()->end());
+ mlir::cf::CondBranchOp::create(*builder, loc, cmp, target, nextBlock);
+ builder->setInsertionPointToEnd(nextBlock);
+ }
}
+ genAccLoopEarlyExit(acc, exitCond);
+ }
+
+ /// Lower an ACC construct whose branching, if any, stays inside it.
+ ///
+ /// The directive owns the loop nest down to its collapse/tile depth: the
+ /// loop op carries the bounds and supplies the induction variables, so only
+ /// what lies inside the absorbed nest is lowered here. Nothing leaves the
+ /// region, so no exit selector is needed.
+ void genStructuredAccConstruct(const Fortran::parser::OpenACCConstruct &acc) {
+ mlir::OpBuilder::InsertPoint insertPt = builder->saveInsertionPoint();
+
+ // Cache constructs should not push/pop a scope because they need to update
+ // the symbol map for subsequent statements in the same loop body.
+ const bool isCacheConstruct =
+ std::holds_alternative<Fortran::parser::OpenACCCacheConstruct>(acc.u);
+ if (!isCacheConstruct)
+ localSymbols.pushScope();
+
mlir::Value exitCond = genOpenACCConstruct(
*this, bridge.getSemanticsContext(), getEval(), acc, localSymbols);
@@ -3833,66 +3902,58 @@ class FirConverter : public Fortran::lower::AbstractConverter {
const Fortran::parser::OpenACCCombinedConstruct *accCombined =
std::get_if<Fortran::parser::OpenACCCombinedConstruct>(&acc.u);
- // TODO: Determining curEval here re-walks the nested evaluations to the
- // collapse/DO CONCURRENT depth that the construct absorbs, mirroring the
- // descent genOpenACCConstruct already performs (visitLoopControl in
- // OpenACC.cpp). That duplication is fragile and couples this code to
- // genOpenACCConstruct's internals. Move the responsibility for determining
- // curEval into genOpenACCConstruct -- where the absorbed evaluations are
- // actually decided -- and return it from there.
Fortran::lower::pft::Evaluation *curEval = &getEval();
- // Determine collapse depth/force and loopCount
bool collapseForce = false;
uint64_t collapseDepth = 1;
uint64_t loopCount = 1;
if (accLoop || accCombined) {
- if (accLoop) {
- const Fortran::parser::AccBeginLoopDirective &beginLoopDir =
- std::get<Fortran::parser::AccBeginLoopDirective>(accLoop->t);
- const Fortran::parser::AccClauseList &clauseList =
- std::get<Fortran::parser::AccClauseList>(beginLoopDir.t);
- loopCount = Fortran::lower::getLoopCountForCollapseAndTile(clauseList);
- std::tie(collapseDepth, collapseForce) =
- Fortran::lower::getCollapseSizeAndForce(clauseList);
- } else if (accCombined) {
- const Fortran::parser::AccBeginCombinedDirective &beginCombinedDir =
- std::get<Fortran::parser::AccBeginCombinedDirective>(
- accCombined->t);
- const Fortran::parser::AccClauseList &clauseList =
- std::get<Fortran::parser::AccClauseList>(beginCombinedDir.t);
- loopCount = Fortran::lower::getLoopCountForCollapseAndTile(clauseList);
- std::tie(collapseDepth, collapseForce) =
- Fortran::lower::getCollapseSizeAndForce(clauseList);
- }
-
- if (curEval->lowerAsStructured()) {
- curEval = &curEval->getFirstNestedEvaluation();
- // A DO CONCURRENT holds all controls in one construct; the per-level
- // descent would overshoot into its body and drop it. collapse(force:
- // ...) explicitly allows prologue/epilogue statements between loop
- // levels and has its own descent below to sink them, so this
- // strict per-level descent -- which does not expect them -- must
- // not run for it; curEval's value here is unused in that case.
- const auto *outerDo = curEval->getIf<Fortran::parser::DoConstruct>();
- if (!collapseForce && !(outerDo && outerDo->IsDoConcurrent()))
- for (uint64_t i = 1; i < loopCount; i++) {
- llvm::SmallVector<Fortran::lower::pft::Evaluation *> skipped;
- Fortran::lower::pft::Evaluation *nextDo =
- Fortran::lower::findNestedDoConstructEvaluation(*curEval,
- &skipped);
- diagnoseSkippedEvaluations(skipped);
- if (!nextDo)
- break;
- curEval = nextDo;
- }
- }
+ const Fortran::parser::AccClauseList &clauseList =
+ accLoop
+ ? std::get<Fortran::parser::AccClauseList>(
+ std::get<Fortran::parser::AccBeginLoopDirective>(accLoop->t)
+ .t)
+ : std::get<Fortran::parser::AccClauseList>(
+ std::get<Fortran::parser::AccBeginCombinedDirective>(
+ accCombined->t)
+ .t);
+ loopCount = Fortran::lower::getLoopCountForCollapseAndTile(clauseList);
+ std::tie(collapseDepth, collapseForce) =
+ Fortran::lower::getCollapseSizeAndForce(clauseList);
+
+ // TODO: This re-walks the nested evaluations to the collapse/DO
+ // CONCURRENT depth that the construct absorbs, mirroring the descent
+ // genOpenACCConstruct already performs (visitLoopControl in
+ // OpenACC.cpp). That duplication is fragile and couples this code to
+ // genOpenACCConstruct's internals. Move the responsibility for
+ // determining the absorbed evaluation into genOpenACCConstruct -- where
+ // it is actually decided -- and return it from there.
+ curEval = &curEval->getFirstNestedEvaluation();
+ // A DO CONCURRENT holds all controls in one construct; the per-level
+ // descent would overshoot into its body and drop it. collapse(force:
+ // ...) explicitly allows prologue/epilogue statements between loop
+ // levels and has its own descent below to sink them, so this strict
+ // per-level descent -- which does not expect them -- must not run for
+ // it; curEval's value here is unused in that case.
+ const auto *outerDo = curEval->getIf<Fortran::parser::DoConstruct>();
+ if (!collapseForce && !(outerDo && outerDo->IsDoConcurrent()))
+ for (uint64_t i = 1; i < loopCount; i++) {
+ llvm::SmallVector<Fortran::lower::pft::Evaluation *> skipped;
+ Fortran::lower::pft::Evaluation *nextDo =
+ Fortran::lower::findNestedDoConstructEvaluation(*curEval,
+ &skipped);
+ diagnoseSkippedEvaluations(skipped);
+ if (!nextDo)
+ break;
+ curEval = nextDo;
+ }
}
- const bool isStructured = curEval && curEval->lowerAsStructured();
- if (isStructured && collapseForce && collapseDepth > 1) {
- // force: collect prologue/epilogue for the first collapseDepth nested
- // loops and sink them into the innermost loop body at that depth
+ // collapse(force: ...) allows statements between the loop levels the
+ // directive absorbs. They cannot stay where they are, since those levels
+ // no longer exist as loops, so each level's leading and trailing
+ // statements are sunk into the innermost absorbed body.
+ auto genCollapseForceBody = [&]() {
llvm::SmallVector<Fortran::lower::pft::Evaluation *> prologue, epilogue;
Fortran::lower::pft::Evaluation *parent = &getEval();
Fortran::lower::pft::Evaluation *innermostLoopEval = nullptr;
@@ -3941,46 +4002,33 @@ class FirConverter : public Fortran::lower::AbstractConverter {
genFIR(e);
sink(epilogue);
- } else {
- // Normal lowering
- if (curEval->hasNestedEvaluations())
- for (Fortran::lower::pft::Evaluation &e :
- curEval->getNestedEvaluations())
- genFIR(e);
+ };
+
+ if (collapseForce && collapseDepth > 1) {
+ genCollapseForceBody();
+ } else if (curEval->hasNestedEvaluations()) {
+ for (Fortran::lower::pft::Evaluation &e : curEval->getNestedEvaluations())
+ genFIR(e);
}
+
if (!isCacheConstruct)
localSymbols.popScope();
builder->restoreInsertionPoint(insertPt);
- // Generate jump table for GOTO exits from the ACC region.
- if (needsExitSelector) {
- auto exitInfo = accRegionExitStack.pop_back_val();
- if (!exitInfo.exits.empty()) {
- mlir::Location loc = toLocation();
- mlir::Value sel = fir::LoadOp::create(*builder, loc, exitInfo.selector);
- for (auto &[id, target] : exitInfo.exits) {
- mlir::Value idVal =
- builder->createIntegerConstant(loc, builder->getI32Type(), id);
- mlir::Value cmp = mlir::arith::CmpIOp::create(
- *builder, loc, mlir::arith::CmpIPredicate::eq, sel, idVal);
- mlir::Block *nextBlock =
- builder->getBlock()->splitBlock(builder->getBlock()->end());
- mlir::cf::CondBranchOp::create(*builder, loc, cmp, target, nextBlock);
- builder->setInsertionPointToEnd(nextBlock);
- }
- }
- }
+ genAccLoopEarlyExit(acc, exitCond);
+ }
- if (accLoop && exitCond) {
- Fortran::lower::pft::FunctionLikeUnit *funit =
- getEval().getOwningProcedure();
- assert(funit && "not inside main program, function or subroutine");
- mlir::Block *continueBlock =
- builder->getBlock()->splitBlock(builder->getBlock()->end());
- mlir::cf::CondBranchOp::create(*builder, toLocation(), exitCond,
- funit->finalBlock, continueBlock);
- builder->setInsertionPointToEnd(continueBlock);
- }
+ void genFIR(const Fortran::parser::OpenACCConstruct &acc) {
+ setCurrentPositionAt(acc);
+ Fortran::lower::clearCollapsedDoConstructs();
+
+ // The two lanes differ in what the directive owns. Branching that leaves
+ // the region keeps the construct unstructured: it absorbs no loop, its
+ // loop op carries no bounds, and a GOTO out of it needs a jump table.
+ if (getEval().lowerAsUnstructured())
+ genUnstructuredAccConstruct(acc);
+ else
+ genStructuredAccConstruct(acc);
}
void genFIR(const Fortran::parser::OpenACCDeclarativeConstruct &accDecl) {
More information about the flang-commits
mailing list