https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/187866
>From efdb9812bf9ca06e58d3a49c07435b7cec10d457 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek <[email protected]> Date: Fri, 20 Mar 2026 09:40:06 -0500 Subject: [PATCH 1/2] [flang][OpenMP] Provide reasons for calculated sequence length If the length was limited by some factor, include the reason for what caused the reduction. Issue: https://github.com/llvm/llvm-project/issues/185287 --- flang/include/flang/Semantics/openmp-utils.h | 10 ++-- flang/lib/Semantics/check-omp-loop.cpp | 16 +++--- flang/lib/Semantics/openmp-utils.cpp | 55 +++++++++++-------- flang/test/Semantics/OpenMP/fuse1.f90 | 1 + .../loop-transformation-construct02.f90 | 1 + .../loop-transformation-construct04.f90 | 2 + 6 files changed, 51 insertions(+), 34 deletions(-) diff --git a/flang/include/flang/Semantics/openmp-utils.h b/flang/include/flang/Semantics/openmp-utils.h index de5f9cc9072a3..7c95edf81ada2 100644 --- a/flang/include/flang/Semantics/openmp-utils.h +++ b/flang/include/flang/Semantics/openmp-utils.h @@ -197,8 +197,8 @@ struct LoopSequence { WithReason<int64_t> perfect; }; - bool isNest() const { return length_ && *length_ == 1; } - std::optional<int64_t> length() const { return length_; } + bool isNest() const { return length_.value == 1; } + const WithReason<int64_t> &length() const { return length_; } const Depth &depth() const { return depth_; } const std::vector<LoopSequence> &children() const { return children_; } @@ -223,8 +223,8 @@ struct LoopSequence { /// Precalculate length and depth. void precalculate(); - std::optional<int64_t> calculateLength() const; - std::optional<int64_t> getNestedLength() const; + WithReason<int64_t> calculateLength() const; + WithReason<int64_t> getNestedLength() const; Depth calculateDepths() const; Depth getNestedDepths() const; @@ -241,7 +241,7 @@ struct LoopSequence { /// the number of children because a child may result in a sequence, for /// example a fuse with a reduced loop range. The length of that sequence /// adds to the length of the owning LoopSequence. - std::optional<int64_t> length_; + WithReason<int64_t> length_; /// Precalculated depths. Only meaningful if the sequence is a nest. Depth depth_; diff --git a/flang/lib/Semantics/check-omp-loop.cpp b/flang/lib/Semantics/check-omp-loop.cpp index 36b7b22de67ea..df9797ac8e56a 100644 --- a/flang/lib/Semantics/check-omp-loop.cpp +++ b/flang/lib/Semantics/check-omp-loop.cpp @@ -294,26 +294,28 @@ void OmpStructureChecker::CheckNestedConstruct( // in it. auto needRange{GetAffectedLoopRangeWithReason(beginSpec, version)}; - if (std::optional<int64_t> numLoops{sequence.length()}) { - if (*numLoops == 0) { + if (auto haveLength{sequence.length()}) { + if (*haveLength.value == 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 (*numLoops > 1 && assoc == llvm::omp::Association::LoopNest) { - context_.Say(beginSource, + 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, - *numLoops); + *haveLength.value)}; + haveLength.reason.AttachTo(msg); } if (assoc == llvm::omp::Association::LoopSeq) { if (auto requiredCount{GetRequiredCount(needRange.value)}) { - if (*requiredCount > 0 && *numLoops < *requiredCount) { + 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, *numLoops)}; + *requiredCount, *haveLength.value)}; + haveLength.reason.AttachTo(msg); needRange.reason.AttachTo(msg); } } diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp index 185673444f0f0..aa3a7d364c62a 100644 --- a/flang/lib/Semantics/openmp-utils.cpp +++ b/flang/lib/Semantics/openmp-utils.cpp @@ -1083,24 +1083,27 @@ void LoopSequence::precalculate() { depth_ = calculateDepths(); } -std::optional<int64_t> LoopSequence::calculateLength() const { +WithReason<int64_t> LoopSequence::calculateLength() const { if (!entry_->owner) { return getNestedLength(); } if (parser::Unwrap<parser::DoConstruct>(entry_->owner)) { - return 1; + return WithReason<int64_t>(1); } auto &omp{DEREF(parser::Unwrap<parser::OpenMPLoopConstruct>(*entry_->owner))}; const parser::OmpDirectiveSpecification &beginSpec{omp.BeginDir()}; llvm::omp::Directive dir{beginSpec.DirId()}; if (!IsLoopTransforming(dir)) { - return 0; + Reason reason; + reason.Say(beginSpec.DirName().source, + "This construct does not result in a loop nest or a loop sequence"_because_en_US); + return {0, std::move(reason)}; } // TODO: Handle split, apply. if (IsFullUnroll(omp)) { - return std::nullopt; + return {}; } auto nestedLength{getNestedLength()}; @@ -1116,24 +1119,33 @@ std::optional<int64_t> LoopSequence::calculateLength() const { // !$omp do ! error: this should contain a loop (superfluous) // !$omp fuse ! error: this should contain a loop // !$omp end fuse - if (!nestedLength || *nestedLength == 0) { - return std::nullopt; + if (!nestedLength.value || *nestedLength.value == 0) { + return {}; } auto *clause{ parser::omp::FindClause(beginSpec, llvm::omp::Clause::OMPC_looprange)}; if (!clause) { - return 1; + 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_)); + return {1, std::move(reason)}; } auto *loopRange{parser::Unwrap<parser::OmpLooprangeClause>(*clause)}; std::optional<int64_t> count{GetIntValue(std::get<1>(loopRange->t))}; if (!count || *count <= 0) { - return std::nullopt; + return {}; } - if (*count <= *nestedLength) { - return 1 + *nestedLength - *count; + if (*count <= *nestedLength.value) { + int64_t result{1 + *nestedLength.value - *count}; + Reason reason; + reason.Say(beginSpec.DirName().source, + "Out of %" PRId64 " loops, %" PRId64 " were fused"_because_en_US, + *nestedLength.value, *count); + return {result, std::move(reason)}; } - return std::nullopt; + return {}; } if (dir == llvm::omp::Directive::OMPD_nothing) { @@ -1141,16 +1153,16 @@ std::optional<int64_t> LoopSequence::calculateLength() const { } // For every other loop construct return 1. - return 1; + return {1, Reason()}; } -std::optional<int64_t> LoopSequence::getNestedLength() const { - int64_t sum{0}; +WithReason<int64_t> LoopSequence::getNestedLength() const { + WithReason<int64_t> sum(0); for (auto &seq : children_) { - if (auto len{seq.length()}) { - sum += *len; + if (const auto &len{seq.length()}) { + sum = sum + len; } else { - return std::nullopt; + return {}; } } return sum; @@ -1160,7 +1172,7 @@ 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. - std::optional<int64_t> length{getNestedLength()}; + 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. auto [semaDepth, perfDepth]{getNestedDepths()}; @@ -1184,7 +1196,7 @@ LoopSequence::Depth LoopSequence::calculateDepths() const { source, "This code prevents perfect nesting"_because_en_US); } } - if (length.value_or(0) != 1) { + 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) { @@ -1229,13 +1241,12 @@ LoopSequence::Depth LoopSequence::calculateDepths() const { beginSpec, llvm::omp::Clause::OMPC_depth)}) { auto &expr{parser::UnwrapRef<parser::Expr>(clause->u)}; auto value{GetIntValue(expr)}; - auto nestedLength{getNestedLength()}; // The result is a perfect nest only if all loop in the sequence // are fused. - if (value && nestedLength) { + if (value && nestedLength.value) { auto range{GetAffectedLoopRangeWithReason(beginSpec, version_)}; if (auto required{GetRequiredCount(range.value)}) { - if (*required == -1 || *required == *nestedLength) { + if (*required == -1 || *required == *nestedLength.value) { return Depth{value, value}; } Reason reason(std::move(range.reason)); diff --git a/flang/test/Semantics/OpenMP/fuse1.f90 b/flang/test/Semantics/OpenMP/fuse1.f90 index 4dab01ca3ec26..a7dd7e190c16a 100644 --- a/flang/test/Semantics/OpenMP/fuse1.f90 +++ b/flang/test/Semantics/OpenMP/fuse1.f90 @@ -10,6 +10,7 @@ subroutine f !ERROR: This construct requires a sequence of 2 loops, but the loop sequence has a length of 1 !BECAUSE: LOOPRANGE clause was specified with a count of 2 starting at loop 1 !$omp fuse looprange(1, 2) + !BECAUSE: LOOPRANGE clause was not specified, all loops in the sequence are fused !$omp fuse do i = 1, 10 end do diff --git a/flang/test/Semantics/OpenMP/loop-transformation-construct02.f90 b/flang/test/Semantics/OpenMP/loop-transformation-construct02.f90 index 25247c3896cae..d0169c4b721bf 100644 --- a/flang/test/Semantics/OpenMP/loop-transformation-construct02.f90 +++ b/flang/test/Semantics/OpenMP/loop-transformation-construct02.f90 @@ -82,6 +82,7 @@ subroutine loop_transformation_construct6 !ERROR: This construct applies to a loop nest, but has a loop sequence of length 2 !$omp do + !BECAUSE: Out of 2 loops, 1 were fused !$omp fuse looprange(1,1) !$omp unroll partial(2) do x = 1, i diff --git a/flang/test/Semantics/OpenMP/loop-transformation-construct04.f90 b/flang/test/Semantics/OpenMP/loop-transformation-construct04.f90 index 158b030906e07..c23acc2c4c266 100644 --- a/flang/test/Semantics/OpenMP/loop-transformation-construct04.f90 +++ b/flang/test/Semantics/OpenMP/loop-transformation-construct04.f90 @@ -10,6 +10,7 @@ subroutine loop_transformation_construct3 !ERROR: This construct applies to a loop nest, but has a loop sequence of length 2 !$omp do + !BECAUSE: Out of 3 loops, 2 were fused !$omp fuse looprange(1,2) do x = 1, i v(x) = x * 2 @@ -32,6 +33,7 @@ subroutine loop_transformation_construct4 !ERROR: This construct applies to a loop nest, but has a loop sequence of length 2 !$omp tile sizes(2) + !BECAUSE: Out of 3 loops, 2 were fused !$omp fuse looprange(1,2) do x = 1, i v(x) = x * 2 >From 49ebf382ef71c02f3ab57fd925aac4d6fdaffd28 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek <[email protected]> Date: Sat, 21 Mar 2026 09:53:32 -0500 Subject: [PATCH 2/2] Fix message for absent LOOPRANGE --- flang/lib/Semantics/openmp-utils.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp index aa3a7d364c62a..20a4de77a1d7f 100644 --- a/flang/lib/Semantics/openmp-utils.cpp +++ b/flang/lib/Semantics/openmp-utils.cpp @@ -926,7 +926,7 @@ WithReason<std::pair<int64_t, int64_t>> GetAffectedLoopRangeWithReason( // associated sequence". Reason reason; reason.Say(spec.source, - "%s clause was not specified, a value of 1 was assumed"_because_en_US, + "%s clause was not specified, the entire sequence is affected by"_because_en_US, name.c_str()); return {std::make_pair(1, -1), std::move(reason)}; } _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
