diff --git a/flang/include/flang/Semantics/openmp-utils.h b/flang/include/flang/Semantics/openmp-utils.h index 252c88c967750..665d5f221b43a 100644 --- a/flang/include/flang/Semantics/openmp-utils.h +++ b/flang/include/flang/Semantics/openmp-utils.h @@ -242,6 +242,15 @@ std::optional GetMinimumSequenceCount( std::optional GetMinimumSequenceCount( std::optional> range); +/// Collect the set of DO loops present in the source code that are directly +/// affected by the given loop construct. This does not include any DO loops +/// that are affected by any construct nested in `x`. +/// Returns std::nullopt if `x` or code nested in `x` was malformed in a +/// way that prevented the function from returning an accurate result. +std::optional> CollectAffectedDoLoops( + const parser::OpenMPLoopConstruct &x, unsigned version, + SemanticsContext *semaCtx = nullptr); + struct LoopSequence { LoopSequence(const parser::ExecutionPartConstruct &root, unsigned version, bool allowAllLoops = false, SemanticsContext *semaCtx = nullptr); diff --git a/flang/lib/Semantics/openmp-utils.cpp b/flang/lib/Semantics/openmp-utils.cpp index 436ee35664f2c..58aefe2e1fc52 100644 --- a/flang/lib/Semantics/openmp-utils.cpp +++ b/flang/lib/Semantics/openmp-utils.cpp @@ -1203,6 +1203,102 @@ std::optional GetMinimumSequenceCount( return GetMinimumSequenceCount(std::nullopt, std::nullopt); } +/// Collect the DO loops that are affected directly by the given loop +/// transformation. Not all DO loops nested in the associated nest are +/// affected by the top-level loop transformation, e.g. +/// +/// !$omp do collapse(5) | [2] +/// !$omp tile sizes(2, 2) | [1] | <- nest of 4 loops +/// do i = 1, 10 | <- affected by TILE | generated by TILE +/// do j = 1, 10 | <- | +/// do k = 1, 10 | <- affected by DO +/// end do +/// end do +/// end do +/// +/// The two DO loops (i and j) in [1] are affected by the TILE construct. +/// The k DO loop is affected by the DO construct [2]. +/// For the top-level DO COLLAPSE(5) construct, the k loop is the only +/// directly affected loop. +std::optional> CollectAffectedDoLoops( + const parser::OpenMPLoopConstruct &x, unsigned version, + SemanticsContext *semaCtx) { + std::vector result; + const parser::OmpDirectiveSpecification &spec{x.BeginDir()}; + + auto [depth, _]{GetAffectedNestDepthWithReason(spec, version, semaCtx)}; + + // If the depth is absent, then there is some issue. Leave it alone here, + // and let the semantic checks diagnose the problem. + if (!depth) { + return std::nullopt; + } + if (*depth.value == 0) { + return result; + } + assert(*depth.value > 0 && "Expecting positive depth"); + + // The algorithm is to descend down the nest and keep track of intervening + // constructs and how many loops they consume and produce. This is similar + // to traversing an expression tree to identify the operands to the top- + // level operation: + // + // ... + + x y z w ... + // ^ ^ ^ + // | | | + // | | +-- produces 1 value consumed by the first + + // | +-- produces 1 value, but first consumes 2 + // +-- consumes 2 operands + // + // The analogous result here would be "z" as the operand to the first +. + + int64_t produced{0}; + int64_t consuming{0}; + int64_t level{*depth.value}; + + auto visit{[&](const LoopSequence &nest, auto &&self) -> bool { + const parser::ExecutionPartConstruct *owner{nest.owner()}; + + if (auto *doLoop{parser::Unwrap(owner)}) { + if (consuming == 0) { + result.push_back(doLoop); + ++produced; + } else { + --consuming; + } + } else if (auto *omp{parser::Unwrap(owner)}) { + const parser::OmpDirectiveSpecification &ods{omp->BeginDir()}; + auto [cons, _1]{GetAffectedNestDepthWithReason(ods, version, semaCtx)}; + auto [prod, _2]{GetGeneratedNestDepthWithReason(ods, version, semaCtx)}; + if (!cons || !prod) { + return false; + } + if (*prod.value <= consuming) { + consuming -= *prod.value; + } else { + produced += (*prod.value - consuming); + consuming = 0; + } + consuming += *cons.value; + } + + bool success{true}; + if (produced < level) { + for (const LoopSequence &child : nest.children()) { + success = success && self(child, self); + } + } + + return success && produced >= level; + }}; + + LoopSequence sequence(std::get(x.t), version, true, semaCtx); + if (visit(sequence, visit)) { + return result; + } + return std::nullopt; +} + #ifdef EXPENSIVE_CHECKS namespace { /// Check that for every value x of type T, there will be a "source" member diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index c50c76647962c..6b42a7290e260 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -1114,8 +1114,7 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor { } // Predetermined DSA rules - void PrivatizeAssociatedLoopIndexAndCheckLoopLevel( - const parser::OpenMPLoopConstruct &); + void PrivatizeAssociatedLoopIndex(const parser::OpenMPLoopConstruct &); void ResolveSeqLoopIndexInParallelOrTaskConstruct(const parser::Name &); bool IsNestedInDirective(llvm::omp::Directive directive); @@ -2068,7 +2067,7 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPLoopConstruct &x) { } } - PrivatizeAssociatedLoopIndexAndCheckLoopLevel(x); + PrivatizeAssociatedLoopIndex(x); return true; } @@ -2162,13 +2161,16 @@ bool OmpAttributeVisitor::Pre(const parser::DoConstruct &x) { // increment of the associated do-loop (only for OpenMP versions <= 4.5) // - The loop iteration variables in the associated do-loops of a simd // construct with multiple associated do-loops are lastprivate. -void OmpAttributeVisitor::PrivatizeAssociatedLoopIndexAndCheckLoopLevel( +void OmpAttributeVisitor::PrivatizeAssociatedLoopIndex( const parser::OpenMPLoopConstruct &x) { + const parser::OmpDirectiveSpecification &spec{x.BeginDir()}; unsigned version{context_.langOptions().OpenMPVersion}; + auto [depth, _]{ - omp::GetAffectedNestDepthWithReason(x.BeginDir(), version, &context_)}; - // If there was a problem obtaining the depth, it will be diagnosed in - // the semantic checks. + omp::GetAffectedNestDepthWithReason(spec, version, &context_)}; + + // If depth is absent, then there is some issue. Leave it alone here, + // and let the semantic checks diagnose the problem. if (!depth || *depth.value <= 0) { return; } @@ -2183,35 +2185,30 @@ void OmpAttributeVisitor::PrivatizeAssociatedLoopIndexAndCheckLoopLevel( ivDSA = Symbol::Flag::OmpLastPrivate; } - for (auto &construct : std::get(x.t)) { - if (const auto *innermostConstruct{parser::omp::GetOmpLoop(construct)}) { - PrivatizeAssociatedLoopIndexAndCheckLoopLevel(*innermostConstruct); - } else if (const auto *doConstruct{ - parser::omp::GetDoConstruct(construct)}) { - for (const parser::DoConstruct *loop{&*doConstruct}; loop && level > 0; - --level) { - // go through all the nested do-loops and resolve index variables - if (const parser::Name *iv{GetLoopIndex(*loop)}) { - if (!iv->symbol || !IsLocalInsideScope(*iv->symbol, currScope())) { - if (auto *symbol{ResolveOmp(*iv, ivDSA, currScope())}) { - if (const auto *details{ - iv->symbol->detailsIf()}) { - const Symbol *tpSymbol = &details->symbol(); - if (tpSymbol->test(Symbol::Flag::OmpThreadprivate)) { - context_.Say(iv->source, - "Loop iteration variable %s is not allowed in THREADPRIVATE."_err_en_US, - iv->ToString()); - } - } - SetSymbolDSA(*symbol, {Symbol::Flag::OmpPreDetermined, ivDSA}); - iv->symbol = symbol; // adjust the symbol within region - AddToContextObjectWithDSA(*symbol, ivDSA); - } - } - const auto &block{std::get(loop->t)}; - const auto it{block.begin()}; - loop = it != block.end() ? GetDoConstructIf(*it) : nullptr; - } + auto checkThreadprivate{[&](const parser::Name &iv) { + if (const auto *details{iv.symbol->detailsIf()}) { + if (details->symbol().test(Symbol::Flag::OmpThreadprivate)) { + context_.Say(iv.source, + "Loop iteration variable %s is not allowed in THREADPRIVATE."_err_en_US, + iv.ToString()); + } + } + }}; + + Scope &scope{currScope()}; + + if (auto doLoops{omp::CollectAffectedDoLoops(x, version, &context_)}) { + for (const parser::DoConstruct *loop : *doLoops) { + const parser::Name *iv{GetLoopIndex(*loop)}; + if (!iv || (iv->symbol && IsLocalInsideScope(*iv->symbol, scope))) { + continue; + } + + if (auto *symbol{ResolveOmp(*iv, ivDSA, scope)}) { + checkThreadprivate(*iv); + SetSymbolDSA(*symbol, {Symbol::Flag::OmpPreDetermined, ivDSA}); + iv->symbol = symbol; // adjust the symbol within region + AddToContextObjectWithDSA(*symbol, ivDSA); } } } diff --git a/flang/test/Semantics/OpenMP/affected-loops.f90 b/flang/test/Semantics/OpenMP/affected-loops.f90 new file mode 100644 index 0000000000000..01273ec4725ec --- /dev/null +++ b/flang/test/Semantics/OpenMP/affected-loops.f90 @@ -0,0 +1,27 @@ +!RUN: %flang_fc1 -fdebug-dump-symbols -fopenmp -fopenmp-version=60 %s | FileCheck %s + +subroutine f + integer :: i, j, k + !$omp do collapse(5) + !$omp tile sizes(2, 2) + do i = 1, 10 + do j = 1, 10 + do k = 1, 10 + end do + end do + end do +end + +! Check that k is privatized in the scope of DO, and that i, j are privatized +! in the scope of TILE. + +!CHECK: Subprogram scope: f size=12 alignment=4 sourceRange=139 bytes +!CHECK: f (Subroutine): HostAssoc => f (Subroutine): Subprogram () +!CHECK: i size=4 offset=0: ObjectEntity type: INTEGER(4) +!CHECK: j size=4 offset=4: ObjectEntity type: INTEGER(4) +!CHECK: k size=4 offset=8: ObjectEntity type: INTEGER(4) +!CHECK: OtherConstruct scope: size=0 alignment=1 sourceRange=98 bytes +!CHECK: k (OmpPrivate, OmpPreDetermined): HostAssoc => k size=4 offset=8: ObjectEntity type: INTEGER(4) +!CHECK: OtherConstruct scope: size=0 alignment=1 sourceRange=77 bytes +!CHECK: i (OmpPrivate, OmpPreDetermined): HostAssoc => i size=4 offset=0: ObjectEntity type: INTEGER(4) +!CHECK: j (OmpPrivate, OmpPreDetermined): HostAssoc => j size=4 offset=4: ObjectEntity type: INTEGER(4)