[flang-commits] [flang] [flang][OpenMP] Implement checks for rectangular loops (PR #190648)
Krzysztof Parzyszek via flang-commits
flang-commits at lists.llvm.org
Tue Apr 7 06:52:59 PDT 2026
https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/190648
>From 87a4c8d9384b4454a88c6d75d0cab148b5fde8d9 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 6 Apr 2026 10:18:06 -0500
Subject: [PATCH 1/8] [flang][OpenMP] Use OmpDirectiveSpecifications in helper
functions
This will make them more reusable, for example when processing APPLY
clause in the future.
Issue: https://github.com/llvm/llvm-project/issues/185287
---
flang/include/flang/Semantics/openmp-utils.h | 6 +-
flang/lib/Semantics/openmp-utils.cpp | 99 +++++++++-----------
2 files changed, 48 insertions(+), 57 deletions(-)
diff --git a/flang/include/flang/Semantics/openmp-utils.h b/flang/include/flang/Semantics/openmp-utils.h
index c29cc4e3a33e2..6a41ada9067c5 100644
--- a/flang/include/flang/Semantics/openmp-utils.h
+++ b/flang/include/flang/Semantics/openmp-utils.h
@@ -113,6 +113,9 @@ bool IsPointerAssignment(const evaluate::Assignment &x);
MaybeExpr MakeEvaluateExpr(const parser::OmpStylizedInstance &inp);
+bool IsLoopTransforming(llvm::omp::Directive dir);
+bool IsFullUnroll(const parser::OmpDirectiveSpecification &spec);
+
/// A representation of a "because" message.
struct Reason {
Reason() = default;
@@ -157,9 +160,6 @@ WithReason<int64_t> GetNumArgumentsWithReason(
const parser::OmpDirectiveSpecification &spec, llvm::omp::Clause clauseId,
unsigned version);
-bool IsLoopTransforming(llvm::omp::Directive dir);
-bool IsFullUnroll(const parser::OpenMPLoopConstruct &x);
-
// Return the depth of the affected nests:
// {affected-depth, reason, must-be-perfect-nest}.
std::pair<WithReason<int64_t>, bool> GetAffectedNestDepthWithReason(
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index e9548816fc665..33707f9d0d95b 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -554,6 +554,44 @@ MaybeExpr MakeEvaluateExpr(const parser::OmpStylizedInstance &inp) {
instance.u);
}
+bool IsLoopTransforming(llvm::omp::Directive dir) {
+ switch (dir) {
+ // TODO case llvm::omp::Directive::OMPD_flatten:
+ case llvm::omp::Directive::OMPD_fuse:
+ case llvm::omp::Directive::OMPD_interchange:
+ case llvm::omp::Directive::OMPD_nothing:
+ case llvm::omp::Directive::OMPD_reverse:
+ // TODO case llvm::omp::Directive::OMPD_split:
+ case llvm::omp::Directive::OMPD_stripe:
+ case llvm::omp::Directive::OMPD_tile:
+ case llvm::omp::Directive::OMPD_unroll:
+ return true;
+ default:
+ return false;
+ }
+}
+
+bool IsFullUnroll(const parser::OmpDirectiveSpecification &spec) {
+ if (spec.DirId() == llvm::omp::Directive::OMPD_unroll) {
+ return !parser::omp::FindClause(spec, llvm::omp::Clause::OMPC_partial);
+ }
+ return false;
+}
+
+static bool IsTransformableLoop(const parser::OmpDirectiveSpecification &spec) {
+ return !IsFullUnroll(spec) && IsLoopTransforming(spec.DirId());
+}
+
+static bool IsTransformableLoop(const parser::ExecutionPartConstruct &epc) {
+ if (auto *loop{parser::Unwrap<parser::DoConstruct>(epc)}) {
+ return loop->IsDoNormal();
+ }
+ if (auto *omp{parser::Unwrap<parser::OpenMPLoopConstruct>(epc)}) {
+ return IsTransformableLoop(omp->BeginDir());
+ }
+ return false;
+}
+
static const auto MsgNotValidAffectedLoop{
"%s is not a valid affected loop"_because_en_US};
static const auto MsgClauseAbsentAssume{
@@ -638,33 +676,6 @@ WithReason<int64_t> GetNumArgumentsWithReason(
return {};
}
-bool IsLoopTransforming(llvm::omp::Directive dir) {
- switch (dir) {
- // TODO case llvm::omp::Directive::OMPD_flatten:
- case llvm::omp::Directive::OMPD_fuse:
- case llvm::omp::Directive::OMPD_interchange:
- case llvm::omp::Directive::OMPD_nothing:
- case llvm::omp::Directive::OMPD_reverse:
- // TODO case llvm::omp::Directive::OMPD_split:
- case llvm::omp::Directive::OMPD_stripe:
- case llvm::omp::Directive::OMPD_tile:
- case llvm::omp::Directive::OMPD_unroll:
- return true;
- default:
- return false;
- }
-}
-
-bool IsFullUnroll(const parser::OpenMPLoopConstruct &x) {
- const parser::OmpDirectiveSpecification &beginSpec{x.BeginDir()};
-
- if (beginSpec.DirName().v == llvm::omp::Directive::OMPD_unroll) {
- return parser::omp::FindClause(
- beginSpec, llvm::omp::Clause::OMPC_partial) == nullptr;
- }
- return false;
-}
-
namespace {
// Helper class to check if a given evaluate::Expr is an array expression.
// This does not check any proper subexpressions of the expression (except
@@ -812,27 +823,6 @@ bool IsTransparentInterveningCode(const parser::ExecutionPartConstruct &x) {
parser::Unwrap<parser::ContinueStmt>(x);
}
-bool IsTransformableLoop(const parser::DoConstruct &loop) {
- return loop.IsDoNormal();
-}
-
-bool IsTransformableLoop(const parser::OpenMPLoopConstruct &omp) {
- if (IsFullUnroll(omp)) {
- return false;
- }
- return IsLoopTransforming(omp.BeginDir().DirId());
-}
-
-bool IsTransformableLoop(const parser::ExecutionPartConstruct &epc) {
- if (auto *loop{parser::Unwrap<parser::DoConstruct>(epc)}) {
- return IsTransformableLoop(*loop);
- }
- if (auto *omp{parser::Unwrap<parser::OpenMPLoopConstruct>(epc)}) {
- return IsTransformableLoop(*omp);
- }
- return false;
-}
-
template <typename T,
typename = std::enable_if_t<std::is_arithmetic_v<llvm::remove_cvref_t<T>>>>
WithReason<T> operator+(const WithReason<T> &a, const WithReason<T> &b) {
@@ -1057,7 +1047,7 @@ LoopSequence::LoopSequence(
std::unique_ptr<LoopSequence::Construct> LoopSequence::createConstructEntry(
const parser::ExecutionPartConstruct &code) {
if (auto *loop{parser::Unwrap<parser::DoConstruct>(code)}) {
- if (allowAllLoops_ || IsTransformableLoop(*loop)) {
+ if (allowAllLoops_ || IsTransformableLoop(code)) {
auto &body{std::get<parser::Block>(loop->t)};
return std::make_unique<Construct>(body, &code);
}
@@ -1126,7 +1116,7 @@ WithReason<int64_t> LoopSequence::calculateLength() const {
}
// TODO: Handle split, apply.
- if (IsFullUnroll(omp)) {
+ if (IsFullUnroll(beginSpec)) {
return {};
}
@@ -1250,10 +1240,10 @@ LoopSequence::Depth LoopSequence::calculateDepths() const {
auto &omp{DEREF(parser::Unwrap<parser::OpenMPLoopConstruct>(*entry_->owner))};
const parser::OmpDirectiveSpecification &beginSpec{omp.BeginDir()};
llvm::omp::Directive dir{beginSpec.DirId()};
- bool isFullUnroll{IsFullUnroll(omp)};
+ bool isFullUnroll{IsFullUnroll(beginSpec)};
// Check full unroll separately.
- if (!isFullUnroll && !IsTransformableLoop(omp)) {
+ if (!isFullUnroll && !IsTransformableLoop(beginSpec)) {
Reason reason;
reason.Say(beginSpec.DirName().source,
"This construct is not a DO-loop or a loop-nest-generating construct"_because_en_US);
@@ -1368,10 +1358,11 @@ static Reason WhyNotWellFormed(
Reason reason;
parser::CharBlock source{*parser::GetSource(badCode)};
if (auto *omp{parser::Unwrap<parser::OpenMPLoopConstruct>(badCode)}) {
- if (IsFullUnroll(*omp)) {
+ const parser::OmpDirectiveSpecification &beginSpec{omp->BeginDir()};
+ if (IsFullUnroll(beginSpec)) {
reason.Say(source, MsgConstructDoesNotResult, "Fully unrolled loop",
isSequence ? "a loop nest or a loop sequence" : "a loop nest");
- } else if (!IsLoopTransforming(omp->BeginDir().DirId())) {
+ } else if (!IsLoopTransforming(beginSpec.DirId())) {
reason.Say(source,
"Only loop-transforming constructs are allowed inside loop constructs"_because_en_US);
}
>From 22d5d5a866b82eff70457312c9b6cf65afb5226c Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 6 Apr 2026 11:27:49 -0500
Subject: [PATCH 2/8] [flang][OpenMP] Fix subtle bug in
GetAffectedNestDepthWithReason
For constructs that allow COLLAPSE or ORDERED clauses, the function
would return an empty value for the affected depth if none of these
clauses were actually present. What should happen is that the return
value should be 1 without a specific reason.
This bug was not detectable with any source program, since the empty
value caused depth checks to be skipped. Detecting the problem would
require a loop nest with a lower depth than needed that the bug would
cause not to be diagnosed. Since the correct value was 1, such a loop
would need to have a depth of 0 and such a nest cannot be constructed.
Issue: https://github.com/llvm/llvm-project/issues/185287
---
flang/lib/Semantics/openmp-utils.cpp | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index 33707f9d0d95b..1d9de49e6cd8e 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -850,17 +850,24 @@ std::pair<WithReason<int64_t>, bool> GetAffectedNestDepthWithReason(
dir, llvm::omp::Clause::OMPC_ordered, version)};
if (allowsCollapse || allowsOrdered) {
- auto [count, reason]{GetArgumentValueWithReason(
+ auto [ccount, creason]{GetArgumentValueWithReason(
spec, llvm::omp::Clause::OMPC_collapse, version)};
- auto [vo, ro]{GetArgumentValueWithReason(
+ auto [ocount, oreason]{GetArgumentValueWithReason(
spec, llvm::omp::Clause::OMPC_ordered, version)};
- if (vo) {
- if (!count || *count < *vo) {
- count = vo;
- reason = std::move(ro);
- }
+ // Ignore invalid arguments.
+ if (ccount <= 0) {
+ ccount = std::nullopt;
+ creason = Reason();
+ }
+ if (ocount <= 0) {
+ ocount = std::nullopt;
+ oreason = Reason();
+ }
+ if (ccount < ocount) {
+ // `ocount` cannot be std::nullopt here (C++ std guarantee).
+ return {{ocount.value_or(1), std::move(oreason)}, true};
}
- return {{count, std::move(reason)}, true};
+ return {{ccount.value_or(1), std::move(creason)}, true};
}
if (IsLoopTransforming(dir)) {
>From aebdfeb0048c992b93c5be77496bb1e1f677c873 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Thu, 26 Mar 2026 11:42:11 -0500
Subject: [PATCH 3/8] [flang][OpenMP] Introduce WithSource<T> to couple T with
source location
The need for that has already happened once with SourcedActionStmt, and
will happen again in upcoming PRs.
Issue: https://github.com/llvm/llvm-project/issues/185287
---
flang/include/flang/Semantics/openmp-utils.h | 26 ++++++++--
flang/lib/Semantics/check-omp-atomic.cpp | 52 ++++++++++----------
flang/lib/Semantics/check-omp-structure.cpp | 6 +--
3 files changed, 50 insertions(+), 34 deletions(-)
diff --git a/flang/include/flang/Semantics/openmp-utils.h b/flang/include/flang/Semantics/openmp-utils.h
index 6a41ada9067c5..3084750bf73de 100644
--- a/flang/include/flang/Semantics/openmp-utils.h
+++ b/flang/include/flang/Semantics/openmp-utils.h
@@ -53,14 +53,30 @@ template <typename T> T &&AsRvalue(T &&t) { return std::move(t); }
const Scope &GetScopingUnit(const Scope &scope);
const Scope &GetProgramUnit(const Scope &scope);
+template <typename T> struct WithSource {
+ template <//
+ typename U = std::remove_reference_t<T>,
+ typename = std::enable_if_t<std::is_default_constructible_v<U>>>
+ WithSource() : value(), source() {}
+ WithSource(const WithSource<T> &) = default;
+ WithSource(WithSource<T> &&) = default;
+ WithSource(const T &t, parser::CharBlock s) : value(t), source(s) {}
+ WithSource(T &&t, parser::CharBlock s) : value(std::move(t)), source(s) {}
+ WithSource &operator=(const WithSource<T> &) = default;
+ WithSource &operator=(WithSource<T> &&) = default;
+
+ using value_type = T;
+ T value;
+ parser::CharBlock source;
+};
+
// There is no consistent way to get the source of an ActionStmt, but there
// is "source" in Statement<T>. This structure keeps the ActionStmt with the
// extracted source for further use.
-struct SourcedActionStmt {
- const parser::ActionStmt *stmt{nullptr};
- parser::CharBlock source;
-
- operator bool() const { return stmt != nullptr; }
+struct SourcedActionStmt : public WithSource<const parser::ActionStmt *> {
+ using WithSource<value_type>::WithSource;
+ value_type stmt() const { return value; }
+ operator bool() const { return stmt() != nullptr; }
};
SourcedActionStmt GetActionStmt(const parser::ExecutionPartConstruct *x);
diff --git a/flang/lib/Semantics/check-omp-atomic.cpp b/flang/lib/Semantics/check-omp-atomic.cpp
index effa3bf68063d..f722de8d8dc46 100644
--- a/flang/lib/Semantics/check-omp-atomic.cpp
+++ b/flang/lib/Semantics/check-omp-atomic.cpp
@@ -298,7 +298,7 @@ static std::optional<AnalyzedCondStmt> AnalyzeConditionalStmt(
if (auto &&action{GetActionStmt(x)}) {
if (auto *ifs{std::get_if<common::Indirection<parser::IfStmt>>(
- &action.stmt->u)}) {
+ &action.stmt()->u)}) {
const parser::IfStmt &s{ifs->value()};
auto &&maybeCond{
getFromLogical(std::get<parser::ScalarLogicalExpr>(s.t))};
@@ -335,13 +335,13 @@ static std::optional<AnalyzedCondStmt> AnalyzeConditionalStmt(
AnalyzedCondStmt result{std::move(*maybeCond), stmt.source,
GetActionStmt(std::get<parser::Block>(s.t)),
GetActionStmt(std::get<parser::Block>(maybeElse->t))};
- if (result.ift.stmt && result.iff.stmt) {
+ if (result.ift.stmt() && result.iff.stmt()) {
return result;
}
} else {
AnalyzedCondStmt result{std::move(*maybeCond), stmt.source,
GetActionStmt(std::get<parser::Block>(s.t)), SourcedActionStmt{}};
- if (result.ift.stmt) {
+ if (result.ift.stmt()) {
return result;
}
}
@@ -655,10 +655,10 @@ OmpStructureChecker::CheckUpdateCapture(
SourcedActionStmt act1{GetActionStmt(ec1)};
SourcedActionStmt act2{GetActionStmt(ec2)};
- auto maybeAssign1{GetEvaluateAssignment(act1.stmt)};
- auto maybeAssign2{GetEvaluateAssignment(act2.stmt)};
+ auto maybeAssign1{GetEvaluateAssignment(act1.stmt())};
+ auto maybeAssign2{GetEvaluateAssignment(act2.stmt())};
if (!maybeAssign1 || !maybeAssign2) {
- if (!IsAssignment(act1.stmt) || !IsAssignment(act2.stmt)) {
+ if (!IsAssignment(act1.stmt()) || !IsAssignment(act2.stmt())) {
context_.Say(source,
"ATOMIC UPDATE operation with CAPTURE should contain two assignments"_err_en_US);
}
@@ -1108,9 +1108,9 @@ void OmpStructureChecker::CheckAtomicConditionalUpdateStmt(
// - cond: associated(x, e) ift: x => expr iff: -
// The if-true statement must be present, and must be an assignment.
- auto maybeAssign{GetEvaluateAssignment(update.ift.stmt)};
+ auto maybeAssign{GetEvaluateAssignment(update.ift.stmt())};
if (!maybeAssign) {
- if (update.ift.stmt && !IsAssignment(update.ift.stmt)) {
+ if (update.ift.stmt() && !IsAssignment(update.ift.stmt())) {
context_.Say(update.ift.source,
"In ATOMIC UPDATE COMPARE the update statement should be an assignment"_err_en_US);
} else {
@@ -1138,7 +1138,7 @@ void OmpStructureChecker::CheckAtomicUpdateOnly(
parser::CharBlock source) {
if (body.size() == 1) {
SourcedActionStmt action{GetActionStmt(&body.front())};
- if (auto maybeUpdate{GetEvaluateAssignment(action.stmt)}) {
+ if (auto maybeUpdate{GetEvaluateAssignment(action.stmt())}) {
const SomeExpr &atom{maybeUpdate->lhs};
auto maybeAssign{
CheckAtomicUpdateAssignment(*maybeUpdate, action.source)};
@@ -1148,7 +1148,7 @@ void OmpStructureChecker::CheckAtomicUpdateOnly(
x.analysis = AtomicAnalysis(atom)
.addOp0(Analysis::Update, updateAssign)
.addOp1(Analysis::None);
- } else if (!IsAssignment(action.stmt)) {
+ } else if (!IsAssignment(action.stmt())) {
context_.Say(
source, "ATOMIC UPDATE operation should be an assignment"_err_en_US);
}
@@ -1197,7 +1197,7 @@ void OmpStructureChecker::CheckAtomicConditionalUpdate(
if (SourcedActionStmt action{GetActionStmt(cst)}) {
// The "condition" statement must be `r = cond`.
- if (auto maybeCond{GetEvaluateAssignment(action.stmt)}) {
+ if (auto maybeCond{GetEvaluateAssignment(action.stmt())}) {
if (maybeCond->lhs != update.cond) {
context_.Say(update.source,
"In ATOMIC UPDATE COMPARE the conditional statement must use %s as the condition"_err_en_US,
@@ -1213,7 +1213,7 @@ void OmpStructureChecker::CheckAtomicConditionalUpdate(
}
}
- evaluate::Assignment assign{*GetEvaluateAssignment(update.ift.stmt)};
+ evaluate::Assignment assign{*GetEvaluateAssignment(update.ift.stmt())};
CheckAtomicConditionalUpdateStmt(update, source);
if (IsCheckForAssociated(update.cond)) {
@@ -1254,8 +1254,8 @@ void OmpStructureChecker::CheckAtomicUpdateCapture(
SourcedActionStmt cact{GetActionStmt(cec)};
// The "dereferences" of std::optional are guaranteed to be valid after
// CheckUpdateCapture.
- evaluate::Assignment update{*GetEvaluateAssignment(uact.stmt)};
- evaluate::Assignment capture{*GetEvaluateAssignment(cact.stmt)};
+ evaluate::Assignment update{*GetEvaluateAssignment(uact.stmt())};
+ evaluate::Assignment capture{*GetEvaluateAssignment(cact.stmt())};
const SomeExpr &atom{update.lhs};
@@ -1280,7 +1280,7 @@ void OmpStructureChecker::CheckAtomicUpdateCapture(
return;
}
- if (GetActionStmt(&body.front()).stmt == uact.stmt) {
+ if (GetActionStmt(&body.front()).stmt() == uact.stmt()) {
x.analysis = AtomicAnalysis(atom)
.addOp0(action, updateAssign)
.addOp1(Analysis::Read, capture);
@@ -1316,7 +1316,7 @@ void OmpStructureChecker::CheckAtomicConditionalUpdateCapture(
auto classifyNonUpdate{[&](const SourcedActionStmt &action) {
// The non-update statement is either "r = cond" or the capture.
- if (auto maybeAssign{GetEvaluateAssignment(action.stmt)}) {
+ if (auto maybeAssign{GetEvaluateAssignment(action.stmt())}) {
if (update.cond == maybeAssign->lhs) {
// If this is "r = cond; if (r) ...", then update the condition.
update.cond = maybeAssign->rhs;
@@ -1384,10 +1384,10 @@ void OmpStructureChecker::CheckAtomicConditionalUpdateCapture(
}
// The update must have a form `x = d` or `x => d`.
- if (auto maybeWrite{GetEvaluateAssignment(update.ift.stmt)}) {
+ if (auto maybeWrite{GetEvaluateAssignment(update.ift.stmt())}) {
const SomeExpr &atom{maybeWrite->lhs};
CheckAtomicWriteAssignment(*maybeWrite, update.ift.source);
- if (auto maybeCapture{GetEvaluateAssignment(capture.stmt)}) {
+ if (auto maybeCapture{GetEvaluateAssignment(capture.stmt())}) {
CheckAtomicCaptureAssignment(*maybeCapture, atom, capture.source);
if (IsPointerAssignment(*maybeWrite) !=
@@ -1397,14 +1397,14 @@ void OmpStructureChecker::CheckAtomicConditionalUpdateCapture(
return;
}
} else {
- if (!IsAssignment(capture.stmt)) {
+ if (!IsAssignment(capture.stmt())) {
context_.Say(capture.source,
"In ATOMIC UPDATE COMPARE CAPTURE the capture statement should be an assignment"_err_en_US);
}
return;
}
} else {
- if (!IsAssignment(update.ift.stmt)) {
+ if (!IsAssignment(update.ift.stmt())) {
context_.Say(update.ift.source,
"In ATOMIC UPDATE COMPARE CAPTURE the update statement should be an assignment"_err_en_US);
}
@@ -1430,8 +1430,8 @@ void OmpStructureChecker::CheckAtomicConditionalUpdateCapture(
int updateWhen{!condUnused ? Analysis::IfTrue : 0};
int captureWhen{!captureAlways ? Analysis::IfFalse : 0};
- evaluate::Assignment updAssign{*GetEvaluateAssignment(update.ift.stmt)};
- evaluate::Assignment capAssign{*GetEvaluateAssignment(capture.stmt)};
+ evaluate::Assignment updAssign{*GetEvaluateAssignment(update.ift.stmt())};
+ evaluate::Assignment capAssign{*GetEvaluateAssignment(capture.stmt())};
const SomeExpr &atom{updAssign.lhs};
if (captureFirst) {
@@ -1465,7 +1465,7 @@ void OmpStructureChecker::CheckAtomicRead(
if (body.size() == 1) {
SourcedActionStmt action{GetActionStmt(&body.front())};
- if (auto maybeRead{GetEvaluateAssignment(action.stmt)}) {
+ if (auto maybeRead{GetEvaluateAssignment(action.stmt())}) {
CheckAtomicReadAssignment(*maybeRead, action.source);
if (auto maybe{GetConvertInput(maybeRead->rhs)}) {
@@ -1475,7 +1475,7 @@ void OmpStructureChecker::CheckAtomicRead(
.addOp0(Analysis::Read, maybeRead)
.addOp1(Analysis::None);
}
- } else if (!IsAssignment(action.stmt)) {
+ } else if (!IsAssignment(action.stmt())) {
context_.Say(
x.source, "ATOMIC READ operation should be an assignment"_err_en_US);
}
@@ -1500,7 +1500,7 @@ void OmpStructureChecker::CheckAtomicWrite(
if (body.size() == 1) {
SourcedActionStmt action{GetActionStmt(&body.front())};
- if (auto maybeWrite{GetEvaluateAssignment(action.stmt)}) {
+ if (auto maybeWrite{GetEvaluateAssignment(action.stmt())}) {
const SomeExpr &atom{maybeWrite->lhs};
CheckAtomicWriteAssignment(*maybeWrite, action.source);
@@ -1508,7 +1508,7 @@ void OmpStructureChecker::CheckAtomicWrite(
x.analysis = AtomicAnalysis(atom)
.addOp0(Analysis::Write, maybeWrite)
.addOp1(Analysis::None);
- } else if (!IsAssignment(action.stmt)) {
+ } else if (!IsAssignment(action.stmt())) {
context_.Say(
x.source, "ATOMIC WRITE operation should be an assignment"_err_en_US);
}
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 9b9da227bdef2..479f1763ee319 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1754,7 +1754,7 @@ static std::pair<const parser::AllocateStmt *, parser::CharBlock>
getAllocateStmtAndSource(const parser::ExecutionPartConstruct *epc) {
if (SourcedActionStmt as{GetActionStmt(epc)}) {
using IndirectionAllocateStmt = common::Indirection<parser::AllocateStmt>;
- if (auto *indirect{std::get_if<IndirectionAllocateStmt>(&as.stmt->u)}) {
+ if (auto *indirect{std::get_if<IndirectionAllocateStmt>(&as.stmt()->u)}) {
return {&indirect->value(), as.source};
}
}
@@ -2258,11 +2258,11 @@ void OmpStructureChecker::Enter(const parser::OpenMPDispatchConstruct &x) {
bool passChecks{false};
omp::SourcedActionStmt action{omp::GetActionStmt(block)};
if (const auto *assignStmt{
- parser::Unwrap<parser::AssignmentStmt>(*action.stmt)}) {
+ parser::Unwrap<parser::AssignmentStmt>(*action.stmt())}) {
if (parser::Unwrap<parser::FunctionReference>(assignStmt->t)) {
passChecks = true;
}
- } else if (parser::Unwrap<parser::CallStmt>(*action.stmt)) {
+ } else if (parser::Unwrap<parser::CallStmt>(*action.stmt())) {
passChecks = true;
}
>From f0f7311f5139126b87060b0deb002a8de5345188 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 24 Mar 2026 12:11:40 -0500
Subject: [PATCH 4/8] [flang][OpenMP] Introduce `semantics::omp::LoopControl`
This structure will contain the symbol for the iteration variable,
the lower and upper bounds, and the increment if present in the
form of evaluate::Expr. A source construct (such as DO CONCURRENT)
may have several iteration variables, each with its own bounds
and increment, represented by a list of LoopControl structures.
For loop-transformation constructs that produce additional loops
the current code returns an empty list of LoopControls, but it
may be extended in the future to represent the yet-nonexistent
loops for more accurate semantic analysis.
The code introduced in this PR is not executed yet, but will be
used in an upcoming PR.
Issue: https://github.com/llvm/llvm-project/issues/185287
---
flang/include/flang/Semantics/openmp-utils.h | 23 ++++++++
flang/lib/Semantics/check-omp-loop.cpp | 1 -
flang/lib/Semantics/check-omp-structure.h | 4 ++
flang/lib/Semantics/openmp-utils.cpp | 62 ++++++++++++++++++--
4 files changed, 84 insertions(+), 6 deletions(-)
diff --git a/flang/include/flang/Semantics/openmp-utils.h b/flang/include/flang/Semantics/openmp-utils.h
index 3084750bf73de..f6aebaf8ffdfc 100644
--- a/flang/include/flang/Semantics/openmp-utils.h
+++ b/flang/include/flang/Semantics/openmp-utils.h
@@ -24,6 +24,7 @@
#include "llvm/ADT/ArrayRef.h"
+#include <memory>
#include <optional>
#include <string>
#include <tuple>
@@ -124,6 +125,7 @@ std::optional<bool> IsContiguous(
std::vector<SomeExpr> GetTopLevelDesignators(const SomeExpr &expr);
const SomeExpr *HasStorageOverlap(
const SomeExpr &base, llvm::ArrayRef<SomeExpr> exprs);
+
bool IsAssignment(const parser::ActionStmt *x);
bool IsPointerAssignment(const evaluate::Assignment &x);
@@ -132,6 +134,21 @@ MaybeExpr MakeEvaluateExpr(const parser::OmpStylizedInstance &inp);
bool IsLoopTransforming(llvm::omp::Directive dir);
bool IsFullUnroll(const parser::OmpDirectiveSpecification &spec);
+struct LoopControl {
+ LoopControl(LoopControl &&x) = default;
+ LoopControl(const LoopControl &x) = default;
+ LoopControl(const parser::LoopControl::Bounds &x);
+ LoopControl(const parser::ConcurrentControl &x);
+
+ const Symbol *iv{nullptr};
+ WithSource<MaybeExpr> lbound, ubound, step;
+
+private:
+ static WithSource<MaybeExpr> fromParserExpr(const parser::Expr &x);
+};
+
+std::vector<LoopControl> GetLoopControls(const parser::DoConstruct &x);
+
/// A representation of a "because" message.
struct Reason {
Reason() = default;
@@ -222,6 +239,12 @@ struct LoopSequence {
WithReason<bool> isWellFormedSequence() const;
WithReason<bool> isWellFormedNest() const;
+ std::vector<LoopControl> getLoopControls() const;
+ // Check if this loop's bounds are invariant in each of the `outer`
+ // constructs.
+ WithReason<bool> isRectangular(
+ const std::vector<const LoopSequence *> &outer) const;
+
private:
using Construct = ExecutionPartIterator::Construct;
diff --git a/flang/lib/Semantics/check-omp-loop.cpp b/flang/lib/Semantics/check-omp-loop.cpp
index 5bbf7bcd627ec..48d0684320e1c 100644
--- a/flang/lib/Semantics/check-omp-loop.cpp
+++ b/flang/lib/Semantics/check-omp-loop.cpp
@@ -267,7 +267,6 @@ void OmpStructureChecker::CheckNestedConstruct(
}
LoopSequence sequence(body, version, true);
-
auto assoc{llvm::omp::getDirectiveAssociation(dir)};
auto needRange{GetAffectedLoopRangeWithReason(beginSpec, version)};
auto haveLength{sequence.length()};
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index dc84c9d9ae9d8..a27b16561d3f7 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -49,6 +49,10 @@ static const OmpDirectiveSet noWaitClauseNotAllowedSet{
namespace Fortran::semantics {
struct AnalyzedCondStmt;
+namespace omp {
+struct LoopSequence;
+}
+
// Mapping from 'Symbol' to 'Source' to keep track of the variables
// used in multiple clauses
using SymbolSourceMap = std::multimap<const Symbol *, parser::CharBlock>;
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index 1d9de49e6cd8e..bdde253580c26 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -33,9 +33,14 @@
#include "llvm/ADT/ArrayRef.h"
#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringExtras.h"
+#include <array>
#include <cinttypes>
+#include <list>
+#include <memory>
#include <optional>
+#include <set>
#include <string>
#include <tuple>
#include <type_traits>
@@ -592,6 +597,44 @@ static bool IsTransformableLoop(const parser::ExecutionPartConstruct &epc) {
return false;
}
+LoopControl::LoopControl(const parser::LoopControl::Bounds &x) {
+ iv = x.Name().thing.symbol;
+ lbound = fromParserExpr(parser::UnwrapRef<parser::Expr>(x.Lower()));
+ ubound = fromParserExpr(parser::UnwrapRef<parser::Expr>(x.Upper()));
+ if (auto &inc{x.Step()}) {
+ step = fromParserExpr(parser::UnwrapRef<parser::Expr>(*inc));
+ }
+}
+
+LoopControl::LoopControl(const parser::ConcurrentControl &x) {
+ auto &[name, lower, upper, inc]{x.t};
+ iv = name.symbol;
+ lbound = fromParserExpr(parser::UnwrapRef<parser::Expr>(lower));
+ ubound = fromParserExpr(parser::UnwrapRef<parser::Expr>(upper));
+ if (inc) {
+ step = fromParserExpr(parser::UnwrapRef<parser::Expr>(inc));
+ }
+}
+
+WithSource<MaybeExpr> LoopControl::fromParserExpr(const parser::Expr &x) {
+ return WithSource<MaybeExpr>(GetEvaluateExpr(x), x.source);
+}
+
+std::vector<LoopControl> GetLoopControls(const parser::DoConstruct &x) {
+ std::vector<LoopControl> controls;
+ if (x.IsDoNormal()) {
+ const parser::LoopControl &control{*x.GetLoopControl()};
+ controls.emplace_back(std::get<parser::LoopControl::Bounds>(control.u));
+ } else if (x.IsDoConcurrent()) {
+ const parser::LoopControl &control{*x.GetLoopControl()};
+ auto &header{parser::UnwrapRef<parser::ConcurrentHeader>(control)};
+ for (auto &cc : std::get<std::list<parser::ConcurrentControl>>(header.t)) {
+ controls.emplace_back(cc);
+ }
+ }
+ return controls;
+}
+
static const auto MsgNotValidAffectedLoop{
"%s is not a valid affected loop"_because_en_US};
static const auto MsgClauseAbsentAssume{
@@ -935,8 +978,6 @@ WithReason<std::pair<int64_t, int64_t>> GetAffectedLoopRangeWithReason(
if (!first || !count || *first <= 0 || *count <= 0) {
return {};
}
- std::string name{
- GetUpperName(llvm::omp::Clause::OMPC_looprange, version)};
Reason reason;
reason.Say(clause->source,
"%s clause was specified with a count of %" PRId64
@@ -1098,6 +1139,17 @@ void LoopSequence::createChildrenFromRange(
}
}
+std::vector<LoopControl> LoopSequence::getLoopControls() const {
+ if (!entry_->owner) {
+ return {};
+ }
+
+ if (auto *loop{parser::Unwrap<parser::DoConstruct>(*entry_->owner)}) {
+ return GetLoopControls(*loop);
+ }
+ return {};
+}
+
void LoopSequence::precalculate() {
// Calculate length before depths.
length_ = calculateLength();
@@ -1210,8 +1262,8 @@ static Reason WhyNotWellFormed(
LoopSequence::Depth LoopSequence::calculateDepths() const {
// Get the length of the nested sequence. The invalidIC_ and opaqueIC_
- // members do not count canonical loop nests, but there can only be one
- // for depth to make sense.
+ // members do not include sibling canonical loop nests, but there can
+ // only be one for depth to make sense.
WithReason<int64_t> nestedLength{getNestedLength()};
// Get the depths of the code nested in this sequence (e.g. contained in
// entry_), and use it as the basis for the depths of entry_->owner.
@@ -1303,7 +1355,7 @@ LoopSequence::Depth LoopSequence::calculateDepths() const {
static_cast<int64_t>(num) + perfDepth};
}
// The SIZES clause is mandatory, if it's missing the result is unknown.
- return {};
+ return Depth{};
case llvm::omp::Directive::OMPD_unroll:
if (isFullUnroll) {
Reason reason;
>From 33e555beae86d04cb4ee268a0a419bb3e52242f3 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 24 Mar 2026 12:11:40 -0500
Subject: [PATCH 5/8] [flang][OpenMP] Implement checks for rectangular loops
Detect non-rectangular loops, emit diagnostic messages when the construct
requires that the affected loops are rectangular. Delete similar checks
from resolve-directive.cpp.
Issue: https://github.com/llvm/llvm-project/issues/185287
---
flang/include/flang/Semantics/openmp-utils.h | 15 ++
flang/lib/Semantics/check-omp-loop.cpp | 31 +++
flang/lib/Semantics/check-omp-structure.h | 2 +
flang/lib/Semantics/openmp-utils.cpp | 191 +++++++++++++++++++
flang/lib/Semantics/resolve-directives.cpp | 84 --------
flang/test/Semantics/OpenMP/do22.f90 | 18 +-
flang/test/Semantics/OpenMP/tile06.f90 | 12 +-
7 files changed, 260 insertions(+), 93 deletions(-)
diff --git a/flang/include/flang/Semantics/openmp-utils.h b/flang/include/flang/Semantics/openmp-utils.h
index f6aebaf8ffdfc..487a0373b10f0 100644
--- a/flang/include/flang/Semantics/openmp-utils.h
+++ b/flang/include/flang/Semantics/openmp-utils.h
@@ -192,6 +192,8 @@ WithReason<int64_t> GetArgumentValueWithReason(
WithReason<int64_t> GetNumArgumentsWithReason(
const parser::OmpDirectiveSpecification &spec, llvm::omp::Clause clauseId,
unsigned version);
+WithReason<int64_t> GetHeightWithReason(
+ const parser::OmpDirectiveSpecification &spec, unsigned version);
// Return the depth of the affected nests:
// {affected-depth, reason, must-be-perfect-nest}.
@@ -202,6 +204,9 @@ std::pair<WithReason<int64_t>, bool> GetAffectedNestDepthWithReason(
// If the range is "the whole sequence", the return value will be {1, -1, ...}.
WithReason<std::pair<int64_t, int64_t>> GetAffectedLoopRangeWithReason(
const parser::OmpDirectiveSpecification &spec, unsigned version);
+/// Return the depth in which all loops must be rectangular.
+WithReason<int64_t> GetRectangularNestDepthWithReason(
+ const parser::OmpDirectiveSpecification &spec, unsigned version);
// Count the required loop count from range. If count == -1, return -1,
// indicating all loops in the sequence.
@@ -233,8 +238,10 @@ struct LoopSequence {
bool isNest() const { return length_.value == 1; }
const WithReason<int64_t> &length() const { return length_; }
+ const WithReason<int64_t> &height() const { return height_; }
const Depth &depth() const { return depth_; }
const std::vector<LoopSequence> &children() const { return children_; }
+ const parser::ExecutionPartConstruct *owner() const { return entry_->owner; }
WithReason<bool> isWellFormedSequence() const;
WithReason<bool> isWellFormedNest() const;
@@ -270,6 +277,7 @@ struct LoopSequence {
WithReason<int64_t> getNestedLength() const;
Depth calculateDepths() const;
Depth getNestedDepths() const;
+ WithReason<int64_t> calculateHeight() const;
/// The construct that is not a loop or a loop-transforming construct,
/// that is also not a valid intervening code. Unset if no such code is
@@ -287,6 +295,13 @@ struct LoopSequence {
WithReason<int64_t> length_;
/// Precalculated depths. Only meaningful if the sequence is a nest.
Depth depth_;
+ /// Precalculated height of the sequence. The height is the difference
+ /// in the nesting level between "this" and any of the children (should
+ /// be the same for each child). Intuitively it is the number of nested
+ /// loops that are added by this construct. If this->depth_ included
+ /// child->depth_ for some child, then
+ /// height_ = this->depth_ - child->depth_
+ WithReason<int64_t> height_;
// The core structure of the class:
unsigned version_; // Needed for GetXyzWithReason
diff --git a/flang/lib/Semantics/check-omp-loop.cpp b/flang/lib/Semantics/check-omp-loop.cpp
index 48d0684320e1c..951751228f34b 100644
--- a/flang/lib/Semantics/check-omp-loop.cpp
+++ b/flang/lib/Semantics/check-omp-loop.cpp
@@ -232,6 +232,35 @@ void OmpStructureChecker::CheckSIMDNest(const parser::OpenMPConstruct &c) {
}
}
+void OmpStructureChecker::CheckRectangularNest(
+ const parser::OmpDirectiveSpecification &spec, const LoopSequence &nest) {
+ unsigned version{context_.langOptions().OpenMPVersion};
+ auto depth{GetRectangularNestDepthWithReason(spec, version)};
+ if (!depth || *depth.value == 0) {
+ return;
+ }
+
+ int64_t height{0};
+ std::vector<const LoopSequence *> outer;
+ for (const LoopSequence *n{&nest}; n;) {
+ if (n->owner()) {
+ WithReason<bool> rect{n->isRectangular(outer)};
+ if (!rect.value.value_or(true)) {
+ auto &msg{context_.Say(spec.DirName().source,
+ "This construct requires a rectangular loop nest, but the associated nest is not"_err_en_US)};
+ depth.reason.AttachTo(msg);
+ rect.reason.AttachTo(msg);
+ }
+ outer.push_back(n);
+ }
+ height += n->height().value.value_or(1);
+ if (height >= *depth.value) {
+ break;
+ }
+ n = n->children().empty() ? nullptr : &n->children().front();
+ }
+}
+
void OmpStructureChecker::CheckNestedConstruct(
const parser::OpenMPLoopConstruct &x) {
const parser::OmpDirectiveSpecification &beginSpec{x.BeginDir()};
@@ -311,6 +340,8 @@ void OmpStructureChecker::CheckNestedConstruct(
perfectTxt, *needDepth.value, perfectTxt, *haveDepth.value)};
haveDepth.reason.AttachTo(msg);
needDepth.reason.AttachTo(msg);
+ } else {
+ CheckRectangularNest(beginSpec, sequence);
}
}
diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h
index a27b16561d3f7..7b4859e312c3f 100644
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -332,6 +332,8 @@ class OmpStructureChecker : public OmpStructureCheckerBase {
void CheckScanModifier(const parser::OmpClause::Reduction &x);
void CheckDistLinear(const parser::OpenMPLoopConstruct &x);
void CheckSIMDNest(const parser::OpenMPConstruct &x);
+ void CheckRectangularNest(const parser::OmpDirectiveSpecification &spec,
+ const omp::LoopSequence &nest);
void CheckNestedConstruct(const parser::OpenMPLoopConstruct &x);
void CheckTargetNest(const parser::OpenMPConstruct &x);
void CheckTargetUpdate();
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index bdde253580c26..18a10be3276a5 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -665,6 +665,24 @@ parser::Message &Reason::AttachTo(parser::Message &msg) {
return msg;
}
+/// From `vars` select the subsequence of symbols that are used in `expr`
+/// either directly, or via some kind of association.
+static SymbolVector SelectUsedSymbols(
+ const SymbolVector &vars, const SomeExpr &expr) {
+ std::set<const Symbol *> uses;
+ for (SymbolRef s : evaluate::GetSymbolVector(expr)) {
+ uses.insert(&s->GetUltimate());
+ }
+
+ SymbolVector deps;
+ for (SymbolRef s : vars) {
+ if (uses.count(&s->GetUltimate())) {
+ deps.push_back(s);
+ }
+ }
+ return deps;
+}
+
WithReason<int64_t> GetArgumentValueWithReason(
const parser::OmpDirectiveSpecification &spec, llvm::omp::Clause clauseId,
unsigned version) {
@@ -719,6 +737,53 @@ WithReason<int64_t> GetNumArgumentsWithReason(
return {};
}
+WithReason<int64_t> GetHeightWithReason(
+ const parser::OmpDirectiveSpecification &spec, unsigned version) {
+ bool isFullUnroll{IsFullUnroll(spec)};
+
+ if (!isFullUnroll && !IsTransformableLoop(spec)) {
+ Reason reason;
+ reason.Say(spec.DirName().source,
+ "This construct is not a DO-loop or a loop-transformation construct"_because_en_US);
+ return {0, reason};
+ }
+ switch (spec.DirName().v) {
+ case llvm::omp::Directive::OMPD_flatten:
+ if (auto &&value{GetArgumentValueWithReason(
+ spec, llvm::omp::Clause::OMPC_depth, version)}) {
+ // FLATTEN DEPTH(n) replaces n loops with 1.
+ return {int64_t(1) - *value.value, std::move(value.reason)};
+ } else {
+ Reason reason;
+ reason.Say(spec.DirName().source, MsgClauseAbsentAssume,
+ GetUpperName(llvm::omp::Clause::OMPC_depth, version),
+ "a depth of 2");
+ return {-1, std::move(reason)};
+ }
+ case llvm::omp::Directive::OMPD_fuse:
+ case llvm::omp::Directive::OMPD_split:
+ return {0, Reason()};
+ case llvm::omp::Directive::OMPD_interchange:
+ case llvm::omp::Directive::OMPD_nothing:
+ case llvm::omp::Directive::OMPD_reverse:
+ return {0, Reason()};
+ case llvm::omp::Directive::OMPD_stripe:
+ case llvm::omp::Directive::OMPD_tile:
+ return GetNumArgumentsWithReason(
+ spec, llvm::omp::Clause::OMPC_sizes, version);
+ case llvm::omp::Directive::OMPD_unroll:
+ if (isFullUnroll) {
+ Reason reason;
+ reason.Say(spec.DirName().source, MsgConstructDoesNotResult,
+ "Fully unrolled loop", "a loop nest");
+ return {-1, std::move(reason)};
+ }
+ return {0, Reason()};
+ default:
+ llvm_unreachable("Expecting loop-transforming construct");
+ }
+}
+
namespace {
// Helper class to check if a given evaluate::Expr is an array expression.
// This does not check any proper subexpressions of the expression (except
@@ -1000,6 +1065,56 @@ WithReason<std::pair<int64_t, int64_t>> GetAffectedLoopRangeWithReason(
return {std::make_pair(1, 1), Reason()};
}
+WithReason<int64_t> GetRectangularNestDepthWithReason(
+ const parser::OmpDirectiveSpecification &spec, unsigned version) {
+ auto [depth, _]{GetAffectedNestDepthWithReason(spec, version)};
+ if (!depth) {
+ return {};
+ }
+
+ // Remove the reasons for the affected depth. Reasons for needing
+ // rectangular loops will be added instead.
+ depth.reason.msgs.clear();
+
+ static const std::array directives{
+ llvm::omp::Directive::OMPD_interchange,
+ llvm::omp::Directive::OMPD_stripe,
+ llvm::omp::Directive::OMPD_tile,
+ };
+
+ llvm::omp::Directive dirId{spec.DirId()};
+ if (llvm::is_contained(directives, dirId)) {
+ depth.reason.Say(spec.DirName().source,
+ "None of the loops affected by %s can be non-rectangular"_because_en_US,
+ GetUpperName(dirId, version));
+ return std::move(depth);
+ }
+
+ static const std::array clauses{
+ llvm::omp::Clause::OMPC_dist_schedule,
+ llvm::omp::Clause::OMPC_grainsize,
+ llvm::omp::Clause::OMPC_induction,
+ llvm::omp::Clause::OMPC_linear,
+ llvm::omp::Clause::OMPC_schedule,
+ };
+
+ auto clauseAt{
+ llvm::find_if(spec.Clauses().v, [&](const parser::OmpClause &c) {
+ llvm::omp::Clause clauseId{c.Id()};
+ return llvm::is_contained(clauses, clauseId) &&
+ llvm::omp::isAllowedClauseForDirective(dirId, clauseId, version);
+ })};
+ if (clauseAt != spec.Clauses().v.end()) {
+ depth.reason.Say(clauseAt->source,
+ "When %s clause is present, none of the loops affected by %s can be non-rectangular"_because_en_US,
+ GetUpperName(clauseAt->Id(), version), GetUpperName(dirId, version));
+ return std::move(depth);
+ }
+
+ // No restrictions.
+ return {0, Reason()};
+}
+
std::optional<int64_t> GetRequiredCount(
std::optional<int64_t> first, std::optional<int64_t> count) {
if (first && count && *first > 0) {
@@ -1154,6 +1269,7 @@ void LoopSequence::precalculate() {
// Calculate length before depths.
length_ = calculateLength();
depth_ = calculateDepths();
+ height_ = calculateHeight();
}
WithReason<int64_t> LoopSequence::calculateLength() const {
@@ -1405,6 +1521,23 @@ LoopSequence::Depth LoopSequence::getNestedDepths() const {
return children_.front().depth_;
}
+WithReason<int64_t> LoopSequence::calculateHeight() const {
+ if (!entry_->owner) {
+ return {0, Reason()};
+ }
+ if (parser::Unwrap<parser::DoConstruct>(*entry_->owner)) {
+ return {1, Reason()};
+ }
+ if (auto *omp{parser::Unwrap<parser::OpenMPLoopConstruct>(*entry_->owner)}) {
+ const parser::OmpDirectiveSpecification &beginSpec{omp->BeginDir()};
+ if (IsLoopTransforming(beginSpec.DirId())) {
+ return GetHeightWithReason(beginSpec, version_);
+ }
+ return {0, Reason()};
+ }
+ return {};
+}
+
static bool IsDoConcurrent(const parser::ExecutionPartConstruct &x) {
if (auto *loop{parser::Unwrap<parser::DoConstruct>(x)}) {
return loop->IsDoConcurrent();
@@ -1465,4 +1598,62 @@ WithReason<bool> LoopSequence::isWellFormedNest() const {
}
return {true, Reason()};
}
+
+static std::string JoinSymbolNames(const SymbolVector &syms) {
+ std::vector<std::string> names;
+ for (SymbolRef s : syms) {
+ names.push_back("'" + s->name().ToString() + "'");
+ }
+ return llvm::join(names, ", ");
+}
+
+static void CheckSymbolExprOverlap(WithReason<bool> &result,
+ const SymbolVector &syms, const SomeExpr &expr, std::string exprName,
+ parser::CharBlock exprSource) {
+ if (auto used{SelectUsedSymbols(syms, expr)}; !used.empty()) {
+ result.value = false;
+ result.reason.Say(exprSource,
+ "The %s of the affected loop uses iteration variables of enclosing loops: %s"_because_en_US,
+ exprName, JoinSymbolNames(used));
+ }
+}
+
+WithReason<bool> LoopSequence::isRectangular(
+ const std::vector<const LoopSequence *> &outer) const {
+ assert(entry_->owner && "Must have owner construct");
+ auto *loop{parser::Unwrap<parser::DoConstruct>(*entry_->owner)};
+ if (!loop) {
+ // Can "rectangular" property be computed for a loop-nest-generating
+ // construct? What if the loops in the nest are not rectangular with
+ // respect to each other?
+ return {};
+ }
+
+ SymbolVector outerIVs;
+ for (auto *sequence : llvm::reverse(outer)) {
+ for (auto &control : sequence->getLoopControls()) {
+ if (control.iv) {
+ outerIVs.emplace_back(*control.iv);
+ }
+ }
+ }
+
+ WithReason<bool> result(true);
+
+ for (auto &control : getLoopControls()) {
+ if (!control.iv || !control.lbound.value || !control.ubound.value) {
+ continue;
+ }
+ CheckSymbolExprOverlap(result, outerIVs, *control.lbound.value,
+ "lower bound", control.lbound.source);
+ CheckSymbolExprOverlap(result, outerIVs, *control.ubound.value,
+ "upper bound", control.ubound.source);
+ if (control.step.value) {
+ CheckSymbolExprOverlap(result, outerIVs, *control.step.value,
+ "iteration step", control.step.source);
+ }
+ }
+
+ return result;
+}
} // namespace Fortran::semantics::omp
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index fb241d2606b47..4553c90d88f7f 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -1097,13 +1097,6 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor<llvm::omp::Directive> {
privateDataSharingAttributeObjects_.clear();
}
- /// Check that loops in the loop nest are perfectly nested, as well that lower
- /// bound, upper bound, and step expressions do not use the iv
- /// of a surrounding loop of the associated loops nest.
- /// We do not support non-perfectly nested loops not non-rectangular loops yet
- /// (both introduced in OpenMP 5.0)
- void CheckPerfectNestAndRectangularLoop(const parser::OpenMPLoopConstruct &x);
-
// Predetermined DSA rules
void PrivatizeAssociatedLoopIndexAndCheckLoopLevel(
const parser::OpenMPLoopConstruct &);
@@ -2062,9 +2055,6 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPLoopConstruct &x) {
}
}
- // Must be done before iv privatization
- CheckPerfectNestAndRectangularLoop(x);
-
PrivatizeAssociatedLoopIndexAndCheckLoopLevel(x);
ordCollapseLevel = GetNumAffectedLoopsFromLoopConstruct(x) + 1;
return true;
@@ -2275,80 +2265,6 @@ void OmpAttributeVisitor::CollectNumAffectedLoopsFromClauses(
}
}
-void OmpAttributeVisitor::CheckPerfectNestAndRectangularLoop(
- const parser::OpenMPLoopConstruct &x) {
- auto &dirContext{GetContext()};
- std::int64_t dirDepth{dirContext.associatedLoopLevel};
- if (dirDepth <= 0)
- return;
-
- auto checkExprHasSymbols = [&](llvm::SmallVector<Symbol *> &ivs,
- const parser::ScalarExpr *bound) {
- if (ivs.empty())
- return;
- auto boundExpr{semantics::AnalyzeExpr(context_, *bound)};
- if (!boundExpr)
- return;
- semantics::UnorderedSymbolSet boundSyms{
- evaluate::CollectSymbols(*boundExpr)};
- if (boundSyms.empty())
- return;
- for (Symbol *iv : ivs) {
- if (boundSyms.count(*iv) != 0) {
- // TODO: Point to occurence of iv in boundExpr, directiveSource as a
- // note
- context_.Say(dirContext.directiveSource,
- "Trip count must be computable and invariant"_err_en_US);
- }
- }
- };
-
- // Find the associated region by skipping nested loop-associated constructs
- // such as loop transformations
- for (auto &construct : std::get<parser::Block>(x.t)) {
- if (const auto *innermostConstruct{parser::omp::GetOmpLoop(construct)}) {
- CheckPerfectNestAndRectangularLoop(*innermostConstruct);
- } else if (const auto *doConstruct{
- parser::omp::GetDoConstruct(construct)}) {
-
- llvm::SmallVector<Symbol *> ivs;
- int curLevel{0};
- const auto *loop{doConstruct};
- while (true) {
- auto [iv, lb, ub, step] = GetLoopBounds(*loop);
-
- if (lb)
- checkExprHasSymbols(ivs, lb);
- if (ub)
- checkExprHasSymbols(ivs, ub);
- if (step)
- checkExprHasSymbols(ivs, step);
- if (iv) {
- if (auto *symbol{currScope().FindSymbol(iv->source)})
- ivs.push_back(symbol);
- }
-
- // Stop after processing all affected loops
- if (curLevel + 1 >= dirDepth)
- break;
-
- // Recurse into nested loop
- const auto &block{std::get<parser::Block>(loop->t)};
- if (block.empty()) {
- break;
- }
-
- loop = GetDoConstructIf(block.front());
- if (!loop) {
- break;
- }
-
- ++curLevel;
- }
- }
- }
-}
-
// 2.15.1.1 Data-sharing Attribute Rules - Predetermined
// - The loop iteration variable(s) in the associated do-loop(s) of a do,
// parallel do, taskloop, or distribute construct is (are) private.
diff --git a/flang/test/Semantics/OpenMP/do22.f90 b/flang/test/Semantics/OpenMP/do22.f90
index a0502aee748ae..2ced881a2af8b 100644
--- a/flang/test/Semantics/OpenMP/do22.f90
+++ b/flang/test/Semantics/OpenMP/do22.f90
@@ -38,9 +38,11 @@ subroutine do_imperfectly_nested_behind
subroutine do_nonrectangular_lb
integer i, j
- !ERROR: Trip count must be computable and invariant
- !$omp do collapse(2)
+ !ERROR: This construct requires a rectangular loop nest, but the associated nest is not
+ !BECAUSE: When SCHEDULE clause is present, none of the loops affected by DO can be non-rectangular
+ !$omp do collapse(2) schedule(auto)
do i = 1, 10
+ !BECAUSE: The lower bound of the affected loop uses iteration variables of enclosing loops: 'i'
do j = i, 10
print *, i, j
end do
@@ -52,9 +54,11 @@ subroutine do_nonrectangular_lb
subroutine do_nonrectangular_ub
integer i, j
- !ERROR: Trip count must be computable and invariant
- !$omp do collapse(2)
+ !ERROR: This construct requires a rectangular loop nest, but the associated nest is not
+ !BECAUSE: When SCHEDULE clause is present, none of the loops affected by DO can be non-rectangular
+ !$omp do collapse(2) schedule(auto)
do i = 1, 10
+ !BECAUSE: The upper bound of the affected loop uses iteration variables of enclosing loops: 'i'
do j = 0, i
print *, i, j
end do
@@ -66,9 +70,11 @@ subroutine do_nonrectangular_ub
subroutine do_nonrectangular_step
integer i, j
- !ERROR: Trip count must be computable and invariant
- !$omp do collapse(2)
+ !ERROR: This construct requires a rectangular loop nest, but the associated nest is not
+ !BECAUSE: When SCHEDULE clause is present, none of the loops affected by DO can be non-rectangular
+ !$omp do collapse(2) schedule(auto)
do i = 1, 10
+ !BECAUSE: The iteration step of the affected loop uses iteration variables of enclosing loops: 'i'
do j = 1, 10, i
print *, i, j
end do
diff --git a/flang/test/Semantics/OpenMP/tile06.f90 b/flang/test/Semantics/OpenMP/tile06.f90
index 52518d43f0554..dfe0594922fc6 100644
--- a/flang/test/Semantics/OpenMP/tile06.f90
+++ b/flang/test/Semantics/OpenMP/tile06.f90
@@ -6,9 +6,11 @@ subroutine nonrectangular_loop_lb
implicit none
integer i, j
- !ERROR: Trip count must be computable and invariant
+ !ERROR: This construct requires a rectangular loop nest, but the associated nest is not
+ !BECAUSE: None of the loops affected by TILE can be non-rectangular
!$omp tile sizes(2,2)
do i = 1, 5
+ !BECAUSE: The upper bound of the affected loop uses iteration variables of enclosing loops: 'i'
do j = 1, i
print *, i, j
end do
@@ -20,9 +22,11 @@ subroutine nonrectangular_loop_ub
implicit none
integer i, j
- !ERROR: Trip count must be computable and invariant
+ !ERROR: This construct requires a rectangular loop nest, but the associated nest is not
+ !BECAUSE: None of the loops affected by TILE can be non-rectangular
!$omp tile sizes(2,2)
do i = 1, 5
+ !BECAUSE: The upper bound of the affected loop uses iteration variables of enclosing loops: 'i'
do j = 1, i
print *, i, j
end do
@@ -34,9 +38,11 @@ subroutine nonrectangular_loop_step
implicit none
integer i, j
- !ERROR: Trip count must be computable and invariant
+ !ERROR: This construct requires a rectangular loop nest, but the associated nest is not
+ !BECAUSE: None of the loops affected by TILE can be non-rectangular
!$omp tile sizes(2,2)
do i = 1, 5
+ !BECAUSE: The iteration step of the affected loop uses iteration variables of enclosing loops: 'i'
do j = 1, 42, i
print *, i, j
end do
>From 2d3fc475416cf44298f29ce5fada8203d952a442 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 6 Apr 2026 13:59:18 -0500
Subject: [PATCH 6/8] format
---
flang/lib/Semantics/openmp-utils.cpp | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index 18a10be3276a5..a8c2c11823ba7 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -756,8 +756,7 @@ WithReason<int64_t> GetHeightWithReason(
} else {
Reason reason;
reason.Say(spec.DirName().source, MsgClauseAbsentAssume,
- GetUpperName(llvm::omp::Clause::OMPC_depth, version),
- "a depth of 2");
+ GetUpperName(llvm::omp::Clause::OMPC_depth, version), "a depth of 2");
return {-1, std::move(reason)};
}
case llvm::omp::Directive::OMPD_fuse:
>From a2959a1fffcc72e222d65d4882e2861240a0f175 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 7 Apr 2026 08:51:58 -0500
Subject: [PATCH 7/8] Remove merge conflict marks
---
flang/include/flang/Semantics/openmp-utils.h | 10 ----------
flang/lib/Semantics/openmp-utils.cpp | 8 --------
2 files changed, 18 deletions(-)
diff --git a/flang/include/flang/Semantics/openmp-utils.h b/flang/include/flang/Semantics/openmp-utils.h
index 7cf7a8f25f857..3a199b6a652c8 100644
--- a/flang/include/flang/Semantics/openmp-utils.h
+++ b/flang/include/flang/Semantics/openmp-utils.h
@@ -55,11 +55,7 @@ const Scope &GetScopingUnit(const Scope &scope);
const Scope &GetProgramUnit(const Scope &scope);
template <typename T> struct WithSource {
-<<<<<<< users/kparzysz/e21-is-rectangular
- template <//
-=======
template < //
->>>>>>> main
typename U = std::remove_reference_t<T>,
typename = std::enable_if_t<std::is_default_constructible_v<U>>>
WithSource() : value(), source() {}
@@ -196,11 +192,8 @@ WithReason<int64_t> GetArgumentValueWithReason(
WithReason<int64_t> GetNumArgumentsWithReason(
const parser::OmpDirectiveSpecification &spec, llvm::omp::Clause clauseId,
unsigned version);
-<<<<<<< users/kparzysz/e21-is-rectangular
WithReason<int64_t> GetHeightWithReason(
const parser::OmpDirectiveSpecification &spec, unsigned version);
-=======
->>>>>>> main
// Return the depth of the affected nests:
// {affected-depth, reason, must-be-perfect-nest}.
@@ -254,13 +247,10 @@ struct LoopSequence {
WithReason<bool> isWellFormedNest() const;
std::vector<LoopControl> getLoopControls() const;
-<<<<<<< users/kparzysz/e21-is-rectangular
// Check if this loop's bounds are invariant in each of the `outer`
// constructs.
WithReason<bool> isRectangular(
const std::vector<const LoopSequence *> &outer) const;
-=======
->>>>>>> main
private:
using Construct = ExecutionPartIterator::Construct;
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index 6dd7d86d760d2..a8c2c11823ba7 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -969,19 +969,11 @@ std::pair<WithReason<int64_t>, bool> GetAffectedNestDepthWithReason(
if (ocount <= 0) {
ocount = std::nullopt;
oreason = Reason();
-<<<<<<< users/kparzysz/e21-is-rectangular
}
if (ccount < ocount) {
// `ocount` cannot be std::nullopt here (C++ std guarantee).
return {{ocount.value_or(1), std::move(oreason)}, true};
}
-=======
- }
- if (ccount < ocount) {
- // `ocount` cannot be std::nullopt here (C++ std guarantee).
- return {{ocount.value_or(1), std::move(oreason)}, true};
- }
->>>>>>> main
return {{ccount.value_or(1), std::move(creason)}, true};
}
>From 645f589997989028406a6695ba083864723def4a Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 7 Apr 2026 08:52:03 -0500
Subject: [PATCH 8/8] Replace std::set with llvm::DenseSet
---
flang/lib/Semantics/openmp-utils.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index a8c2c11823ba7..f183510668449 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -32,6 +32,7 @@
#include "flang/Semantics/symbol.h"
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseSet.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/StringExtras.h"
@@ -40,7 +41,6 @@
#include <list>
#include <memory>
#include <optional>
-#include <set>
#include <string>
#include <tuple>
#include <type_traits>
@@ -669,7 +669,7 @@ parser::Message &Reason::AttachTo(parser::Message &msg) {
/// either directly, or via some kind of association.
static SymbolVector SelectUsedSymbols(
const SymbolVector &vars, const SomeExpr &expr) {
- std::set<const Symbol *> uses;
+ llvm::DenseSet<const Symbol *> uses;
for (SymbolRef s : evaluate::GetSymbolVector(expr)) {
uses.insert(&s->GetUltimate());
}
More information about the flang-commits
mailing list