[llvm-branch-commits] [flang] [flang][OpenMP] Store bad ExecutionPartConstruct in LoopSequence (PR #187556)
Krzysztof Parzyszek via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Mar 19 11:54:18 PDT 2026
https://github.com/kparzysz created https://github.com/llvm/llvm-project/pull/187556
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
>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] [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;
}
More information about the llvm-branch-commits
mailing list