[flang-commits] [flang] [flang][OpenMP] Introduce `WithReason<T>` for nest/sequence properties (PR #187563)
Krzysztof Parzyszek via flang-commits
flang-commits at lists.llvm.org
Fri Mar 20 10:03:07 PDT 2026
https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/187563
>From 7d3379ca600347797a945feb6270963def5d5a5d Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 17 Mar 2026 13:08:46 -0500
Subject: [PATCH 1/4] [flang][OpenMP] Allow "Reason" messages to not have
source locations
When explanatory messages are generated there may be cases when there
is no satisfactory source location to apply them to. This patch allows
storing such messages without a source location.
The messages will be equipped with a source location at the time when
they are attached to the main error message (usually it will be the
same location as used for the main message).
Issue: https://github.com/llvm/llvm-project/issues/185287
---
flang/include/flang/Semantics/openmp-utils.h | 17 ++++++++++++++---
flang/lib/Semantics/check-omp-loop.cpp | 18 +++++++++---------
flang/lib/Semantics/openmp-utils.cpp | 16 ++++++++++++++--
3 files changed, 37 insertions(+), 14 deletions(-)
diff --git a/flang/include/flang/Semantics/openmp-utils.h b/flang/include/flang/Semantics/openmp-utils.h
index d7f46cbc7bd62..66bbbeb4ee0e7 100644
--- a/flang/include/flang/Semantics/openmp-utils.h
+++ b/flang/include/flang/Semantics/openmp-utils.h
@@ -23,6 +23,7 @@
#include "flang/Semantics/tools.h"
#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseSet.h"
#include <optional>
#include <string>
@@ -116,12 +117,22 @@ MaybeExpr MakeEvaluateExpr(const parser::OmpStylizedInstance &inp);
struct Reason {
parser::Messages msgs;
- template <typename... Ts> Reason &Say(Ts &&...args) {
- msgs.Say(std::forward<Ts>(args)...);
+ // Allow messages without a source location. They will acquire a location
+ // during AttachTo.
+ template <typename... Ts>
+ Reason &Say(parser::CharBlock source, Ts &&...args) {
+ auto &msg{msgs.Say(source, std::forward<Ts>(args)...)};
+ if (source.empty()) {
+ unsourced_.insert(&msg);
+ }
return *this;
}
operator bool() const { return !msgs.empty(); }
- parser::Message &AttachTo(parser::Message &msg);
+ parser::Message &AttachTo(parser::CharBlock source, parser::Message &msg);
+
+private:
+ // Set of messages without a source location.
+ llvm::DenseSet<const parser::Message *> unsourced_;
};
std::pair<std::optional<int64_t>, Reason> GetArgumentValueWithReason(
diff --git a/flang/lib/Semantics/check-omp-loop.cpp b/flang/lib/Semantics/check-omp-loop.cpp
index 8f360daf4cfdc..f09b5f315dcc2 100644
--- a/flang/lib/Semantics/check-omp-loop.cpp
+++ b/flang/lib/Semantics/check-omp-loop.cpp
@@ -247,6 +247,7 @@ void OmpStructureChecker::CheckNestedConstruct(
const parser::OmpDirectiveSpecification &beginSpec{x.BeginDir()};
llvm::omp::Directive dir{beginSpec.DirId()};
unsigned version{context_.langOptions().OpenMPVersion};
+ parser::CharBlock beginSource{beginSpec.DirName().source};
// End-directive is not allowed in such cases:
// do 100 i = ...
@@ -257,7 +258,6 @@ void OmpStructureChecker::CheckNestedConstruct(
auto &flags{std::get<parser::OmpDirectiveSpecification::Flags>(beginSpec.t)};
if (flags.test(parser::OmpDirectiveSpecification::Flag::CrossesLabelDo)) {
if (auto &endSpec{x.EndDir()}) {
- parser::CharBlock beginSource{beginSpec.DirName().source};
context_
.Say(endSpec->DirName().source,
"END %s directive is not allowed when the construct does not contain all loops that share a loop-terminating statement"_err_en_US,
@@ -297,12 +297,12 @@ void OmpStructureChecker::CheckNestedConstruct(
if (std::optional<int64_t> numLoops{sequence.length()}) {
if (*numLoops == 0) {
- context_.Say(beginSpec.DirName().source,
+ context_.Say(beginSource,
"This construct should contain a DO-loop or a loop-nest-generating OpenMP construct"_err_en_US);
} else {
auto assoc{llvm::omp::getDirectiveAssociation(dir)};
if (*numLoops > 1 && assoc == llvm::omp::Association::LoopNest) {
- context_.Say(beginSpec.DirName().source,
+ context_.Say(beginSource,
"This construct applies to a loop nest, but has a loop sequence of "
"length %" PRId64 ""_err_en_US,
*numLoops);
@@ -310,12 +310,12 @@ void OmpStructureChecker::CheckNestedConstruct(
if (assoc == llvm::omp::Association::LoopSeq) {
if (auto requiredCount{GetRequiredCount(needFirst, needCount)}) {
if (*requiredCount > 0 && *numLoops < *requiredCount) {
- auto &msg{context_.Say(beginSpec.DirName().source,
+ auto &msg{context_.Say(beginSource,
"This construct requires a sequence of %" PRId64
" loops, but the loop sequence has a length of %" PRId64
""_err_en_US,
*requiredCount, *numLoops)};
- rangeReason.AttachTo(msg);
+ rangeReason.AttachTo(beginSource, msg);
}
}
}
@@ -334,18 +334,18 @@ void OmpStructureChecker::CheckNestedConstruct(
if (needDepth && haveDepth && *haveDepth > 0) {
if (*needDepth > *haveDepth) {
if (needPerfect) {
- auto &msg{context_.Say(beginSpec.DirName().source,
+ auto &msg{context_.Say(beginSource,
"This construct requires a perfect nest of depth %" PRId64
", but the associated nest is a perfect nest of depth %" PRId64
""_err_en_US,
*needDepth, *haveDepth)};
- depthReason.AttachTo(msg);
+ depthReason.AttachTo(beginSource, msg);
} else {
- auto &msg{context_.Say(beginSpec.DirName().source,
+ auto &msg{context_.Say(beginSource,
"This construct requires a nest of depth %" PRId64
", but the associated nest has a depth of %" PRId64 ""_err_en_US,
*needDepth, *haveDepth)};
- depthReason.AttachTo(msg);
+ depthReason.AttachTo(beginSource, msg);
}
}
}
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index e104096cb3af1..116d04cf93968 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -532,8 +532,20 @@ MaybeExpr MakeEvaluateExpr(const parser::OmpStylizedInstance &inp) {
instance.u);
}
-parser::Message &Reason::AttachTo(parser::Message &msg) {
- msgs.AttachTo(msg);
+parser::Message &Reason::AttachTo(
+ parser::CharBlock source, parser::Message &msg) {
+ parser::Messages sourced;
+ for (auto &&msg : msgs.messages()) {
+ if (unsourced_.contains(&msg)) {
+ llvm::StringRef fmt{"%s"};
+ sourced.Say(source,
+ parser::MessageFixedText(fmt.data(), fmt.size(), msg.severity()),
+ msg.ToString());
+ } else {
+ sourced.Say(std::move(msg));
+ }
+ }
+ sourced.AttachTo(msg);
return msg;
}
>From c6a64975363612d6bebeb14e0dba32b90147c914 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Wed, 18 Mar 2026 09:45:43 -0500
Subject: [PATCH 2/4] [flang][OpenMP] Store bad ExecutionPartConstruct in
LoopSequence
LoopSequence keeps track of whether it contains code that would be an
invalid intervening code, or that would prevent loop nesting from
being a perfect nesting. To improve the quality of diagnostic messages
store the pointer to the offending parser::ExecutionPartConstruct.
Issue: https://github.com/llvm/llvm-project/issues/185287
---
flang/include/flang/Semantics/openmp-utils.h | 14 +--
flang/lib/Semantics/openmp-utils.cpp | 94 +++++++++++++++++---
2 files changed, 88 insertions(+), 20 deletions(-)
diff --git a/flang/include/flang/Semantics/openmp-utils.h b/flang/include/flang/Semantics/openmp-utils.h
index 66bbbeb4ee0e7..fdd45081e52b5 100644
--- a/flang/include/flang/Semantics/openmp-utils.h
+++ b/flang/include/flang/Semantics/openmp-utils.h
@@ -213,12 +213,14 @@ struct LoopSequence {
Depth calculateDepths() const;
Depth getNestedDepths() const;
- /// True if the sequence contains any code (besides transformable loops)
- /// that is not a valid intervening code.
- bool hasInvalidIC_{false};
- /// True if the sequence contains any code (besides transformable loops)
- /// that is not a valid transparent code.
- bool hasOpaqueIC_{false};
+ /// 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
+ /// present.
+ const parser::ExecutionPartConstruct *invalidIC_{nullptr};
+ /// The construct that is not a loop or a loop-transforming construct,
+ /// whose presence would prevent perfect nesting of loops (i.e. code that
+ /// is not "transparent" to a perfect nest).
+ const parser::ExecutionPartConstruct *opaqueIC_{nullptr};
/// Precalculated length of the sequence. Note that this is different from
/// the number of children because a child may result in a sequence, for
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index 116d04cf93968..0fcc3e725307d 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -787,6 +787,9 @@ bool IsTransformableLoop(const parser::DoConstruct &loop) {
}
bool IsTransformableLoop(const parser::OpenMPLoopConstruct &omp) {
+ if (IsFullUnroll(omp)) {
+ return false;
+ }
return IsLoopTransforming(omp.BeginDir().DirId());
}
@@ -931,6 +934,60 @@ std::optional<int64_t> GetRequiredCount(
return std::nullopt;
}
+#ifdef EXPENSIVE_CHECKS
+namespace {
+/// Check that for every value x of type T, there will be a "source" member
+/// somewhere in x. This is to specifically make sure that parser::GetSource
+/// will return something for any parser::ExecutionPartConstruct.
+
+template <typename...> struct HasSourceT {
+ static constexpr bool value{false};
+};
+
+template <typename T> struct HasSourceT<T> {
+private:
+ using U = llvm::remove_cvref_t<T>;
+
+ static constexpr bool check() {
+ if constexpr (parser::HasSource<U>::value) {
+ return true;
+ } else if constexpr (ConstraintTrait<U>) {
+ return HasSourceT<decltype(U::thing)>::value;
+ } else if constexpr (WrapperTrait<U>) {
+ return HasSourceT<decltype(U::v)>::value;
+ } else if constexpr (TupleTrait<U>) {
+ return HasSourceT<decltype(U::t)>::value;
+ } else if constexpr (UnionTrait<U>) {
+ return HasSourceT<decltype(U::u)>::value;
+ } else {
+ return false;
+ }
+ }
+
+public:
+ static constexpr bool value{check()};
+};
+
+template <> struct HasSourceT<parser::ErrorRecovery> {
+ static constexpr bool value{true};
+};
+
+template <typename T> struct HasSourceT<common::Indirection<T>> {
+ static constexpr bool value{HasSourceT<T>::value};
+};
+
+template <typename... Ts> struct HasSourceT<std::tuple<Ts...>> {
+ static constexpr bool value{(HasSourceT<Ts>::value || ...)};
+};
+
+template <typename... Ts> struct HasSourceT<std::variant<Ts...>> {
+ static constexpr bool value{(HasSourceT<Ts>::value && ...)};
+};
+
+static_assert(HasSourceT<parser::ExecutionPartConstruct>::value);
+} // namespace
+#endif // EXPENSIVE_CHECKS
+
LoopSequence::LoopSequence(const parser::ExecutionPartConstruct &root,
unsigned version, bool allowAllLoops)
: version_(version), allowAllLoops_(allowAllLoops) {
@@ -957,10 +1014,11 @@ std::unique_ptr<LoopSequence::Construct> LoopSequence::createConstructEntry(
return std::make_unique<Construct>(body, &code);
}
} else if (auto *omp{parser::Unwrap<parser::OpenMPLoopConstruct>(code)}) {
- if (IsTransformableLoop(*omp)) {
- auto &body{std::get<parser::Block>(omp->t)};
- return std::make_unique<Construct>(body, &code);
- }
+ // Allow all loop constructs. This helps with better diagnostics, e.g.
+ // "this is not a loop-transforming construct", insted of just "this is
+ // not a valid intervening code".
+ auto &body{std::get<parser::Block>(omp->t)};
+ return std::make_unique<Construct>(body, &code);
}
return nullptr;
@@ -969,6 +1027,7 @@ std::unique_ptr<LoopSequence::Construct> LoopSequence::createConstructEntry(
void LoopSequence::createChildrenFromRange(
ExecutionPartIterator::IteratorType begin,
ExecutionPartIterator::IteratorType end) {
+ bool invalidWithEntry{false};
// Create children. If there is zero or one, this LoopSequence could be
// a nest. If there are more, it could be a proper sequence. In the latter
// case any code between consecutive children must be "transparent".
@@ -976,13 +1035,20 @@ void LoopSequence::createChildrenFromRange(
if (auto entry{createConstructEntry(code)}) {
children_.push_back(
LoopSequence(std::move(entry), version_, allowAllLoops_));
- if (!IsTransformableLoop(code)) {
- hasInvalidIC_ = true;
- hasOpaqueIC_ = true;
+ // Even when DO WHILE et al are allowed to have entries, still treat
+ // them as invalid intervening code.
+ // Give it priority over other kinds of invalid interveninig code.
+ if (!invalidWithEntry && !IsTransformableLoop(code)) {
+ invalidIC_ = &code;
+ invalidWithEntry = true;
}
} else {
- hasInvalidIC_ = hasInvalidIC_ || !IsValidInterveningCode(code);
- hasOpaqueIC_ = hasOpaqueIC_ || !IsTransparentInterveningCode(code);
+ if (!invalidIC_ && !IsValidInterveningCode(code)) {
+ invalidIC_ = &code;
+ }
+ if (!opaqueIC_ && !IsTransparentInterveningCode(code)) {
+ opaqueIC_ = &code;
+ }
}
}
}
@@ -1081,16 +1147,16 @@ LoopSequence::Depth LoopSequence::calculateDepths() const {
return Depth{0, 0};
}
- // Get the length of the nested sequence. The hasInvalidIC_ and hasOpaqueIC_
- // flags do not count canonical loop nests, but there can only be one for
- // depth to make sense.
+ // 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.
std::optional<int64_t> length{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.
auto [semaDepth, perfDepth]{getNestedDepths()};
- if (hasInvalidIC_ || length.value_or(0) != 1) {
+ if (invalidIC_ || length.value_or(0) != 1) {
semaDepth = perfDepth = 0;
- } else if (hasOpaqueIC_ || length.value_or(0) != 1) {
+ } else if (opaqueIC_ || length.value_or(0) != 1) {
perfDepth = 0;
}
>From 29e2922997b88e1a51c06b6b19d0be9cd01c2404 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Thu, 19 Mar 2026 14:21:06 -0500
Subject: [PATCH 3/4] [flang][OpenMP] Introduce `WithReason<T>` for
nest/sequence properties
This helper class contains an optional value and a "reason" message.
It replaces the uses of std::pair<optional<...>, Reason>.
Issue: https://github.com/llvm/llvm-project/issues/185287
---
flang/include/flang/Semantics/openmp-utils.h | 41 +++++--
flang/lib/Semantics/check-omp-loop.cpp | 25 ++--
flang/lib/Semantics/openmp-utils.cpp | 121 +++++++++++++------
3 files changed, 126 insertions(+), 61 deletions(-)
diff --git a/flang/include/flang/Semantics/openmp-utils.h b/flang/include/flang/Semantics/openmp-utils.h
index fdd45081e52b5..ef8dcb821a0e4 100644
--- a/flang/include/flang/Semantics/openmp-utils.h
+++ b/flang/include/flang/Semantics/openmp-utils.h
@@ -115,6 +115,12 @@ MaybeExpr MakeEvaluateExpr(const parser::OmpStylizedInstance &inp);
/// A representation of a "because" message.
struct Reason {
+ Reason() = default;
+ Reason(Reason &&) = default;
+ Reason(const Reason &);
+ Reason &operator=(Reason &&) = default;
+ Reason &operator=(const Reason &);
+
parser::Messages msgs;
// Allow messages without a source location. They will acquire a location
@@ -127,18 +133,36 @@ struct Reason {
}
return *this;
}
- operator bool() const { return !msgs.empty(); }
parser::Message &AttachTo(parser::CharBlock source, parser::Message &msg);
+ Reason &Append(const Reason &other) {
+ CopyFrom(other);
+ return *this;
+ }
+ operator bool() const { return !msgs.empty(); }
private:
// Set of messages without a source location.
llvm::DenseSet<const parser::Message *> unsourced_;
+
+ void CopyFrom(const Reason &other);
};
-std::pair<std::optional<int64_t>, Reason> GetArgumentValueWithReason(
+// A property with an explanation of its value. Both, the property and the
+// reason are optional (the reason can have no messages in it).
+template <typename T> struct WithReason {
+ std::optional<T> value;
+ Reason reason;
+
+ WithReason() = default;
+ WithReason(std::optional<T> v, const Reason &r = Reason())
+ : value(v), reason(r) {}
+ operator bool() const { return value.has_value(); }
+};
+
+WithReason<int64_t> GetArgumentValueWithReason(
const parser::OmpDirectiveSpecification &spec, llvm::omp::Clause clauseId,
unsigned version);
-std::pair<std::optional<int64_t>, Reason> GetNumArgumentsWithReason(
+WithReason<int64_t> GetNumArgumentsWithReason(
const parser::OmpDirectiveSpecification &spec, llvm::omp::Clause clauseId,
unsigned version);
@@ -146,20 +170,21 @@ bool IsLoopTransforming(llvm::omp::Directive dir);
bool IsFullUnroll(const parser::OpenMPLoopConstruct &x);
// Return the depth of the affected nests:
-// {affected-depth, must-be-perfect-nest, reason}.
-std::tuple<std::optional<int64_t>, bool, Reason> GetAffectedNestDepthWithReason(
+// {affected-depth, reason, must-be-perfect-nest}.
+std::pair<WithReason<int64_t>, bool> GetAffectedNestDepthWithReason(
const parser::OmpDirectiveSpecification &spec, unsigned version);
// Return the range of the affected nests in the sequence:
// {first, count, reason}.
// If the range is "the whole sequence", the return value will be {1, -1, ...}.
-std::tuple<std::optional<int64_t>, std::optional<int64_t>, Reason>
-GetAffectedLoopRangeWithReason(
+WithReason<std::pair<int64_t, int64_t>> GetAffectedLoopRangeWithReason(
const parser::OmpDirectiveSpecification &spec, unsigned version);
// Count the required loop count from range. If count == -1, return -1,
// indicating all loops in the sequence.
std::optional<int64_t> GetRequiredCount(
std::optional<int64_t> first, std::optional<int64_t> count);
+std::optional<int64_t> GetRequiredCount(
+ std::optional<std::pair<int64_t, int64_t>> range);
struct LoopSequence {
LoopSequence(const parser::ExecutionPartConstruct &root, unsigned version,
@@ -184,7 +209,7 @@ struct LoopSequence {
bool isNest() const { return length_ && *length_ == 1; }
std::optional<int64_t> length() const { return length_; }
- Depth depth() const { return depth_; }
+ const Depth &depth() const { return depth_; }
const std::vector<LoopSequence> &children() const { return children_; }
private:
diff --git a/flang/lib/Semantics/check-omp-loop.cpp b/flang/lib/Semantics/check-omp-loop.cpp
index f09b5f315dcc2..1d22dccd74827 100644
--- a/flang/lib/Semantics/check-omp-loop.cpp
+++ b/flang/lib/Semantics/check-omp-loop.cpp
@@ -292,8 +292,7 @@ void OmpStructureChecker::CheckNestedConstruct(
// Check if a loop-nest-associated construct has only one top-level loop
// in it.
- auto [needFirst, needCount, rangeReason]{
- GetAffectedLoopRangeWithReason(beginSpec, version)};
+ auto needRange{GetAffectedLoopRangeWithReason(beginSpec, version)};
if (std::optional<int64_t> numLoops{sequence.length()}) {
if (*numLoops == 0) {
@@ -308,14 +307,14 @@ void OmpStructureChecker::CheckNestedConstruct(
*numLoops);
}
if (assoc == llvm::omp::Association::LoopSeq) {
- if (auto requiredCount{GetRequiredCount(needFirst, needCount)}) {
+ if (auto requiredCount{GetRequiredCount(needRange.value)}) {
if (*requiredCount > 0 && *numLoops < *requiredCount) {
auto &msg{context_.Say(beginSource,
"This construct requires a sequence of %" PRId64
" loops, but the loop sequence has a length of %" PRId64
""_err_en_US,
*requiredCount, *numLoops)};
- rangeReason.AttachTo(beginSource, msg);
+ needRange.reason.AttachTo(beginSource, msg);
}
}
}
@@ -323,29 +322,29 @@ void OmpStructureChecker::CheckNestedConstruct(
}
// Check requirements on nest depth.
- auto [needDepth, needPerfect, depthReason]{
+ auto [needDepth, needPerfect]{
GetAffectedNestDepthWithReason(beginSpec, version)};
- auto [haveSema, havePerf]{sequence.depth()};
+ auto &[haveSema, havePerf]{sequence.depth()};
if (dir != llvm::omp::Directive::OMPD_fuse) {
- auto haveDepth = needPerfect ? havePerf : haveSema;
+ auto &haveDepth = needPerfect ? havePerf : haveSema;
// If the present depth is 0, it's likely that the construct doesn't
// have any loops in it, which would be diagnosed above.
- if (needDepth && haveDepth && *haveDepth > 0) {
- if (*needDepth > *haveDepth) {
+ if (needDepth && haveDepth > 0) {
+ if (*needDepth.value > *haveDepth) {
if (needPerfect) {
auto &msg{context_.Say(beginSource,
"This construct requires a perfect nest of depth %" PRId64
", but the associated nest is a perfect nest of depth %" PRId64
""_err_en_US,
- *needDepth, *haveDepth)};
- depthReason.AttachTo(beginSource, msg);
+ *needDepth.value, *haveDepth)};
+ needDepth.reason.AttachTo(beginSource, msg);
} else {
auto &msg{context_.Say(beginSource,
"This construct requires a nest of depth %" PRId64
", but the associated nest has a depth of %" PRId64 ""_err_en_US,
- *needDepth, *haveDepth)};
- depthReason.AttachTo(beginSource, msg);
+ *needDepth.value, *haveDepth)};
+ needDepth.reason.AttachTo(beginSource, msg);
}
}
}
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index 0fcc3e725307d..f3901d62df596 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -532,6 +532,28 @@ MaybeExpr MakeEvaluateExpr(const parser::OmpStylizedInstance &inp) {
instance.u);
}
+Reason::Reason(const Reason &other) { //
+ CopyFrom(other);
+}
+
+Reason &Reason::operator=(const Reason &other) {
+ if (this != &other) {
+ msgs.clear();
+ unsourced_.clear();
+ CopyFrom(other);
+ }
+ return *this;
+}
+
+void Reason::CopyFrom(const Reason &other) {
+ for (auto &msg : other.msgs.messages()) {
+ auto ©{msgs.Say(parser::Message(msg))};
+ if (other.unsourced_.contains(&msg)) {
+ unsourced_.insert(©);
+ }
+ }
+}
+
parser::Message &Reason::AttachTo(
parser::CharBlock source, parser::Message &msg) {
parser::Messages sourced;
@@ -549,7 +571,7 @@ parser::Message &Reason::AttachTo(
return msg;
}
-std::pair<std::optional<int64_t>, Reason> GetArgumentValueWithReason(
+WithReason<int64_t> GetArgumentValueWithReason(
const parser::OmpDirectiveSpecification &spec, llvm::omp::Clause clauseId,
unsigned version) {
if (auto *clause{parser::omp::FindClause(spec, clauseId)}) {
@@ -564,12 +586,11 @@ std::pair<std::optional<int64_t>, Reason> GetArgumentValueWithReason(
}
}
}
- return {std::nullopt, Reason()};
+ return {};
}
template <typename T>
-static std::pair<std::optional<int64_t>, Reason>
-GetNumArgumentsWithReasonForType(
+static WithReason<int64_t> GetNumArgumentsWithReasonForType(
const parser::OmpClause &clause, const std::string &name) {
if (auto *args{parser::Unwrap<std::list<T>>(clause.u)}) {
auto num{static_cast<int64_t>(args->size())};
@@ -579,10 +600,10 @@ GetNumArgumentsWithReasonForType(
name.c_str(), num);
return {num, std::move(reason)};
}
- return {std::nullopt, Reason()};
+ return {};
}
-std::pair<std::optional<int64_t>, Reason> GetNumArgumentsWithReason(
+WithReason<int64_t> GetNumArgumentsWithReason(
const parser::OmpDirectiveSpecification &spec, llvm::omp::Clause clauseId,
unsigned version) {
if (auto *clause{parser::omp::FindClause(spec, clauseId)}) {
@@ -590,22 +611,18 @@ std::pair<std::optional<int64_t>, Reason> GetNumArgumentsWithReason(
// Try the types used for list items.
{
using Ty = parser::ScalarIntExpr;
- if (auto [num, reason]{
- GetNumArgumentsWithReasonForType<Ty>(*clause, name)};
- num) {
- return {num, std::move(reason)};
+ if (auto n{GetNumArgumentsWithReasonForType<Ty>(*clause, name)}) {
+ return n;
}
}
{
using Ty = parser::ScalarIntConstantExpr;
- if (auto [num, reason]{
- GetNumArgumentsWithReasonForType<Ty>(*clause, name)};
- num) {
- return {num, std::move(reason)};
+ if (auto n{GetNumArgumentsWithReasonForType<Ty>(*clause, name)}) {
+ return n;
}
}
}
- return {std::nullopt, Reason()};
+ return {};
}
bool IsLoopTransforming(llvm::omp::Directive dir) {
@@ -803,9 +820,25 @@ bool IsTransformableLoop(const parser::ExecutionPartConstruct &epc) {
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) {
+ if (a.value && b.value) {
+ return WithReason<T>{
+ *a.value + *b.value, Reason().Append(a.reason).Append(b.reason)};
+ }
+ return WithReason<T>();
+}
+
+template <typename T,
+ typename = std::enable_if_t<std::is_arithmetic_v<llvm::remove_cvref_t<T>>>>
+WithReason<T> operator+(T a, const WithReason<T> &b) {
+ return WithReason<T>{a, Reason()} + b;
+}
+
// Return the depth of the affected nests:
// {affected-depth, must-be-perfect-nest}.
-std::tuple<std::optional<int64_t>, bool, Reason> GetAffectedNestDepthWithReason(
+std::pair<WithReason<int64_t>, bool> GetAffectedNestDepthWithReason(
const parser::OmpDirectiveSpecification &spec, unsigned version) {
llvm::omp::Directive dir{spec.DirId()};
bool allowsCollapse{llvm::omp::isAllowedClauseForDirective(
@@ -824,7 +857,7 @@ std::tuple<std::optional<int64_t>, bool, Reason> GetAffectedNestDepthWithReason(
reason = std::move(ro);
}
}
- return {count, true, std::move(reason)};
+ return {{count, std::move(reason)}, true};
}
if (IsLoopTransforming(dir)) {
@@ -834,7 +867,7 @@ std::tuple<std::optional<int64_t>, bool, Reason> GetAffectedNestDepthWithReason(
if (parser::omp::FindClause(spec, llvm::omp::Clause::OMPC_permutation)) {
auto [num, reason]{GetNumArgumentsWithReason(
spec, llvm::omp::Clause::OMPC_permutation, version)};
- return {num, true, std::move(reason)};
+ return {{num, std::move(reason)}, true};
}
// PERMUTATION not specified, assume PERMUTATION(2, 1).
std::string name{parser::omp::GetUpperName(
@@ -843,21 +876,21 @@ std::tuple<std::optional<int64_t>, bool, Reason> GetAffectedNestDepthWithReason(
reason.Say(spec.source,
"%s clause was not specified, %s(2, 1) was assumed"_because_en_US,
name.c_str(), name.c_str());
- return {2, true, std::move(reason)};
+ return {{2, std::move(reason)}, true};
}
case llvm::omp::Directive::OMPD_stripe:
case llvm::omp::Directive::OMPD_tile: {
// Get the length of the argument list to SIZES.
auto [num, reason]{GetNumArgumentsWithReason(
spec, llvm::omp::Clause::OMPC_sizes, version)};
- return {num, true, std::move(reason)};
+ return {{num, std::move(reason)}, true};
}
case llvm::omp::Directive::OMPD_fuse: {
// Get the value from the argument to DEPTH.
if (parser::omp::FindClause(spec, llvm::omp::Clause::OMPC_depth)) {
auto [count, reason]{GetArgumentValueWithReason(
spec, llvm::omp::Clause::OMPC_depth, version)};
- return {count, true, std::move(reason)};
+ return {{count, std::move(reason)}, true};
}
std::string name{
parser::omp::GetUpperName(llvm::omp::Clause::OMPC_depth, version)};
@@ -865,11 +898,11 @@ std::tuple<std::optional<int64_t>, bool, Reason> GetAffectedNestDepthWithReason(
reason.Say(spec.source,
"%s clause was not specified, a value of 1 was assumed"_because_en_US,
name.c_str());
- return {1, true, std::move(reason)};
+ return {{1, std::move(reason)}, true};
}
case llvm::omp::Directive::OMPD_reverse:
case llvm::omp::Directive::OMPD_unroll:
- return {1, false, Reason()};
+ return {WithReason<int64_t>(1), false};
// TODO: case llvm::omp::Directive::OMPD_flatten:
// TODO: case llvm::omp::Directive::OMPD_split:
default:
@@ -877,13 +910,12 @@ std::tuple<std::optional<int64_t>, bool, Reason> GetAffectedNestDepthWithReason(
}
}
- return {std::nullopt, false, Reason()};
+ return {{}, false};
}
// Return the range of the affected nests in the sequence:
// {first, count, std::move(reason)}.
-std::tuple<std::optional<int64_t>, std::optional<int64_t>, Reason>
-GetAffectedLoopRangeWithReason(
+WithReason<std::pair<int64_t, int64_t>> GetAffectedLoopRangeWithReason(
const parser::OmpDirectiveSpecification &spec, unsigned version) {
llvm::omp::Directive dir{spec.DirId()};
@@ -895,7 +927,7 @@ GetAffectedLoopRangeWithReason(
std::optional<int64_t> first{GetIntValue(std::get<0>(range.t))};
std::optional<int64_t> count{GetIntValue(std::get<1>(range.t))};
if (!first || !count || *first <= 0 || *count <= 0) {
- return {std::nullopt, std::nullopt, Reason()};
+ return {};
}
std::string name{parser::omp::GetUpperName(
llvm::omp::Clause::OMPC_looprange, version)};
@@ -904,7 +936,7 @@ GetAffectedLoopRangeWithReason(
"%s clause was specified with a count of %" PRId64
" starting at loop %" PRId64 ""_because_en_US,
name.c_str(), *count, *first);
- return {*first, *count, std::move(reason)};
+ return {std::make_pair(*first, *count), std::move(reason)};
}
// If LOOPRANGE was not found, return {1, -1}, where -1 means "the whole
// associated sequence".
@@ -912,14 +944,14 @@ GetAffectedLoopRangeWithReason(
reason.Say(spec.source,
"%s clause was not specified, a value of 1 was assumed"_because_en_US,
name.c_str());
- return {1, -1, std::move(reason)};
+ return {std::make_pair(1, -1), std::move(reason)};
}
assert(llvm::omp::getDirectiveAssociation(dir) ==
llvm::omp::Association::LoopNest &&
"Expecting loop-nest-associated construct");
// For loop-nest constructs, a single loop-nest is affected.
- return {1, 1, Reason()};
+ return {std::make_pair(1, 1), Reason()};
}
std::optional<int64_t> GetRequiredCount(
@@ -934,6 +966,14 @@ std::optional<int64_t> GetRequiredCount(
return std::nullopt;
}
+std::optional<int64_t> GetRequiredCount(
+ std::optional<std::pair<int64_t, int64_t>> range) {
+ if (range) {
+ return GetRequiredCount(range->first, range->second);
+ }
+ return GetRequiredCount(std::nullopt, std::nullopt);
+}
+
#ifdef EXPENSIVE_CHECKS
namespace {
/// Check that for every value x of type T, there will be a "source" member
@@ -1141,11 +1181,11 @@ LoopSequence::Depth LoopSequence::calculateDepths() const {
return std::nullopt;
}};
- // The sequence length is calculated first, so we already know if this
- // sequence is a nest or not.
- if (!isNest()) {
+ // The sequence length is calculated first, so we already know if this
+ // sequence is a nest or not.
+ if (!isNest()) {
return Depth{0, 0};
- }
+ }
// Get the length of the nested sequence. The invalidIC_ and opaqueIC_
// members do not count canonical loop nests, but there can only be one
@@ -1186,9 +1226,8 @@ LoopSequence::Depth LoopSequence::calculateDepths() const {
// The result is a perfect nest only if all loop in the sequence
// are fused.
if (value && nestedLength) {
- auto [first, count, _]{
- GetAffectedLoopRangeWithReason(beginSpec, version_)};
- if (auto required{GetRequiredCount(first, count)}) {
+ auto range{GetAffectedLoopRangeWithReason(beginSpec, version_)};
+ if (auto required{GetRequiredCount(range.value)}) {
if (*required == -1 || *required == *nestedLength) {
return Depth{value, value};
}
@@ -1197,6 +1236,7 @@ LoopSequence::Depth LoopSequence::calculateDepths() const {
}
return Depth{std::nullopt, std::nullopt};
}
+ // FUSE cannot create a nest of depth > 1 without DEPTH clause.
return Depth{1, 1};
case llvm::omp::Directive::OMPD_interchange:
case llvm::omp::Directive::OMPD_nothing:
@@ -1213,7 +1253,7 @@ LoopSequence::Depth LoopSequence::calculateDepths() const {
return Depth{plus(num, semaDepth), plus(num, perfDepth)};
}
// The SIZES clause is mandatory, if it's missing the result is unknown.
- return {std::nullopt, std::nullopt};
+ return {};
case llvm::omp::Directive::OMPD_unroll:
if (IsFullUnroll(omp)) {
return Depth{0, 0};
@@ -1243,9 +1283,10 @@ LoopSequence::Depth LoopSequence::calculateDepths() const {
}
LoopSequence::Depth LoopSequence::getNestedDepths() const {
- if (length() != 1) {
- return Depth{0, 0};
+ if (!isNest()) {
+ return {std::nullopt, std::nullopt};
} else if (children_.empty()) {
+ // No children, but length == 1.
assert(entry_->owner &&
parser::Unwrap<parser::DoConstruct>(entry_->owner) &&
"Expecting DO construct");
>From 2b910435aa3f35984304acacf2f2810f73f4a447 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Thu, 19 Mar 2026 14:43:12 -0500
Subject: [PATCH 4/4] format
---
flang/lib/Semantics/openmp-utils.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index f3901d62df596..bb9b58747941d 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -1181,11 +1181,11 @@ LoopSequence::Depth LoopSequence::calculateDepths() const {
return std::nullopt;
}};
- // The sequence length is calculated first, so we already know if this
- // sequence is a nest or not.
- if (!isNest()) {
+ // The sequence length is calculated first, so we already know if this
+ // sequence is a nest or not.
+ if (!isNest()) {
return Depth{0, 0};
- }
+ }
// Get the length of the nested sequence. The invalidIC_ and opaqueIC_
// members do not count canonical loop nests, but there can only be one
More information about the flang-commits
mailing list