[flang-commits] [flang] [flang][OpenMP] Introduce `semantics::omp::LoopControl` (PR #190647)
Krzysztof Parzyszek via flang-commits
flang-commits at lists.llvm.org
Tue Apr 7 06:10:41 PDT 2026
https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/190647
>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/7] [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/7] [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/7] [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/7] [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 a301a7852faa93185d6aaf63f5a5b6cdfa9818a8 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 7 Apr 2026 08:08:26 -0500
Subject: [PATCH 5/7] Update check-omp-loop.cpp
---
flang/lib/Semantics/check-omp-loop.cpp | 1 +
1 file changed, 1 insertion(+)
diff --git a/flang/lib/Semantics/check-omp-loop.cpp b/flang/lib/Semantics/check-omp-loop.cpp
index 48d0684320e1c..28a3b558c4835 100644
--- a/flang/lib/Semantics/check-omp-loop.cpp
+++ b/flang/lib/Semantics/check-omp-loop.cpp
@@ -267,6 +267,7 @@ void OmpStructureChecker::CheckNestedConstruct(
}
LoopSequence sequence(body, version, true);
+
auto assoc{llvm::omp::getDirectiveAssociation(dir)};
auto needRange{GetAffectedLoopRangeWithReason(beginSpec, version)};
auto haveLength{sequence.length()};
>From 87c63e8a1d41d735e49fc9ddb100329cafd92f77 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 7 Apr 2026 08:08:52 -0500
Subject: [PATCH 6/7] Update check-omp-loop.cpp
---
flang/lib/Semantics/check-omp-loop.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/flang/lib/Semantics/check-omp-loop.cpp b/flang/lib/Semantics/check-omp-loop.cpp
index 28a3b558c4835..5bbf7bcd627ec 100644
--- a/flang/lib/Semantics/check-omp-loop.cpp
+++ b/flang/lib/Semantics/check-omp-loop.cpp
@@ -267,7 +267,7 @@ void OmpStructureChecker::CheckNestedConstruct(
}
LoopSequence sequence(body, version, true);
-
+
auto assoc{llvm::omp::getDirectiveAssociation(dir)};
auto needRange{GetAffectedLoopRangeWithReason(beginSpec, version)};
auto haveLength{sequence.length()};
>From bcbccf0c90cd876540d3b6682762ee2ed68ff720 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 7 Apr 2026 08:10:27 -0500
Subject: [PATCH 7/7] Apply suggestion from @Meinersbur
Co-authored-by: Michael Kruse <llvm-project at meinersbur.de>
---
flang/include/flang/Semantics/openmp-utils.h | 1 -
1 file changed, 1 deletion(-)
diff --git a/flang/include/flang/Semantics/openmp-utils.h b/flang/include/flang/Semantics/openmp-utils.h
index b567fd1ba8bd9..c5b7034aab61a 100644
--- a/flang/include/flang/Semantics/openmp-utils.h
+++ b/flang/include/flang/Semantics/openmp-utils.h
@@ -125,7 +125,6 @@ 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);
More information about the flang-commits
mailing list