[flang-commits] [flang] Revert "[flang][OpenMP] Implement collapse for imperfectly nested loops" (PR #208456)
via flang-commits
flang-commits at lists.llvm.org
Thu Jul 9 06:23:06 PDT 2026
llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-semantics
Author: Caroline Newcombe (cenewcombe)
<details>
<summary>Changes</summary>
Reverts llvm/llvm-project#<!-- -->202435
---
Patch is 68.01 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/208456.diff
19 Files Affected:
- (modified) flang/include/flang/Semantics/openmp-utils.h (-6)
- (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+4-183)
- (modified) flang/lib/Lower/OpenMP/Utils.cpp (+6-27)
- (modified) flang/lib/Semantics/check-omp-loop.cpp (-10)
- (modified) flang/lib/Semantics/openmp-utils.cpp (+3-84)
- (removed) flang/test/Lower/OpenMP/collapse-imperfect-nest.f90 (-648)
- (removed) flang/test/Lower/OpenMP/collapse-loop-transform.f90 (-21)
- (modified) flang/test/Semantics/OpenMP/do-collapse.f90 (+4-4)
- (modified) flang/test/Semantics/OpenMP/do-concurrent-collapse-60.f90 (+2-2)
- (modified) flang/test/Semantics/OpenMP/do-concurrent-collapse.f90 (+2-2)
- (modified) flang/test/Semantics/OpenMP/do08.f90 (+15)
- (modified) flang/test/Semantics/OpenMP/do10.f90 (+1-1)
- (modified) flang/test/Semantics/OpenMP/do13.f90 (+5-5)
- (modified) flang/test/Semantics/OpenMP/do15.f90 (+9)
- (modified) flang/test/Semantics/OpenMP/do16.f90 (+6)
- (modified) flang/test/Semantics/OpenMP/do22.f90 (+5-122)
- (removed) flang/test/Semantics/OpenMP/doacross-nesting-omp60.f90 (-148)
- (removed) flang/test/Semantics/OpenMP/ordered-nesting-omp50.f90 (-50)
- (removed) flang/test/Semantics/OpenMP/ordered-nesting-omp51.f90 (-36)
``````````diff
diff --git a/flang/include/flang/Semantics/openmp-utils.h b/flang/include/flang/Semantics/openmp-utils.h
index 0764d6e2b05af..6fc65c5e71765 100644
--- a/flang/include/flang/Semantics/openmp-utils.h
+++ b/flang/include/flang/Semantics/openmp-utils.h
@@ -360,12 +360,6 @@ std::optional<std::vector<const parser::DoConstruct *>> CollectAffectedDoLoops(
const parser::OpenMPLoopConstruct &x, unsigned version,
SemanticsContext *semaCtx = nullptr);
-/// Returns whether the loop nest associated with `x` is a doacross loop nest,
-/// i.e. its body contains an `ordered` directive carrying a doacross
-/// dependence (the `doacross` clause, or the pre-5.2 `depend(sink/source)`
-/// equivalent) that binds to `x`. Such a nest must be perfectly nested.
-bool IsDoacrossAffected(const parser::OpenMPLoopConstruct &x);
-
struct LoopSequence {
LoopSequence(const parser::ExecutionPartConstruct &root, unsigned version,
bool allowAllLoops = false, SemanticsContext *semaCtx = nullptr);
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 7939e0db655e4..9c6352b73a585 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -36,7 +36,6 @@
#include "flang/Optimizer/Builder/BoxValue.h"
#include "flang/Optimizer/Builder/FIRBuilder.h"
#include "flang/Optimizer/Builder/Todo.h"
-#include "flang/Optimizer/Dialect/FIROpsSupport.h"
#include "flang/Optimizer/Dialect/FIRType.h"
#include "flang/Optimizer/HLFIR/HLFIROps.h"
#include "flang/Parser/openmp-utils.h"
@@ -933,167 +932,6 @@ static void genNestedEvaluations(lower::AbstractConverter &converter,
converter.genEval(e);
}
-static mlir::Operation *setLoopVar(lower::AbstractConverter &converter,
- mlir::Location loc, mlir::Value indexVal,
- const semantics::Symbol *sym);
-
-/// Emit the body of a collapsed loop nest, including any intervening code
-/// from imperfect nesting at intermediate levels (CLN relaxation, applied
-/// retroactively for all OMP versions).
-///
-/// Because omp.loop_nest places its entire body at the innermost nesting
-/// level, intervening code must be guarded so that it only executes on the
-/// iterations where the corresponding inner induction variables are at their
-/// initial (for intervening code before nested loop) or final (for intervening
-/// code after nested loop) values.
-///
-/// \param [in] converter - PFT to MLIR conversion interface.
-/// \param [in] outerEval - the evaluation containing the outermost loop
-/// (typically the OpenMP construct evaluation).
-/// \param [in] collapseValue - number of loops being collapsed (>= 1).
-static void genCollapsedLoopNestBody(lower::AbstractConverter &converter,
- lower::pft::Evaluation &outerEval,
- int collapseValue) {
- assert(collapseValue >= 1);
- if (collapseValue == 1) {
- genNestedEvaluations(converter, outerEval, /*collapseValue=*/1);
- return;
- }
-
- fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
- const mlir::Location loc = converter.getCurrentLocation();
-
- // Get the enclosing omp.loop_nest to access induction variables and bounds.
- auto loopNestOp = mlir::dyn_cast<mlir::omp::LoopNestOp>(
- firOpBuilder.getInsertionBlock()->getParentOp());
- assert(loopNestOp && "expected to be inside omp.loop_nest");
-
- // Collect before/after evaluations at each intermediate level.
- struct LevelInfo {
- llvm::SmallVector<lower::pft::Evaluation *> before;
- llvm::SmallVector<lower::pft::Evaluation *> after;
- };
- llvm::SmallVector<LevelInfo> levels;
-
- // DO-variable symbol of each collapsed level (index 0 = outermost). Used to
- // restore an inner loop's variable to its Fortran terminal value before
- // emitting "after" intervening code (see below).
- llvm::SmallVector<const semantics::Symbol *> ivSyms;
-
- lower::pft::Evaluation *curEval = &outerEval;
- for (int i = 0; i < collapseValue - 1; ++i) {
- lower::pft::Evaluation *doEval = getNestedDoConstruct(*curEval);
- const semantics::Symbol *ivSym = getIterationVariableSymbol(*doEval);
- assert(ivSym && "expected iteration variable on collapsed DO loop");
- ivSyms.push_back(ivSym);
- LevelInfo level;
- bool pastDo = false;
- for (lower::pft::Evaluation &e : doEval->getNestedEvaluations()) {
- if (e.getIf<parser::NonLabelDoStmt>() || e.getIf<parser::EndDoStmt>())
- continue;
- // Semantics guarantees the only DoConstruct here is the next associated
- // loop (non-associated DO loops are rejected as intervening code).
- if (e.getIf<parser::DoConstruct>()) {
- pastDo = true;
- continue;
- }
- if (!pastDo)
- level.before.push_back(&e);
- else
- level.after.push_back(&e);
- }
- levels.push_back(std::move(level));
- curEval = doEval;
- }
- // DO-variable symbol of the innermost collapsed loop must be restored
- // inside enclosing "after" regions.
- const semantics::Symbol *innermostIvSym =
- getIterationVariableSymbol(*getNestedDoConstruct(*curEval));
- assert(innermostIvSym && "expected iteration variable on collapsed DO loop");
- ivSyms.push_back(innermostIvSym);
-
- // Build a guard condition: all induction variables from
- // startLevel..endLevel-1 equal their respective bound values.
- // For "before" guards (useLowerBound=true), compare iv == lb (first iter).
- // For "after" guards (useLowerBound=false), compare iv == last_iv.
- const auto lbs = loopNestOp.getLoopLowerBounds();
- const auto ubs = loopNestOp.getLoopUpperBounds();
- const auto steps = loopNestOp.getLoopSteps();
-
- // Last value the induction variable at \p lvl actually takes:
- // lb + ((ub - lb) / step) * step. For unit steps this is exactly ub.
- auto computeLastIV = [&](const int lvl) -> mlir::Value {
- const auto constStep = fir::getIntIfConstant(steps[lvl]);
- if (constStep && (*constStep == 1 || *constStep == -1))
- return ubs[lvl];
- const mlir::Value lb = lbs[lvl];
- const mlir::Value ub = ubs[lvl];
- const mlir::Value step = steps[lvl];
- const mlir::Value range =
- mlir::arith::SubIOp::create(firOpBuilder, loc, ub, lb);
- const mlir::Value tripMinus1 =
- mlir::arith::DivSIOp::create(firOpBuilder, loc, range, step);
- const mlir::Value lastOffset =
- mlir::arith::MulIOp::create(firOpBuilder, loc, tripMinus1, step);
- return mlir::arith::AddIOp::create(firOpBuilder, loc, lb, lastOffset);
- };
-
- auto buildGuard = [&](const int startLevel, const int endLevel,
- const bool useLowerBound) -> mlir::Value {
- mlir::Value cond;
- for (int lvl = startLevel; lvl < endLevel; ++lvl) {
- const mlir::Value iv = loopNestOp.getRegion().getArgument(lvl);
- const mlir::Value target = useLowerBound ? lbs[lvl] : computeLastIV(lvl);
- const mlir::Value cmp = mlir::arith::CmpIOp::create(
- firOpBuilder, loc, mlir::arith::CmpIPredicate::eq, iv, target);
- if (!cond)
- cond = cmp;
- else
- cond = mlir::arith::AndIOp::create(firOpBuilder, loc, cond, cmp);
- }
- return cond;
- };
-
- // Emit "before" code at each level, guarded by inner IVs == lower bounds.
- for (int i = 0; i < static_cast<int>(levels.size()); ++i) {
- if (levels[i].before.empty())
- continue;
- const mlir::Value guard =
- buildGuard(i + 1, collapseValue, /*useLowerBound=*/true);
- auto ifOp = fir::IfOp::create(firOpBuilder, loc, guard, /*else*/ false);
- firOpBuilder.setInsertionPointToStart(&ifOp.getThenRegion().front());
- for (auto *e : levels[i].before)
- converter.genEval(*e);
- firOpBuilder.setInsertionPointAfter(ifOp);
- }
-
- // Emit innermost loop body.
- genNestedEvaluations(converter, *curEval, /*collapseValue=*/1);
-
- // Emit "after" code at each level (innermost first), guarded by
- // inner IVs == last iteration values (accounts for non-unit steps).
- for (int i = static_cast<int>(levels.size()) - 1; i >= 0; --i) {
- if (levels[i].after.empty())
- continue;
- const mlir::Value guard =
- buildGuard(i + 1, collapseValue, /*useLowerBound=*/false);
- auto ifOp = fir::IfOp::create(firOpBuilder, loc, guard, /*else*/ false);
- firOpBuilder.setInsertionPointToStart(&ifOp.getThenRegion().front());
- // A normally-terminated Fortran DO loop leaves its variable one step past
- // the last executed value, but the flattened nest leaves each at its last
- // executed value. Restore the terminal value before running "after" code
- // that may read it.
- for (int lvl = i + 1; lvl < collapseValue; ++lvl) {
- const mlir::Value terminal = mlir::arith::AddIOp::create(
- firOpBuilder, loc, computeLastIV(lvl), steps[lvl]);
- setLoopVar(converter, loc, terminal, ivSyms[lvl]);
- }
- for (auto *e : levels[i].after)
- converter.genEval(*e);
- firOpBuilder.setInsertionPointAfter(ifOp);
- }
-}
-
static fir::GlobalOp globalInitialization(lower::AbstractConverter &converter,
fir::FirOpBuilder &firOpBuilder,
const semantics::Symbol &sym,
@@ -1785,13 +1623,6 @@ struct OpWithBodyGenInfo {
return *this;
}
- OpWithBodyGenInfo &setCollapseInfo(int value,
- lower::pft::Evaluation &outerEval) {
- collapseValue = value;
- outerCollapseEval = &outerEval;
- return *this;
- }
-
/// [inout] converter to use for the clauses.
lower::AbstractConverter &converter;
/// [in] Symbol table
@@ -1820,10 +1651,6 @@ struct OpWithBodyGenInfo {
bool genSkeletonOnly = false;
/// [in] enables handling of privatized variable unless set to `false`.
bool privatize = true;
- /// [in] if set, outermost evaluation and collapse depth for emitting
- /// intervening code from imperfect collapsed loop nests.
- lower::pft::Evaluation *outerCollapseEval = nullptr;
- int collapseValue = 0;
};
static mlir::Value getReductionOverrideValue(fir::FirOpBuilder &builder,
@@ -2123,11 +1950,7 @@ static void createBodyOfOp(mlir::Operation &op, const OpWithBodyGenInfo &info,
firOpBuilder.setInsertionPointToEnd(&op.getRegion(0).back());
auto *temp = lower::genOpenMPTerminator(firOpBuilder, &op, info.loc);
firOpBuilder.setInsertionPointAfter(marker);
- if (info.outerCollapseEval)
- genCollapsedLoopNestBody(info.converter, *info.outerCollapseEval,
- info.collapseValue);
- else
- genNestedEvaluations(info.converter, info.eval);
+ genNestedEvaluations(info.converter, info.eval);
temp->erase();
}
@@ -2983,8 +2806,7 @@ genLoopNestOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
directive)
.setClauses(&item->clauses)
.setDataSharingProcessor(&dsp)
- .setGenRegionEntryCb(ivCallback)
- .setCollapseInfo(nestValue, eval),
+ .setGenRegionEntryCb(ivCallback),
queue, item, clauseOps);
converter.overrideExprValues(oldOverrides);
return loopNestOp;
@@ -3076,7 +2898,7 @@ static void genCanonicalLoopNest(
// Step 1: Loop prologues
// Computing the trip count must happen before entering the outermost loop
lower::pft::Evaluation *innermostEval = nestedEval;
- for (std::size_t i = 0; i < ivs.size(); ++i) {
+ for ([[maybe_unused]] auto iv : ivs) {
if (innermostEval->getIf<parser::DoConstruct>()->IsDoConcurrent()) {
// OpenMP specifies DO CONCURRENT only with the `!omp loop` construct.
// Will need to add special cases for this combination.
@@ -3158,8 +2980,7 @@ static void genCanonicalLoopNest(
mlir::Value cli = newcli.getResult();
clis.push_back(cli);
- if (i + 1 < ivs.size())
- innermostEval = getNestedDoConstruct(*innermostEval);
+ innermostEval = &*std::next(innermostEval->getNestedEvaluations().begin());
}
// Step 2: Create nested canoncial loops
diff --git a/flang/lib/Lower/OpenMP/Utils.cpp b/flang/lib/Lower/OpenMP/Utils.cpp
index fd6e8ee16c3e2..2ecc168f207dc 100644
--- a/flang/lib/Lower/OpenMP/Utils.cpp
+++ b/flang/lib/Lower/OpenMP/Utils.cpp
@@ -698,12 +698,7 @@ static void processTileSizesFromOpenMPConstruct(
}
}
-// Searches the nested evaluations of \p eval for the next DO construct of a
-// loop nest, returning nullptr when there is none left to descend into. This
-// can happen when COLLAPSE counts loops that a transforming construct such as
-// TILE generates from the source DO loops. getNestedDoConstruct wraps this for
-// callers that require a DO construct and asserts when none is found.
-static pft::Evaluation *tryGetNestedDoConstruct(pft::Evaluation &eval) {
+pft::Evaluation *getNestedDoConstruct(pft::Evaluation &eval) {
for (pft::Evaluation &nested : eval.getNestedEvaluations()) {
// In an OpenMPConstruct there can be compiler directives:
// 1 <<OpenMPConstruct>>
@@ -723,18 +718,10 @@ static pft::Evaluation *tryGetNestedDoConstruct(pft::Evaluation &eval) {
// Loop transformations can introduce nested OpenMP
// constructs between the directive and the actual do-loop nest.
if (nested.getIf<parser::OpenMPConstruct>())
- return tryGetNestedDoConstruct(nested);
- // Skip valid intervening code in imperfect loop nests
- continue;
+ return getNestedDoConstruct(nested);
+ assert(false && "Unexpected construct in the nested evaluations");
}
- return nullptr;
-}
-
-pft::Evaluation *getNestedDoConstruct(pft::Evaluation &eval) {
- pft::Evaluation *doConstruct{tryGetNestedDoConstruct(eval)};
- if (!doConstruct)
- llvm_unreachable("Expected do loop to be in the nested evaluations");
- return doConstruct;
+ llvm_unreachable("Expected do loop to be in the nested evaluations");
}
/// Populates the sizes vector with values if the given OpenMPConstruct
@@ -859,16 +846,8 @@ void collectLoopRelatedInfo(
iv.push_back(bounds->Name().thing.symbol);
loopVarTypeSize = std::max(
loopVarTypeSize, bounds->Name().thing.symbol->GetUltimate().size());
- if (--collapseValue) {
- doConstructEval = tryGetNestedDoConstruct(*doConstructEval);
- // No DO construct left to descend into: COLLAPSE (or a tile SIZES list)
- // counts loops generated by a nested loop-transforming construct. Not
- // yet supported in lowering.
- if (!doConstructEval)
- TODO(currentLocation,
- "Collapsing a loop nest that contains a loop-transforming "
- "construct");
- }
+ if (--collapseValue)
+ doConstructEval = getNestedDoConstruct(*doConstructEval);
} while (collapseValue > 0);
convertLoopBounds(converter, currentLocation, result, loopVarTypeSize);
diff --git a/flang/lib/Semantics/check-omp-loop.cpp b/flang/lib/Semantics/check-omp-loop.cpp
index 7023b44253ea9..38b77157e33b8 100644
--- a/flang/lib/Semantics/check-omp-loop.cpp
+++ b/flang/lib/Semantics/check-omp-loop.cpp
@@ -326,16 +326,6 @@ void OmpStructureChecker::CheckNestedConstruct(
// Check requirements on nest depth.
auto [needDepth, needPerfect]{
GetAffectedNestDepthWithReason(beginSpec, version)};
-
- // Perfect nesting for doacross loop nests is handled differently across
- // versions. Only in 6.0+ is the requirement keyed off the body
- // actually containing an ORDERED directive with a doacross dependence
- // rather than the ORDERED clause, so the body scan applies only to those
- // later versions.
- if (!needPerfect && version > 52 && IsDoacrossAffected(x)) {
- needPerfect = true;
- }
-
auto &[haveSema, havePerf]{sequence.depth()};
auto haveDepth{needPerfect ? havePerf : haveSema};
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index 96f774fe34a7b..3503eb0e3a004 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -1296,33 +1296,11 @@ std::pair<WithReason<int64_t>, bool> GetAffectedNestDepthWithReason(
ocount = std::nullopt;
oreason = Reason();
}
- bool hasOrdered{parser::omp::FindClause(
- spec, llvm::omp::Clause::OMPC_ordered) != nullptr};
- // Perfect-nesting requirement for the ORDERED clause, by version:
- //
- // 5.0: Any ORDERED clause makes the associated loops a doacross loop
- // nest that must be perfectly nested, whether or not the clause
- // has an argument.
- // 5.1/5.2: Only an ORDERED clause *with* an argument requires perfect
- // nesting; a bare ORDERED clause does not.
- // 6.0: Perfect nesting is required only when the body actually
- // contains an ORDERED directive with a doacross dependence;
- // that is detected separately by the caller via
- // IsDoacrossAffected, so ORDERED(n) alone does not force
- // perfect nesting here.
if (ccount < ocount) {
- return {{ocount.value_or(1), std::move(oreason)}, version <= 52};
+ // `ocount` cannot be std::nullopt here (C++ std guarantee).
+ return {{ocount.value_or(1), std::move(oreason)}, true};
}
- // Same rule as above when COLLAPSE drives the depth: ORDERED(n) requires a
- // perfect nest through 5.2, while > 5.2 defers to IsDoacrossAffected. In
- // 5.0, an ORDERED clause without argument also requires perfect nesting.
- // The CLN relaxation for COLLAPSE is applied retroactively for all
- // versions.
- bool needPerfect{false};
- if (version <= 52) {
- needPerfect = ocount.has_value() || (version == 50 && hasOrdered);
- }
- return {{ccount.value_or(1), std::move(creason)}, needPerfect};
+ return {{ccount.value_or(1), std::move(creason)}, true};
}
if (IsLoopTransforming(dir)) {
@@ -1540,65 +1518,6 @@ std::optional<int64_t> GetMinimumSequenceCount(
return GetMinimumSequenceCount(std::nullopt, std::nullopt);
}
-namespace {
-/// Visitor that detects an `ordered` directive carrying a doacross dependence
-/// (the `doacross` clause, or the pre-5.2 `depend(sink/source)` equivalent)
-/// that binds to the loop construct being checked. Prunes nested constructs
-/// that start their own associated loop nest, but descends into
-/// loop-transforming constructs (e.g. tile, unroll), whose generated loops
-/// extend the current nest.
-struct DoacrossFinder {
- bool found{false};
- bool inOrdered{false};
- template <typename T> bool Pre(const T &) { return !found; }
- template <typename T> void Post(const T &) {}
-
- // Prune nested constructs that start their own associated loop nest; a
- // doacross inside them binds there, not here. Loop-transforming constructs
- // are the exception: their generated loops extend the current nest, so a
- // doacross inside one still binds to the construct being checked.
- bool Pre(const parser::OmpBlockConstruct &) { return false; }
- bool Pre(const parser::OpenMPLoopConstruct &x) {
- if (IsLoopTransforming(x.BeginDir().DirId())) {
- return !found;
- }
- return false;
- }
-
- bool Pre(const parser::OpenMPSimpleStandaloneConstruct &x) {
- inOrdered = x.v.DirId() == llvm::omp::Directive::OMPD_ordered;
- return !found;
- }
- void Post(const parser::OpenMPSimpleStandaloneConstruct &) {
- inOrdered = false;
- }
-
- bool Pre(const parser::OmpDoacross &) {
- if (inOrdered) {
- found = true;
- }
- return false;
- }
-};
-
-static bool ContainsOrderedDoacross(const parser::Block &block) {
- DoacrossFinder finder;
- parser::Walk(block, finder);
- return finder.found;
-}
-} // namespace
-
-bool IsDoacrossAffected(const parser::OpenMPLoopConstruct &x) {
- // A loop nest is doacross-affected when it has an `ordered` clause and a
- // stand-alone `ordered` construct carrying a doacross dependence is closely
- // nested in its body.
- const parser::OmpDirectiveSpecification &spec{x.BeginDir()};
- if (!parser::omp::FindClause(spec, llvm::omp::Clause::OMPC_ordered)) {
- return false;
- }
- return ContainsOrderedDoacross(std::get<parser::Block>(x.t));
-}
-
/// Collect the DO loops that are affected directly by the given loop
/// transformation. Not all DO loops nested in the associated nes...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/208456
More information about the flang-commits
mailing list