[llvm-branch-commits] [flang] [flang][OpenM] Check if loop nest/sequence is well-formed (PR #188025)
via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Mon Mar 23 05:06:50 PDT 2026
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-openmp
Author: Krzysztof Parzyszek (kparzysz)
<details>
<summary>Changes</summary>
Check if the code associated with a nest or sequence construct is well formed. Emit diagnostic messages if not.
Make a clearer separation for checks of loop-nest-associated and loop- sequence-associated constructs.
Unify structure of some of the more common messages.
Issue: https://github.com/llvm/llvm-project/issues/185287
---
Patch is 41.74 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/188025.diff
23 Files Affected:
- (modified) flang/include/flang/Semantics/openmp-utils.h (+3)
- (modified) flang/lib/Semantics/check-omp-loop.cpp (+53-73)
- (modified) flang/lib/Semantics/openmp-utils.cpp (+115-47)
- (modified) flang/lib/Semantics/resolve-directives.cpp (-5)
- (removed) flang/test/Parser/OpenMP/interchange-fail.f90 (-31)
- (removed) flang/test/Parser/OpenMP/tile-fail.f90 (-31)
- (modified) flang/test/Semantics/OpenMP/do-collapse.f90 (+1-2)
- (modified) flang/test/Semantics/OpenMP/do-concurrent-collapse.f90 (+6-5)
- (modified) flang/test/Semantics/OpenMP/do09.f90 (+4-2)
- (modified) flang/test/Semantics/OpenMP/do13.f90 (+5-5)
- (modified) flang/test/Semantics/OpenMP/do21.f90 (+5-5)
- (modified) flang/test/Semantics/OpenMP/fuse1.f90 (+1-1)
- (added) flang/test/Semantics/OpenMP/interchange-fail.f90 (+18)
- (modified) flang/test/Semantics/OpenMP/interchange01.f90 (+5-3)
- (modified) flang/test/Semantics/OpenMP/loop-association.f90 (+9-6)
- (modified) flang/test/Semantics/OpenMP/loop-transformation-construct01.f90 (+10-7)
- (modified) flang/test/Semantics/OpenMP/loop-transformation-construct02.f90 (+6-5)
- (modified) flang/test/Semantics/OpenMP/loop-transformation-construct04.f90 (+2-2)
- (added) flang/test/Semantics/OpenMP/tile-fail.f90 (+18)
- (modified) flang/test/Semantics/OpenMP/tile02.f90 (+2-1)
- (modified) flang/test/Semantics/OpenMP/tile03.f90 (+2-1)
- (modified) flang/test/Semantics/OpenMP/tile08.f90 (+2-2)
- (modified) flang/test/Semantics/OpenMP/tile09.f90 (+2-1)
``````````diff
diff --git a/flang/include/flang/Semantics/openmp-utils.h b/flang/include/flang/Semantics/openmp-utils.h
index 7c95edf81ada2..bf35598177e17 100644
--- a/flang/include/flang/Semantics/openmp-utils.h
+++ b/flang/include/flang/Semantics/openmp-utils.h
@@ -202,6 +202,9 @@ struct LoopSequence {
const Depth &depth() const { return depth_; }
const std::vector<LoopSequence> &children() const { return children_; }
+ WithReason<bool> isWellFormedSequence() const;
+ WithReason<bool> isWellFormedNest() 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 df9797ac8e56a..962537496e580 100644
--- a/flang/lib/Semantics/check-omp-loop.cpp
+++ b/flang/lib/Semantics/check-omp-loop.cpp
@@ -61,16 +61,6 @@ class AssociatedLoopChecker {
constructNamesAndLevels_.emplace(
constructName.value().ToString(), level_);
}
- if (level_ >= 0) {
- if (dc.IsDoWhile()) {
- context_.Say(doStmt.source,
- "The associated loop of a loop-associated directive cannot be a DO WHILE."_err_en_US);
- }
- if (!dc.GetLoopControl()) {
- context_.Say(doStmt.source,
- "The associated loop of a loop-associated directive cannot be a DO without control."_err_en_US);
- }
- }
return true;
}
@@ -268,93 +258,83 @@ void OmpStructureChecker::CheckNestedConstruct(
// Check constructs contained in the body of the loop construct.
auto &body{std::get<parser::Block>(x.t)};
+
for (auto &stmt : BlockRange(body, BlockRange::Step::Over)) {
if (auto *d{parser::Unwrap<parser::CompilerDirective>(stmt)}) {
context_.Say(d->source,
"Compiler directives are not allowed inside OpenMP loop constructs"_warn_en_US);
- } else if (auto *omp{parser::Unwrap<parser::OpenMPLoopConstruct>(stmt)}) {
- if (!IsLoopTransforming(omp->BeginDir().DirId())) {
- context_.Say(omp->source,
- "Only loop-transforming OpenMP constructs are allowed inside OpenMP loop constructs"_err_en_US);
- }
- if (IsFullUnroll(*omp)) {
- context_.Say(x.source,
- "OpenMP loop construct cannot apply to a fully unrolled loop"_err_en_US);
- }
- } else if (!parser::Unwrap<parser::DoConstruct>(stmt)) {
- parser::CharBlock source{parser::GetSource(stmt).value_or(x.source)};
- context_.Say(source,
- "OpenMP loop construct can only contain DO loops or loop-nest-generating OpenMP constructs"_err_en_US);
}
}
LoopSequence sequence(body, version, true);
- // Check if a loop-nest-associated construct has only one top-level loop
- // in it.
+ auto assoc{llvm::omp::getDirectiveAssociation(dir)};
auto needRange{GetAffectedLoopRangeWithReason(beginSpec, version)};
+ auto haveLength{sequence.length()};
- if (auto haveLength{sequence.length()}) {
- if (*haveLength.value == 0) {
+ if (assoc == llvm::omp::Association::LoopNest) {
+ if (sequence.children().size() == 0) {
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 (*haveLength.value > 1 && assoc == llvm::omp::Association::LoopNest) {
- auto &msg{context_.Say(beginSource,
- "This construct applies to a loop nest, but has a loop sequence of "
- "length %" PRId64 ""_err_en_US,
- *haveLength.value)};
- haveLength.reason.AttachTo(msg);
- }
- if (assoc == llvm::omp::Association::LoopSeq) {
- if (auto requiredCount{GetRequiredCount(needRange.value)}) {
- if (*requiredCount > 0 && *haveLength.value < *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, *haveLength.value)};
- haveLength.reason.AttachTo(msg);
- needRange.reason.AttachTo(msg);
- }
- }
- }
+ "This construct should contain a DO-loop or a loop-nest-generating construct"_err_en_US);
+ } else if (haveLength.value > 1) {
+ auto &msg{context_.Say(beginSource,
+ "This construct applies to a loop nest, but has a loop sequence of "
+ "length %" PRId64 ""_err_en_US,
+ *haveLength.value)};
+ haveLength.reason.AttachTo(msg);
+ }
+ auto [isWellFormed, whyNot]{sequence.isWellFormedNest()};
+ if (isWellFormed && !*isWellFormed) {
+ auto &msg{context_.Say(beginSource,
+ "This construct requires a canonical loop nest"_err_en_US)};
+ whyNot.AttachTo(msg);
}
- }
- // Check requirements on nest depth.
- auto [needDepth, needPerfect]{
- GetAffectedNestDepthWithReason(beginSpec, version)};
- auto &[haveSema, havePerf]{sequence.depth()};
+ // Check requirements on nest depth.
+ auto [needDepth, needPerfect]{
+ GetAffectedNestDepthWithReason(beginSpec, version)};
+ auto &[haveSema, havePerf]{sequence.depth()};
+
+ auto haveDepth{needPerfect ? havePerf : haveSema};
+ std::string_view perfectTxt{needPerfect ? " perfect" : ""};
- if (dir != llvm::omp::Directive::OMPD_fuse) {
- 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.value > 0) {
if (*needDepth.value > *haveDepth.value) {
- if (needPerfect) {
+ auto &msg{context_.Say(beginSource,
+ "This construct requires a%s nest of depth %" PRId64
+ ", but the associated nest is a%s nest of depth %" PRId64
+ ""_err_en_US,
+ perfectTxt, *needDepth.value, perfectTxt, *haveDepth.value)};
+ haveDepth.reason.AttachTo(msg);
+ needDepth.reason.AttachTo(msg);
+ }
+ }
+
+ } else if (assoc == llvm::omp::Association::LoopSeq) {
+ if (haveLength.value == 0) {
+ context_.Say(beginSource,
+ "This construct should contain a DO-loop or a loop-sequence-generating construct"_err_en_US);
+ } else {
+ auto [isWellFormed, whyNot]{sequence.isWellFormedSequence()};
+ if (isWellFormed && !*isWellFormed) {
+ auto &msg{context_.Say(beginSource,
+ "This construct requires a canonical loop sequence"_err_en_US)};
+ whyNot.AttachTo(msg);
+ }
+ if (auto requiredCount{GetRequiredCount(needRange.value)}) {
+ if (*requiredCount > 0 && haveLength.value < *requiredCount) {
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
+ "This construct requires a sequence of %" PRId64
+ " loops, but the loop sequence has a length of %" PRId64
""_err_en_US,
- *needDepth.value, *haveDepth.value)};
- haveDepth.reason.AttachTo(msg);
- needDepth.reason.AttachTo(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.value, *haveDepth.value)};
- haveDepth.reason.AttachTo(msg);
- needDepth.reason.AttachTo(msg);
+ *requiredCount, *haveLength.value)};
+ haveLength.reason.AttachTo(msg);
+ needRange.reason.AttachTo(msg);
}
}
}
- } else {
- // FUSE requires a sequence of perfect nests.
- // TODO: Defer this check for now.
}
}
diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp
index 20a4de77a1d7f..d9fe3c4a5d02f 100644
--- a/flang/lib/Semantics/openmp-utils.cpp
+++ b/flang/lib/Semantics/openmp-utils.cpp
@@ -532,6 +532,13 @@ MaybeExpr MakeEvaluateExpr(const parser::OmpStylizedInstance &inp) {
instance.u);
}
+static const auto MsgNotValidAffectedLoop{
+ "%s is not a valid affected loop"_because_en_US};
+static const auto MsgClauseAbsentAssume{
+ "%s clause was not specified, %s is assumed"_because_en_US};
+static const auto MsgConstructDoesNotResult{
+ "%s does not result in %s"_because_en_US};
+
Reason::Reason(const Reason &other) { //
CopyFrom(other);
}
@@ -565,7 +572,7 @@ WithReason<int64_t> GetArgumentValueWithReason(
Reason reason;
reason.Say(clause->source,
"%s clause was specified with argument %" PRId64 ""_because_en_US,
- name.c_str(), *value);
+ name, *value);
return {*value, std::move(reason)};
}
}
@@ -581,7 +588,7 @@ static WithReason<int64_t> GetNumArgumentsWithReasonForType(
Reason reason;
reason.Say(clause.source,
"%s clause was specified with %" PRId64 " arguments"_because_en_US,
- name.c_str(), num);
+ name, num);
return {num, std::move(reason)};
}
return {};
@@ -857,9 +864,8 @@ std::pair<WithReason<int64_t>, bool> GetAffectedNestDepthWithReason(
std::string name{parser::omp::GetUpperName(
llvm::omp::Clause::OMPC_permutation, version)};
Reason reason;
- reason.Say(spec.source,
- "%s clause was not specified, %s(2, 1) was assumed"_because_en_US,
- name.c_str(), name.c_str());
+ reason.Say(
+ spec.source, MsgClauseAbsentAssume, name, "a permutation (2, 1)");
return {{2, std::move(reason)}, true};
}
case llvm::omp::Directive::OMPD_stripe:
@@ -879,9 +885,7 @@ std::pair<WithReason<int64_t>, bool> GetAffectedNestDepthWithReason(
std::string name{
parser::omp::GetUpperName(llvm::omp::Clause::OMPC_depth, version)};
Reason reason;
- reason.Say(spec.source,
- "%s clause was not specified, a value of 1 was assumed"_because_en_US,
- name.c_str());
+ reason.Say(spec.source, MsgClauseAbsentAssume, name, "a value of 1");
return {{1, std::move(reason)}, true};
}
case llvm::omp::Directive::OMPD_reverse:
@@ -919,15 +923,14 @@ WithReason<std::pair<int64_t, int64_t>> GetAffectedLoopRangeWithReason(
reason.Say(clause->source,
"%s clause was specified with a count of %" PRId64
" starting at loop %" PRId64 ""_because_en_US,
- name.c_str(), *count, *first);
+ name, *count, *first);
return {std::make_pair(*first, *count), std::move(reason)};
}
// If LOOPRANGE was not found, return {1, -1}, where -1 means "the whole
// associated sequence".
Reason reason;
- reason.Say(spec.source,
- "%s clause was not specified, the entire sequence is affected by"_because_en_US,
- name.c_str());
+ reason.Say(
+ spec.source, MsgClauseAbsentAssume, name, "the entire loop sequence");
return {std::make_pair(1, -1), std::move(reason)};
}
@@ -1096,8 +1099,8 @@ WithReason<int64_t> LoopSequence::calculateLength() const {
llvm::omp::Directive dir{beginSpec.DirId()};
if (!IsLoopTransforming(dir)) {
Reason reason;
- reason.Say(beginSpec.DirName().source,
- "This construct does not result in a loop nest or a loop sequence"_because_en_US);
+ reason.Say(beginSpec.DirName().source, MsgConstructDoesNotResult,
+ GetUpperName(dir, version_), "a loop nest or a loop sequence");
return {0, std::move(reason)};
}
@@ -1126,9 +1129,9 @@ WithReason<int64_t> LoopSequence::calculateLength() const {
parser::omp::FindClause(beginSpec, llvm::omp::Clause::OMPC_looprange)};
if (!clause) {
Reason reason;
- reason.Say(beginSpec.DirName().source,
- "%s clause was not specified, all loops in the sequence are fused"_because_en_US,
- GetUpperName(llvm::omp::Clause::OMPC_looprange, version_));
+ reason.Say(beginSpec.DirName().source, MsgClauseAbsentAssume,
+ GetUpperName(llvm::omp::Clause::OMPC_looprange, version_),
+ "the entire loop sequence");
return {1, std::move(reason)};
}
@@ -1141,7 +1144,7 @@ WithReason<int64_t> LoopSequence::calculateLength() const {
int64_t result{1 + *nestedLength.value - *count};
Reason reason;
reason.Say(beginSpec.DirName().source,
- "Out of %" PRId64 " loops, %" PRId64 " were fused"_because_en_US,
+ "Out of %" PRId64 " loops, %" PRId64 " are fused"_because_en_US,
*nestedLength.value, *count);
return {result, std::move(reason)};
}
@@ -1168,6 +1171,25 @@ WithReason<int64_t> LoopSequence::getNestedLength() const {
return sum;
}
+static void ResetIfPositiveWithReason(
+ WithReason<int64_t> &quantity, const Reason &reason) {
+ if (quantity.value > 0) {
+ quantity.value = 0;
+ quantity.reason.Append(reason);
+ }
+}
+
+static void ResetIfPositiveWithReason(WithReason<int64_t> &quantity,
+ parser::CharBlock source, parser::MessageFixedText msg) {
+ if (quantity.value > 0) {
+ quantity.value = 0;
+ quantity.reason.Say(source, msg);
+ }
+}
+
+static Reason whyNotWellFormed(
+ const parser::ExecutionPartConstruct &badCode, bool isSequence);
+
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
@@ -1177,38 +1199,22 @@ LoopSequence::Depth LoopSequence::calculateDepths() const {
// entry_), and use it as the basis for the depths of entry_->owner.
auto [semaDepth, perfDepth]{getNestedDepths()};
if (invalidIC_) {
- parser::CharBlock source{*parser::GetSource(*invalidIC_)};
- if (semaDepth.value > 9) {
- semaDepth.value = 0;
- semaDepth.reason.Say(
- source, "This is not a valid intervening code"_because_en_US);
- }
- if (perfDepth.value > 0) {
- perfDepth.value = 0;
- perfDepth.reason.Say(
- source, "This is not a valid intervening code"_because_en_US);
- }
+ auto whyNot{whyNotWellFormed(*invalidIC_, false)};
+ ResetIfPositiveWithReason(semaDepth, whyNot);
+ ResetIfPositiveWithReason(perfDepth, whyNot);
} else if (opaqueIC_) {
+ auto message{"This code prevents perfect nesting"_because_en_US};
parser::CharBlock source{*parser::GetSource(*opaqueIC_)};
- if (perfDepth.value > 0) {
- perfDepth.value = 0;
- perfDepth.reason.Say(
- source, "This code prevents perfect nesting"_because_en_US);
- }
+ ResetIfPositiveWithReason(perfDepth, source, message);
}
if (nestedLength.value.value_or(0) != 1) {
// This may simply be the bottom of the loop nest. Only emit messages
// if the depths are reset back to 0.
if (entry_->owner) {
+ auto message{"This construct does not contain a loop nest"_because_en_US};
parser::CharBlock source{*parser::GetSource(*entry_->owner)};
- if (semaDepth.value > 0) {
- semaDepth.reason.Say(source,
- "This construct does not contain a loop nest"_because_en_US);
- }
- if (perfDepth.value > 9) {
- perfDepth.reason.Say(source,
- "This construct does not contain a loop nest"_because_en_US);
- }
+ ResetIfPositiveWithReason(semaDepth, source, message);
+ ResetIfPositiveWithReason(perfDepth, source, message);
}
semaDepth.value = perfDepth.value = 0;
}
@@ -1249,10 +1255,12 @@ LoopSequence::Depth LoopSequence::calculateDepths() const {
if (*required == -1 || *required == *nestedLength.value) {
return Depth{value, value};
}
+ std::string name{
+ GetUpperName(llvm::omp::Directive::OMPD_fuse, version_)};
Reason reason(std::move(range.reason));
- reason.Say(beginSpec.DirName().source,
- "%s construct results in a proper loop-sequence"_because_en_US,
- GetUpperName(llvm::omp::Directive::OMPD_fuse, version_));
+ reason.Say(beginSpec.DirName().source, MsgConstructDoesNotResult,
+ "This " + name + " construct",
+ "a loop nest, but a proper loop sequence");
return Depth{{1, reason}, {1, reason}};
}
}
@@ -1281,8 +1289,8 @@ LoopSequence::Depth LoopSequence::calculateDepths() const {
case llvm::omp::Directive::OMPD_unroll:
if (isFullUnroll) {
Reason reason;
- reason.Say(beginSpec.DirName().source,
- "Fully unrolled loop does not result in a loop nest"_because_en_US);
+ reason.Say(beginSpec.DirName().source, MsgConstructDoesNotResult,
+ "Fully unrolled loop", "a loop nest");
return Depth{{0, reason}, {0, reason}};
}
// If this is not a full unroll then look for a PARTIAL clause.
@@ -1326,4 +1334,64 @@ LoopSequence::Depth LoopSequence::getNestedDepths() const {
}
return children_.front().depth_;
}
+
+static bool IsDoConcurrent(const parser::ExecutionPartConstruct &x) {
+ if (auto *loop{parser::Unwrap<parser::DoConstruct>(x)}) {
+ return loop->IsDoConcurrent();
+ }
+ return false;
+}
+
+static Reason whyNotWellFormed(
+ const parser::ExecutionPartConstruct &badCode, bool isSequence) {
+ Reason reason;
+ parser::CharBlock source{*parser::GetSource(badCode)};
+ if (auto *omp{parser::Unwrap<parser::OpenMPLoopConstruct>(badCode)}) {
+ if (IsFullUnroll(*omp)) {
+ reason.Say(source, MsgConstructDoesNotResult, "Fully unrolled loop",
+ isSequence ? "a loop nest or a loop sequence" : "a loop nest");
+ } else if (!IsLoopTransforming(omp->BeginDir().DirId())) {
+ reason.Say(source,
+ "Only loop-transforming constructs are allowed inside loop constructs"_because_en_US);
+ }
+ return reason;
+ }
+
+ if (auto *loop{parser::Unwrap<parser::DoConstruct>(badCode)}) {
+ if (loop->IsDoWhile()) {
+ reason.Say(source, MsgNotValidAffectedLoop, "DO WHILE loop");
+ } else if (loop->IsDoConcurrent()) {
+ reason.Say(source, MsgNotValidAffectedLoop, "DO CONCURRENT loop");
+ } else if (!loop->GetLoopControl()) {
+ reason.Say(
+ source, MsgNotValidAffectedLoop, "DO loop without loop control");
+ }
+ if (reason) {
+ return reason;
+ }
+ }
+ reason.Say(source,
+ "The %s contains code that prevents it from being canonical at this nesting level"_because_en_US,
+ isSequence ? "sequence" : "nest");
+ return reason;
+}
+
+WithReason<bool> LoopSequence::isWellFormedSequence() const {
+ const parser::ExecutionPartConstruct *badCode{
+ invalidIC_ ? invalidIC_ : opaqueIC_};
+ if (badCode) {
+ return {false, whyNotWellFormed(*badCode, true)};
+ }
+ return {true, Reason()};
+}
+
+WithReason<bool> LoopSequence::isWellFormedNest() const {
+ // DO CONCURRENT is allowed at the top level in OpenMP 6.0+.
+ if (invalidIC_) {
+ if (version_ < 60 || !IsDoConcurrent(*invalidIC_)) {
+ return {false, whyNotWellFormed(*invalidIC_, false)};
+ }
+ }
+ return {true, Reason()};
+}
} // namespace Fortran::semantics::omp
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 7fca738d43ca6..3d55568c0cd03 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2395,11 +2395,6 @@ void OmpAttributeVisitor::PrivatizeAssociatedLoopIndexAndCheckLoopLevel(
context_.Say(clause->source,
"DO CONCURRENT loops cannot be used with the COLLAPSE clause."_err_en_US);
}
- } else {
- auto &stmt =
- std::get<parser::Statement<parser::NonLabelDoStmt>>(loop->t);
- context_.Say(stmt.source,
- "DO CONCURRENT loops cannot form part of a loop nest."_err_en_US);
}
}
// go through all the nested do-loops and resolve index variables
diff --git a/flang/test/Parser/OpenMP/interchange-fail.f90 b/flang/test/Parser/OpenMP/interchange-fail.f90
deleted file mode 100644
index d83ef1746f30f..0000000000000
--- a/flang/test/Parser/OpenMP/interchange-fail.f90
+++ /dev/null
@@ -1,31 +0,0 @@
-! RUN: split-file %s %t
-! RUN: not %flang_fc1 -fsyntax-only -fopenmp -fopenmp-version=60 %t/stray_end1.f90 2>&1 | Fi...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/188025
More information about the llvm-branch-commits
mailing list