diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp index 6b8f709be2161..3dedd4864bafc 100644 --- a/flang/lib/Lower/OpenMP/Clauses.cpp +++ b/flang/lib/Lower/OpenMP/Clauses.cpp @@ -574,20 +574,17 @@ Defaultmap make(const parser::OmpClause::Defaultmap &inp, /*VariableCategory=*/maybeApply(convert2, t1)}}; } -Depend make(const parser::OmpClause::Depend &inp, - semantics::SemanticsContext &semaCtx) { - // inp.v -> parser::OmpDependClause - using wrapped = parser::OmpDependClause; - using Variant = decltype(Depend::u); +Doacross makeDoacross(const parser::OmpDoacross &doa, + semantics::SemanticsContext &semaCtx) { // Iteration is the equivalent of parser::OmpIteration using Iteration = Doacross::Vector::value_type; // LoopIterationT - auto visitSource = [&](const parser::OmpDoacross::Source &) -> Variant { + auto visitSource = [&](const parser::OmpDoacross::Source &) { return Doacross{{/*DependenceType=*/Doacross::DependenceType::Source, /*Vector=*/{}}}; }; - auto visitSink = [&](const parser::OmpDoacross::Sink &s) -> Variant { + auto visitSink = [&](const parser::OmpDoacross::Sink &s) { using IterOffset = parser::OmpIterationOffset; auto convert2 = [&](const parser::OmpIteration &v) { auto &t0 = std::get(v.t); @@ -605,6 +602,15 @@ Depend make(const parser::OmpClause::Depend &inp, /*Vector=*/makeList(s.v.v, convert2)}}; }; + return common::visit(common::visitors{visitSink, visitSource}, doa.u); +} + +Depend make(const parser::OmpClause::Depend &inp, + semantics::SemanticsContext &semaCtx) { + // inp.v -> parser::OmpDependClause + using wrapped = parser::OmpDependClause; + using Variant = decltype(Depend::u); + auto visitTaskDep = [&](const wrapped::TaskDep &s) -> Variant { auto &t0 = std::get>(s.t); auto &t1 = std::get(s.t); @@ -617,11 +623,11 @@ Depend make(const parser::OmpClause::Depend &inp, /*LocatorList=*/makeObjects(t2, semaCtx)}}; }; - return Depend{Fortran::common::visit( // + return Depend{common::visit( // common::visitors{ // Doacross [&](const parser::OmpDoacross &s) -> Variant { - return common::visit(common::visitors{visitSink, visitSource}, s.u); + return makeDoacross(s, semaCtx); }, // Depend::TaskDep visitTaskDep, @@ -692,8 +698,8 @@ DistSchedule make(const parser::OmpClause::DistSchedule &inp, Doacross make(const parser::OmpClause::Doacross &inp, semantics::SemanticsContext &semaCtx) { - // inp -> empty - llvm_unreachable("Empty: doacross"); + // inp.v -> OmpDoacrossClause + return makeDoacross(inp.v.v, semaCtx); } // DynamicAllocators: empty diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index 23e1cf8f6b5a6..dc90b4cccabd2 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -575,6 +575,7 @@ void OmpStructureChecker::Leave(const parser::OpenMPConstruct &) { } void OmpStructureChecker::Enter(const parser::OpenMPLoopConstruct &x) { + loopStack_.push_back(&x); const auto &beginLoopDir{std::get(x.t)}; const auto &beginDir{std::get(beginLoopDir.t)}; @@ -968,11 +969,19 @@ void OmpStructureChecker::CheckDistLinear( } } -void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &) { +void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &x) { if (llvm::omp::allSimdSet.test(GetContext().directive)) { ExitDirectiveNest(SIMDNest); } dirContext_.pop_back(); + + assert(!loopStack_.empty() && "Expecting non-empty loop stack"); + const LoopConstruct &top{loopStack_.back()}; +#ifndef NDEBUG + auto *loopc{std::get_if(&top)}; + assert(loopc != nullptr && *loopc == &x && "Mismatched loop constructs"); +#endif + loopStack_.pop_back(); } void OmpStructureChecker::Enter(const parser::OmpEndLoopDirective &x) { @@ -1103,8 +1112,7 @@ void OmpStructureChecker::Leave(const parser::OpenMPBlockConstruct &) { void OmpStructureChecker::ChecksOnOrderedAsBlock() { if (FindClause(llvm::omp::Clause::OMPC_depend)) { context_.Say(GetContext().clauseSource, - "DEPEND(*) clauses are not allowed when ORDERED construct is a block" - " construct with an ORDERED region"_err_en_US); + "DEPEND clauses are not allowed when ORDERED construct is a block construct with an ORDERED region"_err_en_US); return; } @@ -1654,15 +1662,14 @@ void OmpStructureChecker::ChecksOnOrderedAsStandalone() { if (FindClause(llvm::omp::Clause::OMPC_threads) || FindClause(llvm::omp::Clause::OMPC_simd)) { context_.Say(GetContext().clauseSource, - "THREADS, SIMD clauses are not allowed when ORDERED construct is a " - "standalone construct with no ORDERED region"_err_en_US); + "THREADS and SIMD clauses are not allowed when ORDERED construct is a standalone construct with no ORDERED region"_err_en_US); } int dependSinkCount{0}, dependSourceCount{0}; bool exclusiveShown{false}, duplicateSourceShown{false}; - auto visitDoacross = [&](const parser::OmpDoacross &doa, - const parser::CharBlock &src) { + auto visitDoacross{[&](const parser::OmpDoacross &doa, + const parser::CharBlock &src) { common::visit( common::visitors{ [&](const parser::OmpDoacross::Source &) { dependSourceCount++; }, @@ -1678,10 +1685,11 @@ void OmpStructureChecker::ChecksOnOrderedAsStandalone() { context_.Say(src, "At most one SOURCE dependence type can appear on the ORDERED directive"_err_en_US); } - }; + }}; - auto clauseAll = FindClauses(llvm::omp::Clause::OMPC_depend); - for (auto itr = clauseAll.first; itr != clauseAll.second; ++itr) { + // Visit the DEPEND and DOACROSS clauses. + auto depClauses{FindClauses(llvm::omp::Clause::OMPC_depend)}; + for (auto itr{depClauses.first}; itr != depClauses.second; ++itr) { const auto &dependClause{ std::get(itr->second->u)}; if (auto *doAcross{std::get_if(&dependClause.v.u)}) { @@ -1691,6 +1699,11 @@ void OmpStructureChecker::ChecksOnOrderedAsStandalone() { "Only SINK or SOURCE dependence types are allowed when ORDERED construct is a standalone construct with no ORDERED region"_err_en_US); } } + auto doaClauses{FindClauses(llvm::omp::Clause::OMPC_doacross)}; + for (auto itr{doaClauses.first}; itr != doaClauses.second; ++itr) { + auto &doaClause{std::get(itr->second->u)}; + visitDoacross(doaClause.v.v, itr->second->source); + } bool isNestedInDoOrderedWithPara{false}; if (CurrentDirectiveIsNested() && @@ -1718,8 +1731,8 @@ void OmpStructureChecker::ChecksOnOrderedAsStandalone() { void OmpStructureChecker::CheckOrderedDependClause( std::optional orderedValue) { - auto visitDoacross = [&](const parser::OmpDoacross &doa, - const parser::CharBlock &src) { + auto visitDoacross{[&](const parser::OmpDoacross &doa, + const parser::CharBlock &src) { if (auto *sinkVector{std::get_if(&doa.u)}) { int64_t numVar = sinkVector->v.v.size(); if (orderedValue != numVar) { @@ -1727,14 +1740,19 @@ void OmpStructureChecker::CheckOrderedDependClause( "The number of variables in the SINK iteration vector does not match the parameter specified in ORDERED clause"_err_en_US); } } - }; - auto clauseAll{FindClauses(llvm::omp::Clause::OMPC_depend)}; - for (auto itr = clauseAll.first; itr != clauseAll.second; ++itr) { + }}; + auto depClauses{FindClauses(llvm::omp::Clause::OMPC_depend)}; + for (auto itr{depClauses.first}; itr != depClauses.second; ++itr) { auto &dependClause{std::get(itr->second->u)}; if (auto *doAcross{std::get_if(&dependClause.v.u)}) { visitDoacross(*doAcross, itr->second->source); } } + auto doaClauses = FindClauses(llvm::omp::Clause::OMPC_doacross); + for (auto itr{doaClauses.first}; itr != doaClauses.second; ++itr) { + auto &doaClause{std::get(itr->second->u)}; + visitDoacross(doaClause.v.v, itr->second->source); + } } void OmpStructureChecker::CheckTargetUpdate() { @@ -2712,7 +2730,6 @@ CHECK_SIMPLE_CLAUSE(Bind, OMPC_bind) CHECK_SIMPLE_CLAUSE(Align, OMPC_align) CHECK_SIMPLE_CLAUSE(Compare, OMPC_compare) CHECK_SIMPLE_CLAUSE(CancellationConstructType, OMPC_cancellation_construct_type) -CHECK_SIMPLE_CLAUSE(Doacross, OMPC_doacross) CHECK_SIMPLE_CLAUSE(OmpxAttribute, OMPC_ompx_attribute) CHECK_SIMPLE_CLAUSE(OmpxBare, OMPC_ompx_bare) CHECK_SIMPLE_CLAUSE(Fail, OMPC_fail) @@ -3493,6 +3510,7 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Depend &x) { "Unexpected alternative in update clause"); if (doaDep) { + CheckDoacross(*doaDep); CheckDependenceType(doaDep->GetDepType()); } else { CheckTaskDependenceType(taskDep->GetTaskDepType()); @@ -3572,6 +3590,93 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Depend &x) { } } +void OmpStructureChecker::Enter(const parser::OmpClause::Doacross &x) { + CheckAllowedClause(llvm::omp::Clause::OMPC_doacross); + CheckDoacross(x.v.v); +} + +void OmpStructureChecker::CheckDoacross(const parser::OmpDoacross &doa) { + if (std::holds_alternative(doa.u)) { + // Nothing to check here. + return; + } + + // Process SINK dependence type. SINK may only appear in an ORDER construct, + // which references a prior ORDERED(n) clause on a DO or SIMD construct + // that marks the top of the loop nest. + + auto &sink{std::get(doa.u)}; + const std::list &vec{sink.v.v}; + + // Check if the variables in the iteration vector are unique. + struct Less { + bool operator()( + const parser::OmpIteration *a, const parser::OmpIteration *b) const { + auto namea{std::get(a->t)}; + auto nameb{std::get(b->t)}; + assert(namea.symbol && nameb.symbol && "Unresolved symbols"); + // The non-determinism of the "<" doesn't matter, we only care about + // equality, i.e. a == b <=> !(a < b) && !(b < a) + return reinterpret_cast(namea.symbol) < + reinterpret_cast(nameb.symbol); + } + }; + if (auto *duplicate{FindDuplicateEntry(vec)}) { + auto name{std::get(duplicate->t)}; + context_.Say(name.source, + "Duplicate variable '%s' in the iteration vector"_err_en_US, + name.ToString()); + } + + // Check if the variables in the iteration vector are induction variables. + // Ignore any mismatch between the size of the iteration vector and the + // number of DO constructs on the stack. This is checked elsewhere. + + auto GetLoopDirective{[](const parser::OpenMPLoopConstruct &x) { + auto &begin{std::get(x.t)}; + return std::get(begin.t).v; + }}; + auto GetLoopClauses{[](const parser::OpenMPLoopConstruct &x) + -> const std::list & { + auto &begin{std::get(x.t)}; + return std::get(begin.t).v; + }}; + + std::set inductionVars; + for (const LoopConstruct &loop : llvm::reverse(loopStack_)) { + if (auto *doc{std::get_if(&loop)}) { + // Do-construct, collect the induction variable. + if (auto &control{(*doc)->GetLoopControl()}) { + if (auto *b{std::get_if(&control->u)}) { + inductionVars.insert(b->name.thing.symbol); + } + } + } else { + // Omp-loop-construct, check if it's do/simd with an ORDERED clause. + auto *loopc{std::get_if(&loop)}; + assert(loopc && "Expecting OpenMPLoopConstruct"); + llvm::omp::Directive loopDir{GetLoopDirective(**loopc)}; + if (loopDir == llvm::omp::OMPD_do || loopDir == llvm::omp::OMPD_simd) { + auto IsOrdered{[](const parser::OmpClause &c) { + return c.Id() == llvm::omp::OMPC_ordered; + }}; + // If it has ORDERED clause, stop the traversal. + if (llvm::any_of(GetLoopClauses(**loopc), IsOrdered)) { + break; + } + } + } + } + for (const parser::OmpIteration &iter : vec) { + auto &name{std::get(iter.t)}; + if (!inductionVars.count(name.symbol)) { + context_.Say(name.source, + "The iteration vector element '%s' is not an induction variable within the ORDERED loop nest"_err_en_US, + name.ToString()); + } + } +} + void OmpStructureChecker::CheckCopyingPolymorphicAllocatable( SymbolSourceMap &symbols, const llvm::omp::Clause clause) { if (context_.ShouldWarn(common::UsageWarning::Portability)) { @@ -4326,6 +4431,22 @@ void OmpStructureChecker::Enter( CheckAllowedRequiresClause(llvm::omp::Clause::OMPC_unified_shared_memory); } +void OmpStructureChecker::Enter(const parser::DoConstruct &x) { + Base::Enter(x); + loopStack_.push_back(&x); +} + +void OmpStructureChecker::Leave(const parser::DoConstruct &x) { + assert(!loopStack_.empty() && "Expecting non-empty loop stack"); + const LoopConstruct &top = loopStack_.back(); +#ifndef NDEBUG + auto *doc{std::get_if(&top)}; + assert(doc != nullptr && *doc == &x && "Mismatched loop constructs"); +#endif + loopStack_.pop_back(); + Base::Leave(x); +} + void OmpStructureChecker::CheckAllowedRequiresClause(llvmOmpClause clause) { CheckAllowedClause(clause); diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h index 2998b4f054931..8c13dd20d1e39 100644 --- a/flang/lib/Semantics/check-omp-structure.h +++ b/flang/lib/Semantics/check-omp-structure.h @@ -60,6 +60,9 @@ class OmpStructureChecker : public DirectiveStructureChecker { public: + using Base = DirectiveStructureChecker; + OmpStructureChecker(SemanticsContext &context) : DirectiveStructureChecker(context, #define GEN_FLANG_DIRECTIVE_CLAUSE_MAP @@ -131,6 +134,9 @@ class OmpStructureChecker void Enter(const parser::OmpAtomicCapture &); void Leave(const parser::OmpAtomic &); + void Enter(const parser::DoConstruct &); + void Leave(const parser::DoConstruct &); + #define GEN_FLANG_CLAUSE_CHECK_ENTER #include "llvm/Frontend/OpenMP/OMP.inc" @@ -157,13 +163,19 @@ class OmpStructureChecker const parser::OmpScheduleModifierType::ModType &); void CheckAllowedMapTypes(const parser::OmpMapClause::Type &, const std::list &); - template const T *FindDuplicateEntry(const std::list &); llvm::StringRef getClauseName(llvm::omp::Clause clause) override; llvm::StringRef getDirectiveName(llvm::omp::Directive directive) override; + template struct DefaultLess { + bool operator()(const T *a, const T *b) const { return *a < *b; } + }; + template > + const T *FindDuplicateEntry(const std::list &); + void CheckDependList(const parser::DataRef &); void CheckDependArraySection( const common::Indirection &, const parser::Name &); + void CheckDoacross(const parser::OmpDoacross &doa); bool IsDataRefTypeParamInquiry(const parser::DataRef *dataRef); void CheckIsVarPartOfAnotherVar(const parser::CharBlock &source, const parser::OmpObjectList &objList, llvm::StringRef clause = ""); @@ -255,9 +267,13 @@ class OmpStructureChecker int directiveNest_[LastType + 1] = {0}; SymbolSourceMap deferredNonVariables_; + + using LoopConstruct = std::variant; + std::vector loopStack_; }; -template +template const T *OmpStructureChecker::FindDuplicateEntry(const std::list &list) { // Add elements of the list to a set. If the insertion fails, return // the address of the failing element. @@ -265,10 +281,7 @@ const T *OmpStructureChecker::FindDuplicateEntry(const std::list &list) { // The objects of type T may not be copyable, so add their addresses // to the set. The set will need to compare the actual objects, so // the custom comparator is provided. - struct less { - bool operator()(const T *a, const T *b) const { return *a < *b; } - }; - std::set uniq; + std::set uniq; for (const T &item : list) { if (!uniq.insert(&item).second) { diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index 4e6ea4f266016..83d666283a48c 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -554,8 +554,16 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor { } void Post(const parser::OmpIteration &x) { - const auto &name{std::get(x.t)}; - ResolveName(&name); + if (const auto &name{std::get(x.t)}; !name.symbol) { + auto *symbol{currScope().FindSymbol(name.source)}; + if (!symbol) { + // OmpIteration must use an existing object. If there isn't one, + // create a fake one and flag an error later. + symbol = &currScope().MakeSymbol( + name.source, Attrs{}, EntityDetails(/*isDummy=*/true)); + } + Resolve(name, symbol); + } } bool Pre(const parser::OmpClause::UseDevicePtr &x) { diff --git a/flang/test/Lower/OpenMP/Todo/ordered.f90 b/flang/test/Lower/OpenMP/Todo/ordered.f90 new file mode 100644 index 0000000000000..2f91e5ed28a1a --- /dev/null +++ b/flang/test/Lower/OpenMP/Todo/ordered.f90 @@ -0,0 +1,20 @@ +!RUN: %not_todo_cmd bbc -emit-hlfir -fopenmp -fopenmp-version=52 -o - %s 2>&1 | FileCheck %s +!RUN: %not_todo_cmd %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=52 -o - %s 2>&1 | FileCheck %s + +!CHECK: not yet implemented: OMPD_ordered +subroutine f00(x) + integer :: a(10) + + do i = 1, 10 + !$omp do ordered(3) + do j = 1, 10 + do k = 1, 10 + do m = 1, 10 + !$omp ordered doacross(sink: m+1, k+0, j-2) + a(i) = i + enddo + enddo + enddo + !$omp end do + enddo +end diff --git a/flang/test/Semantics/OpenMP/doacross.f90 b/flang/test/Semantics/OpenMP/doacross.f90 new file mode 100644 index 0000000000000..381a4118ce7bf --- /dev/null +++ b/flang/test/Semantics/OpenMP/doacross.f90 @@ -0,0 +1,28 @@ +!RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=52 + +subroutine f00(x) + integer :: x(10, 10) + !$omp do ordered(2) + do i = 1, 10 + do j = 1, 10 +!ERROR: Duplicate variable 'i' in the iteration vector + !$omp ordered doacross(sink: i+1, i-2) + x(i, j) = 0 + enddo + enddo + !$omp end do +end + +subroutine f01(x) + integer :: x(10, 10) + do i = 1, 10 + !$omp do ordered(1) + do j = 1, 10 +!ERROR: The iteration vector element 'i' is not an induction variable within the ORDERED loop nest + !$omp ordered doacross(sink: i+1) + x(i, j) = 0 + enddo + !$omp end do + enddo +end + diff --git a/flang/test/Semantics/OpenMP/ordered01.f90 b/flang/test/Semantics/OpenMP/ordered01.f90 index 9f3a258d470a6..12543acb2916b 100644 --- a/flang/test/Semantics/OpenMP/ordered01.f90 +++ b/flang/test/Semantics/OpenMP/ordered01.f90 @@ -54,11 +54,11 @@ program main !$omp do ordered(1) do i = 2, N - !ERROR: DEPEND(*) clauses are not allowed when ORDERED construct is a block construct with an ORDERED region + !ERROR: DEPEND clauses are not allowed when ORDERED construct is a block construct with an ORDERED region !$omp ordered depend(source) arrayA(i) = foo(i) !$omp end ordered - !ERROR: DEPEND(*) clauses are not allowed when ORDERED construct is a block construct with an ORDERED region + !ERROR: DEPEND clauses are not allowed when ORDERED construct is a block construct with an ORDERED region !$omp ordered depend(sink: i - 1) arrayB(i) = bar(arrayA(i), arrayB(i-1)) !$omp end ordered @@ -67,12 +67,12 @@ program main contains subroutine work1() - !ERROR: THREADS, SIMD clauses are not allowed when ORDERED construct is a standalone construct with no ORDERED region + !ERROR: THREADS and SIMD clauses are not allowed when ORDERED construct is a standalone construct with no ORDERED region !$omp ordered simd end subroutine work1 subroutine work2() - !ERROR: THREADS, SIMD clauses are not allowed when ORDERED construct is a standalone construct with no ORDERED region + !ERROR: THREADS and SIMD clauses are not allowed when ORDERED construct is a standalone construct with no ORDERED region !$omp ordered threads end subroutine work2 diff --git a/flang/test/Semantics/OpenMP/ordered03.f90 b/flang/test/Semantics/OpenMP/ordered03.f90 index e96d4557f8f18..6a7037e2b750c 100644 --- a/flang/test/Semantics/OpenMP/ordered03.f90 +++ b/flang/test/Semantics/OpenMP/ordered03.f90 @@ -100,6 +100,7 @@ subroutine sub1() !$omp do ordered(1) do i = 1, N !ERROR: The number of variables in the SINK iteration vector does not match the parameter specified in ORDERED clause + !ERROR: The iteration vector element 'j' is not an induction variable within the ORDERED loop nest !$omp ordered depend(sink: i - 1) depend(sink: i - 1, j) arrayB(i) = bar(i - 1, j) end do @@ -119,5 +120,6 @@ subroutine sub1() !$omp ordered depend(source) !ERROR: An ORDERED construct with the DEPEND clause must be closely nested in a worksharing-loop (or parallel worksharing-loop) construct with ORDERED clause with a parameter + !ERROR: The iteration vector element 'i' is not an induction variable within the ORDERED loop nest !$omp ordered depend(sink: i - 1) end