From c4a850e5d8c6d12160fa36594dfe5af471598de6 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Thu, 7 Nov 2024 14:24:37 -0600 Subject: [PATCH 1/8] [flang][OpenMP] Parse DOACROSS clause Extract the SINK/SOURCE parse tree elements into a separate class `OmpDoacross`, share them between DEPEND and DOACROSS clauses. Most of the changes in Semantics are to accommodate the new contents of OmpDependClause, and a mere introduction of OmpDoacrossClause. There are no semantic checks specifically for DOACROSS. --- flang/include/flang/Parser/dump-parse-tree.h | 19 +- flang/include/flang/Parser/parse-tree.h | 79 +++++-- flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 30 +-- flang/lib/Lower/OpenMP/Clauses.cpp | 112 +++++----- flang/lib/Lower/OpenMP/Clauses.h | 2 +- flang/lib/Parser/openmp-parsers.cpp | 48 +++-- flang/lib/Parser/parse-tree.cpp | 19 +- flang/lib/Parser/unparse.cpp | 30 +-- flang/lib/Semantics/check-omp-structure.cpp | 195 ++++++++++-------- flang/lib/Semantics/check-omp-structure.h | 1 + flang/lib/Semantics/resolve-directives.cpp | 4 +- flang/test/Parser/OpenMP/depobj-construct.f90 | 2 +- flang/test/Parser/OpenMP/doacross-clause.f90 | 90 ++++++++ flang/test/Parser/OpenMP/ordered-depend.f90 | 90 ++++++++ .../Semantics/OpenMP/clause-validity01.f90 | 2 +- flang/test/Semantics/OpenMP/depend06.f90 | 4 +- .../Semantics/OpenMP/depobj-construct-v50.f90 | 2 +- .../Semantics/OpenMP/depobj-construct-v51.f90 | 4 +- .../Semantics/OpenMP/depobj-construct-v52.f90 | 4 +- flang/test/Semantics/OpenMP/ordered01.f90 | 13 +- flang/test/Semantics/OpenMP/ordered03.f90 | 4 +- llvm/include/llvm/Frontend/OpenMP/ClauseT.h | 17 +- llvm/include/llvm/Frontend/OpenMP/OMP.td | 1 + 23 files changed, 517 insertions(+), 255 deletions(-) create mode 100644 flang/test/Parser/OpenMP/doacross-clause.f90 create mode 100644 flang/test/Parser/OpenMP/ordered-depend.f90 diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h index bfeb23de53539..f7730141ecf92 100644 --- a/flang/include/flang/Parser/dump-parse-tree.h +++ b/flang/include/flang/Parser/dump-parse-tree.h @@ -509,15 +509,20 @@ class ParseTreeDumper { NODE(parser, OmpDefaultmapClause) NODE_ENUM(OmpDefaultmapClause, ImplicitBehavior) NODE_ENUM(OmpDefaultmapClause, VariableCategory) - NODE(parser, OmpDependClause) - NODE(parser, OmpDetachClause) - NODE(OmpDependClause, InOut) - NODE(OmpDependClause, Sink) - NODE(OmpDependClause, Source) + NODE(parser, OmpDependenceType) + NODE_ENUM(OmpDependenceType, Type) NODE(parser, OmpTaskDependenceType) NODE_ENUM(OmpTaskDependenceType, Type) - NODE(parser, OmpDependSinkVec) - NODE(parser, OmpDependSinkVecLength) + NODE(parser, OmpIterationOffset) + NODE(parser, OmpIteration) + NODE(parser, OmpIterationVector) + NODE(parser, OmpDoacross) + NODE(OmpDoacross, Sink) + NODE(OmpDoacross, Source) + NODE(parser, OmpDependClause) + NODE(OmpDependClause, TaskDep) + NODE(parser, OmpDetachClause) + NODE(parser, OmpDoacrossClause) NODE(parser, OmpDestroyClause) NODE(parser, OmpEndAllocators) NODE(parser, OmpEndAtomic) diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index d2c5b45d99581..e0369426364ed 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -3439,16 +3439,35 @@ struct OmpObject { WRAPPER_CLASS(OmpObjectList, std::list); +// Ref: [4.5:169-170], [5.0:255-256], [5.1:288-289] +// +// dependence-type -> +// SINK | SOURCE | // since 4.5 +// IN | OUT | INOUT | // since 4.5, until 5.1 +// MUTEXINOUTSET | DEPOBJ | // since 5.0, until 5.1 +// INOUTSET // since 5.1, until 5.1 +// +// All of these, except SINK and SOURCE became task-dependence-type in 5.2. +// +// Keeping these two as separate types, since having them all together +// creates conflicts when parsing the DEPEND clause. For DEPEND(SINK: ...), +// the SINK may be parsed as 'task-dependence-type', and the list after +// the ':' would then be parsed as OmpObjectList (instead of the iteration +// vector). This would accept the vector "i, j, k" (although interpreted +// incorrectly), while flagging a syntax error for "i+1, j, k". +struct OmpDependenceType { + ENUM_CLASS(Type, Sink, Source); + WRAPPER_CLASS_BOILERPLATE(OmpDependenceType, Type); +}; + // Ref: [4.5:169-170], [5.0:254-256], [5.1:287-289], [5.2:321] // // task-dependence-type -> // "dependence-type" in 5.1 and before // IN | OUT | INOUT | // since 4.5 -// SOURCE | SINK | // since 4.5, until 5.1 // MUTEXINOUTSET | DEPOBJ | // since 5.0 // INOUTSET // since 5.2 struct OmpTaskDependenceType { - ENUM_CLASS( - Type, In, Out, Inout, Inoutset, Mutexinoutset, Source, Sink, Depobj) + ENUM_CLASS(Type, In, Out, Inout, Inoutset, Mutexinoutset, Depobj) WRAPPER_CLASS_BOILERPLATE(OmpTaskDependenceType, Type); }; @@ -3528,41 +3547,55 @@ struct OmpDefaultmapClause { std::tuple> t; }; -// 2.13.9 depend-vec-length -> +/- non-negative-constant -struct OmpDependSinkVecLength { - TUPLE_CLASS_BOILERPLATE(OmpDependSinkVecLength); +// 2.13.9 iteration-offset -> +/- non-negative-constant +struct OmpIterationOffset { + TUPLE_CLASS_BOILERPLATE(OmpIterationOffset); std::tuple t; }; -// 2.13.9 depend-vec -> induction-variable [depend-vec-length], ... -struct OmpDependSinkVec { - TUPLE_CLASS_BOILERPLATE(OmpDependSinkVec); - std::tuple> t; +// 2.13.9 iteration -> induction-variable [iteration-offset] +struct OmpIteration { + TUPLE_CLASS_BOILERPLATE(OmpIteration); + std::tuple> t; +}; + +WRAPPER_CLASS(OmpIterationVector, std::list); + +// Extract this into a separate structure (instead of having it directly in +// OmpDoacrossClause), so that the context in TYPE_CONTEXT_PARSER can be set +// separately for OmpDependClause and OmpDoacrossClause. +struct OmpDoacross { + OmpDependenceType::Type GetDepType() const; + + WRAPPER_CLASS(Sink, OmpIterationVector); + EMPTY_CLASS(Source); + UNION_CLASS_BOILERPLATE(OmpDoacross); + std::variant u; }; // Ref: [4.5:169-170], [5.0:255-256], [5.1:288-289], [5.2:323-324] // // depend-clause -> // DEPEND(SOURCE) | // since 4.5, until 5.1 -// DEPEND(SINK: depend-vec) | // since 4.5, until 5.1 -// DEPEND([depend-modifier,]dependence-type: locator-list) // since 4.5 +// DEPEND(SINK: iteration-vector) | // since 4.5, until 5.1 +// DEPEND([depend-modifier,] +// task-dependence-type: locator-list) // since 4.5 // // depend-modifier -> iterator-modifier // since 5.0 struct OmpDependClause { - OmpTaskDependenceType::Type GetDepType() const; - UNION_CLASS_BOILERPLATE(OmpDependClause); - EMPTY_CLASS(Source); - WRAPPER_CLASS(Sink, std::list); - struct InOut { - TUPLE_CLASS_BOILERPLATE(InOut); + struct TaskDep { + OmpTaskDependenceType::Type GetTaskDepType() const; + TUPLE_CLASS_BOILERPLATE(TaskDep); std::tuple, OmpTaskDependenceType, OmpObjectList> t; }; - std::variant u; + std::variant u; }; +WRAPPER_CLASS(OmpDoacrossClause, OmpDoacross); + // Ref: [5.0:254-255], [5.1:287-288], [5.2:73] // // destroy-clause -> @@ -3775,8 +3808,12 @@ struct OmpNumTasksClause { // Ref: [5.0:254-255], [5.1:287-288], [5.2:321-322] // -// update-clause -> UPDATE(task-dependence-type) // since 5.0 -WRAPPER_CLASS(OmpUpdateClause, OmpTaskDependenceType); +// update-clause -> UPDATE(dependence-type) // since 5.0, until 5.1 +// update-clause -> UPDATE(task-dependence-type) // since 5.2 +struct OmpUpdateClause { + UNION_CLASS_BOILERPLATE(OmpUpdateClause); + std::variant u; +}; // OpenMP Clauses struct OmpClause { diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp index e768c1cbc0784..72b9018f2d280 100644 --- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp +++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp @@ -122,28 +122,28 @@ genProcBindKindAttr(fir::FirOpBuilder &firOpBuilder, static mlir::omp::ClauseTaskDependAttr genDependKindAttr(lower::AbstractConverter &converter, - const omp::clause::Depend::TaskDependenceType kind) { + const omp::clause::DependenceType kind) { fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder(); mlir::Location currentLocation = converter.getCurrentLocation(); mlir::omp::ClauseTaskDepend pbKind; switch (kind) { - case omp::clause::Depend::TaskDependenceType::In: + case omp::clause::DependenceType::In: pbKind = mlir::omp::ClauseTaskDepend::taskdependin; break; - case omp::clause::Depend::TaskDependenceType::Out: + case omp::clause::DependenceType::Out: pbKind = mlir::omp::ClauseTaskDepend::taskdependout; break; - case omp::clause::Depend::TaskDependenceType::Inout: + case omp::clause::DependenceType::Inout: pbKind = mlir::omp::ClauseTaskDepend::taskdependinout; break; - case omp::clause::Depend::TaskDependenceType::Mutexinoutset: - case omp::clause::Depend::TaskDependenceType::Inoutset: + case omp::clause::DependenceType::Mutexinoutset: + case omp::clause::DependenceType::Inoutset: TODO(currentLocation, "INOUTSET and MUTEXINOUTSET are not supported yet"); break; - case omp::clause::Depend::TaskDependenceType::Depobj: - case omp::clause::Depend::TaskDependenceType::Sink: - case omp::clause::Depend::TaskDependenceType::Source: + case omp::clause::DependenceType::Depobj: + case omp::clause::DependenceType::Sink: + case omp::clause::DependenceType::Source: llvm_unreachable("unhandled parser task dependence type"); break; } @@ -803,20 +803,20 @@ bool ClauseProcessor::processDepend(mlir::omp::DependClauseOps &result) const { auto process = [&](const omp::clause::Depend &clause, const parser::CharBlock &) { using Depend = omp::clause::Depend; - if (!std::holds_alternative(clause.u)) { + if (!std::holds_alternative(clause.u)) { TODO(converter.getCurrentLocation(), "DEPEND clause with SINK or SOURCE is not supported yet"); } - auto &depType = std::get(clause.u); - auto kind = std::get(depType.t); - auto &objects = std::get(depType.t); + auto &taskDep = std::get(clause.u); + auto depType = std::get(taskDep.t); + auto &objects = std::get(taskDep.t); - if (std::get>(depType.t)) { + if (std::get>(taskDep.t)) { TODO(converter.getCurrentLocation(), "Support for iterator modifiers is not implemented yet"); } mlir::omp::ClauseTaskDependAttr dependTypeOperand = - genDependKindAttr(converter, kind); + genDependKindAttr(converter, depType); result.dependKinds.append(objects.size(), dependTypeOperand); for (const omp::Object &object : objects) { diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp index 46caafeef8e4a..f6633dd53f6f2 100644 --- a/flang/lib/Lower/OpenMP/Clauses.cpp +++ b/flang/lib/Lower/OpenMP/Clauses.cpp @@ -338,27 +338,32 @@ ReductionOperator makeReductionOperator(const parser::OmpReductionOperator &inp, inp.u); } -clause::TaskDependenceType -makeDepType(const parser::OmpTaskDependenceType &inp) { +clause::DependenceType makeDepType(const parser::OmpDependenceType &inp) { + switch (inp.v) { + case parser::OmpDependenceType::Type::Sink: + return clause::DependenceType::Sink; + case parser::OmpDependenceType::Type::Source: + return clause::DependenceType::Source; + } + llvm_unreachable("Unexpected dependence type"); +} + +clause::DependenceType makeDepType(const parser::OmpTaskDependenceType &inp) { switch (inp.v) { case parser::OmpTaskDependenceType::Type::Depobj: - return clause::TaskDependenceType::Depobj; + return clause::DependenceType::Depobj; case parser::OmpTaskDependenceType::Type::In: - return clause::TaskDependenceType::In; + return clause::DependenceType::In; case parser::OmpTaskDependenceType::Type::Inout: - return clause::TaskDependenceType::Inout; + return clause::DependenceType::Inout; case parser::OmpTaskDependenceType::Type::Inoutset: - return clause::TaskDependenceType::Inoutset; + return clause::DependenceType::Inoutset; case parser::OmpTaskDependenceType::Type::Mutexinoutset: - return clause::TaskDependenceType::Mutexinoutset; + return clause::DependenceType::Mutexinoutset; case parser::OmpTaskDependenceType::Type::Out: - return clause::TaskDependenceType::Out; - case parser::OmpTaskDependenceType::Type::Sink: - return clause::TaskDependenceType::Sink; - case parser::OmpTaskDependenceType::Type::Source: - return clause::TaskDependenceType::Source; + return clause::DependenceType::Out; } - llvm_unreachable("Unexpected dependence type"); + llvm_unreachable("Unexpected task dependence type"); } // -------------------------------------------------------------------- @@ -574,49 +579,52 @@ Depend make(const parser::OmpClause::Depend &inp, // inp.v -> parser::OmpDependClause using wrapped = parser::OmpDependClause; using Variant = decltype(Depend::u); - // Iteration is the equivalent of parser::OmpDependSinkVec + // Iteration is the equivalent of parser::OmpIteration using Iteration = Doacross::Vector::value_type; // LoopIterationT + auto visitSource = [&](const parser::OmpDoacross::Source &) -> Variant { + return Doacross{{/*DependenceType=*/Doacross::DependenceType::Source, + /*Vector=*/{}}}; + }; + + auto visitSink = [&](const parser::OmpDoacross::Sink &s) -> Variant { + using IterOffset = parser::OmpIterationOffset; + auto convert2 = [&](const parser::OmpIteration &v) { + auto &t0 = std::get(v.t); + auto &t1 = std::get>(v.t); + + auto convert3 = [&](const IterOffset &u) { + auto &s0 = std::get(u.t); + auto &s1 = std::get(u.t); + return Iteration::Distance{ + {makeDefinedOperator(s0, semaCtx), makeExpr(s1, semaCtx)}}; + }; + return Iteration{{makeObject(t0, semaCtx), maybeApply(convert3, t1)}}; + }; + return Doacross{{/*DependenceType=*/Doacross::DependenceType::Sink, + /*Vector=*/makeList(s.v.v, convert2)}}; + }; + + auto visitTaskDep = [&](const wrapped::TaskDep &s) -> Variant { + auto &t0 = std::get>(s.t); + auto &t1 = std::get(s.t); + auto &t2 = std::get(s.t); + + auto &&maybeIter = + maybeApply([&](auto &&s) { return makeIterator(s, semaCtx); }, t0); + return Depend::TaskDep{{/*DependenceType=*/makeDepType(t1), + /*Iterator=*/std::move(maybeIter), + /*LocatorList=*/makeObjects(t2, semaCtx)}}; + }; + return Depend{Fortran::common::visit( // common::visitors{ // Doacross - [&](const wrapped::Source &s) -> Variant { - return Doacross{ - {/*DependenceType=*/Doacross::DependenceType::Source, - /*Vector=*/{}}}; - }, - // Doacross - [&](const wrapped::Sink &s) -> Variant { - using DependLength = parser::OmpDependSinkVecLength; - auto convert2 = [&](const parser::OmpDependSinkVec &v) { - auto &t0 = std::get(v.t); - auto &t1 = std::get>(v.t); - - auto convert3 = [&](const DependLength &u) { - auto &s0 = std::get(u.t); - auto &s1 = std::get(u.t); - return Iteration::Distance{ - {makeDefinedOperator(s0, semaCtx), makeExpr(s1, semaCtx)}}; - }; - return Iteration{ - {makeObject(t0, semaCtx), maybeApply(convert3, t1)}}; - }; - return Doacross{{/*DependenceType=*/Doacross::DependenceType::Sink, - /*Vector=*/makeList(s.v, convert2)}}; - }, - // Depend::DepType - [&](const wrapped::InOut &s) -> Variant { - auto &t0 = - std::get>(s.t); - auto &t1 = std::get(s.t); - auto &t2 = std::get(s.t); - - auto &&maybeIter = maybeApply( - [&](auto &&s) { return makeIterator(s, semaCtx); }, t0); - return Depend::DepType{{/*TaskDependenceType=*/makeDepType(t1), - /*Iterator=*/std::move(maybeIter), - /*LocatorList=*/makeObjects(t2, semaCtx)}}; + [&](const parser::OmpDoacross &s) -> Variant { + return common::visit(common::visitors{visitSink, visitSource}, s.u); }, + // Depend::TaskDep + visitTaskDep, }, inp.v.u)}; } @@ -1356,7 +1364,9 @@ Uniform make(const parser::OmpClause::Uniform &inp, Update make(const parser::OmpClause::Update &inp, semantics::SemanticsContext &semaCtx) { // inp.v -> parser::OmpUpdateClause - return Update{/*TaskDependenceType=*/makeDepType(inp.v.v)}; + auto depType = + common::visit([](auto &&s) { return makeDepType(s); }, inp.v.u); + return Update{/*DependenceType=*/depType}; } Use make(const parser::OmpClause::Use &inp, diff --git a/flang/lib/Lower/OpenMP/Clauses.h b/flang/lib/Lower/OpenMP/Clauses.h index 51180ebfe5745..514f0d1ee466a 100644 --- a/flang/lib/Lower/OpenMP/Clauses.h +++ b/flang/lib/Lower/OpenMP/Clauses.h @@ -152,7 +152,7 @@ using IteratorSpecifier = tomp::type::IteratorSpecifierT; using DefinedOperator = tomp::type::DefinedOperatorT; using ProcedureDesignator = tomp::type::ProcedureDesignatorT; using ReductionOperator = tomp::type::ReductionIdentifierT; -using TaskDependenceType = tomp::type::TaskDependenceType; +using DependenceType = tomp::type::DependenceType; // "Requires" clauses are handled early on, and the aggregated information // is stored in the Symbol details of modules, programs, and subprograms. diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp index 7a0ecc59a2c5c..1a0f8acae4948 100644 --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -392,12 +392,9 @@ TYPE_PARSER(construct( ":"), Parser{})) -// 2.13.9 DEPEND (SOURCE | SINK : vec | (IN | OUT | INOUT) : list -TYPE_PARSER(construct( - Parser{}, scalarIntConstantExpr)) - -TYPE_PARSER( - construct(name, maybe(Parser{}))) +TYPE_PARSER(construct( + "SINK" >> pure(OmpDependenceType::Type::Sink) || + "SOURCE" >> pure(OmpDependenceType::Type::Source))) TYPE_PARSER(construct( "DEPOBJ" >> pure(OmpTaskDependenceType::Type::Depobj) || @@ -405,18 +402,31 @@ TYPE_PARSER(construct( "INOUT"_id >> pure(OmpTaskDependenceType::Type::Inout) || "INOUTSET"_id >> pure(OmpTaskDependenceType::Type::Inoutset) || "MUTEXINOUTSET" >> pure(OmpTaskDependenceType::Type::Mutexinoutset) || - "OUT" >> pure(OmpTaskDependenceType::Type::Out) || - "SINK" >> pure(OmpTaskDependenceType::Type::Sink) || - "SOURCE" >> pure(OmpTaskDependenceType::Type::Source))) + "OUT" >> pure(OmpTaskDependenceType::Type::Out))) + +// iteration-offset -> +/- non-negative-constant-expr +TYPE_PARSER(construct( + Parser{}, scalarIntConstantExpr)) + +// iteration -> iteration-variable [+/- nonnegative-scalar-integer-constant] +TYPE_PARSER(construct(name, maybe(Parser{}))) + +TYPE_PARSER(construct(nonemptyList(Parser{}))) + +TYPE_PARSER(construct( + construct(construct( + "SINK"_tok >> ":"_tok >> Parser{})) || + construct(construct("SOURCE"_tok)))) TYPE_CONTEXT_PARSER("Omp Depend clause"_en_US, - construct(construct( - "SINK :" >> nonemptyList(Parser{}))) || - construct( - construct("SOURCE"_tok)) || - construct(construct( + construct( + construct(construct( maybe(Parser{} / ","_tok), - Parser{} / ":", Parser{}))) + Parser{} / ":", Parser{})) || + construct(Parser{}))) + +TYPE_CONTEXT_PARSER("Omp Doacross clause"_en_US, + construct(Parser{})) TYPE_PARSER(construct( "PRESENT" >> pure(OmpFromClause::Expectation::Present))) @@ -466,6 +476,10 @@ TYPE_PARSER(construct(Parser{})) TYPE_PARSER(construct( Parser{}, maybe(":" >> scalarIntConstantExpr))) +TYPE_PARSER(construct( + construct(Parser{}) || + construct(Parser{}))) + // 2.9.5 ORDER ([order-modifier :]concurrent) TYPE_PARSER(construct( "REPRODUCIBLE" >> pure(OmpOrderModifier::Kind::Reproducible)) || @@ -531,6 +545,8 @@ TYPE_PARSER( "DIST_SCHEDULE" >> construct(construct( parenthesized("STATIC" >> maybe("," >> scalarIntExpr)))) || + "DOACROSS" >> + construct(parenthesized(Parser{})) || "DYNAMIC_ALLOCATORS" >> construct(construct()) || "ENTER" >> construct(construct( @@ -634,7 +650,7 @@ TYPE_PARSER( parenthesized(nonemptyList(name)))) || "UNTIED" >> construct(construct()) || "UPDATE" >> construct(construct( - parenthesized(Parser{})))) + parenthesized(Parser{})))) // [Clause, [Clause], ...] TYPE_PARSER(sourced(construct( diff --git a/flang/lib/Parser/parse-tree.cpp b/flang/lib/Parser/parse-tree.cpp index 60aef1666e9ba..574e5fd84862e 100644 --- a/flang/lib/Parser/parse-tree.cpp +++ b/flang/lib/Parser/parse-tree.cpp @@ -253,22 +253,23 @@ llvm::raw_ostream &operator<<(llvm::raw_ostream &os, const Name &x) { return os << x.ToString(); } -OmpTaskDependenceType::Type OmpDependClause::GetDepType() const { - return common::visit( +OmpDependenceType::Type OmpDoacross::GetDepType() const { + return common::visit( // common::visitors{ - [&](const parser::OmpDependClause::Source &) { - return parser::OmpTaskDependenceType::Type::Source; - }, - [&](const parser::OmpDependClause::Sink &) { - return parser::OmpTaskDependenceType::Type::Sink; + [](const OmpDoacross::Sink &) { + return OmpDependenceType::Type::Sink; }, - [&](const parser::OmpDependClause::InOut &y) { - return std::get(y.t).v; + [](const OmpDoacross::Source &) { + return OmpDependenceType::Type::Source; }, }, u); } +OmpTaskDependenceType::Type OmpDependClause::TaskDep::GetTaskDepType() const { + return std::get(t).v; +} + } // namespace Fortran::parser template static llvm::omp::Clause getClauseIdForClass(C &&) { diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp index bbb126dcdb6d5..14e128fb15bec 100644 --- a/flang/lib/Parser/unparse.cpp +++ b/flang/lib/Parser/unparse.cpp @@ -2228,36 +2228,16 @@ class UnparseVisitor { std::get>(x.t), ":"); Walk(std::get(x.t)); } - void Unparse(const OmpDependSinkVecLength &x) { - Walk(std::get(x.t)); - Walk(std::get(x.t)); + void Unparse(const OmpDoacross::Sink &x) { + Word("SINK: "); + Walk(x.v.v); } - void Unparse(const OmpDependSinkVec &x) { - Walk(std::get(x.t)); - Walk(std::get>(x.t)); - } - void Unparse(const OmpDependClause::InOut &x) { + void Unparse(const OmpDoacross::Source &) { Word("SOURCE"); } + void Unparse(const OmpDependClause::TaskDep &x) { Walk(std::get(x.t)); Put(":"); Walk(std::get(x.t)); } - bool Pre(const OmpDependClause &x) { - return common::visit( - common::visitors{ - [&](const OmpDependClause::Source &) { - Word("SOURCE"); - return false; - }, - [&](const OmpDependClause::Sink &y) { - Word("SINK:"); - Walk(y.v); - Put(")"); - return false; - }, - [&](const OmpDependClause::InOut &) { return true; }, - }, - x.u); - } void Unparse(const OmpDefaultmapClause &x) { Walk(std::get(x.t)); Walk(":", diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index 014604627f2cd..132fb6484bcfc 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -1623,36 +1623,37 @@ void OmpStructureChecker::ChecksOnOrderedAsStandalone() { "standalone construct with no ORDERED region"_err_en_US); } - bool isSinkPresent{false}; - int dependSourceCount{0}; + int dependSinkCount{0}, dependSourceCount{0}; + bool exclusiveShown{false}, duplicateSourceShown{false}; + + auto visitDoacross = [&](const parser::OmpDoacross &doa, + const parser::CharBlock &src) { + common::visit( + common::visitors{ + [&](const parser::OmpDoacross::Source &) { dependSourceCount++; }, + [&](const parser::OmpDoacross::Sink &) { dependSinkCount++; }}, + doa.u); + if (!exclusiveShown && dependSinkCount > 0 && dependSourceCount > 0) { + exclusiveShown = true; + context_.Say(src, + "The SINK and SOURCE dependence types are mutually exclusive"_err_en_US); + } + if (!duplicateSourceShown && dependSourceCount > 1) { + duplicateSourceShown = true; + 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) { const auto &dependClause{ std::get(itr->second->u)}; - if (std::get_if(&dependClause.v.u)) { - dependSourceCount++; - if (isSinkPresent) { - context_.Say(itr->second->source, - "DEPEND(SOURCE) is not allowed when DEPEND(SINK: vec) is present " - "on ORDERED directive"_err_en_US); - } - if (dependSourceCount > 1) { - context_.Say(itr->second->source, - "At most one DEPEND(SOURCE) clause can appear on the ORDERED " - "directive"_err_en_US); - } - } else if (std::get_if(&dependClause.v.u)) { - isSinkPresent = true; - if (dependSourceCount > 0) { - context_.Say(itr->second->source, - "DEPEND(SINK: vec) is not allowed when DEPEND(SOURCE) is present " - "on ORDERED directive"_err_en_US); - } + if (auto *doAcross{std::get_if(&dependClause.v.u)}) { + visitDoacross(*doAcross, itr->second->source); } else { context_.Say(itr->second->source, - "Only DEPEND(SOURCE) or DEPEND(SINK: vec) are allowed when ORDERED " - "construct is a standalone construct with no ORDERED " - "region"_err_en_US); + "Only SINK or SOURCE dependence types are allowed when ORDERED construct is a standalone construct with no ORDERED region"_err_en_US); } } @@ -1681,20 +1682,23 @@ void OmpStructureChecker::ChecksOnOrderedAsStandalone() { } void OmpStructureChecker::CheckOrderedDependClause( - std::optional orderedValue) { - auto clauseAll{FindClauses(llvm::omp::Clause::OMPC_depend)}; - for (auto itr = clauseAll.first; itr != clauseAll.second; ++itr) { - const auto &dependClause{ - std::get(itr->second->u)}; - if (const auto *sinkVectors{ - std::get_if(&dependClause.v.u)}) { - std::int64_t numVar = sinkVectors->v.size(); + std::optional orderedValue) { + 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) { - context_.Say(itr->second->source, - "The number of variables in DEPEND(SINK: vec) clause does not " - "match the parameter specified in ORDERED clause"_err_en_US); + context_.Say(src, + "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 &dependClause{std::get(itr->second->u)}; + if (auto *doAcross{std::get_if(&dependClause.v.u)}) { + visitDoacross(*doAcross, itr->second->source); + } } } @@ -1736,17 +1740,13 @@ void OmpStructureChecker::CheckTaskDependenceType( const parser::OmpTaskDependenceType::Type &x) { // Common checks for task-dependence-type (DEPEND and UPDATE clauses). unsigned version{context_.langOptions().OpenMPVersion}; - unsigned since{0}, deprecatedIn{~0u}; + unsigned since{0}; switch (x) { case parser::OmpTaskDependenceType::Type::In: case parser::OmpTaskDependenceType::Type::Out: case parser::OmpTaskDependenceType::Type::Inout: break; - case parser::OmpTaskDependenceType::Type::Source: - case parser::OmpTaskDependenceType::Type::Sink: - deprecatedIn = 52; - break; case parser::OmpTaskDependenceType::Type::Mutexinoutset: case parser::OmpTaskDependenceType::Type::Depobj: since = 50; @@ -1756,21 +1756,36 @@ void OmpStructureChecker::CheckTaskDependenceType( break; } - if (version >= deprecatedIn) { + if (version < since) { context_.Say(GetContext().clauseSource, - "%s task-dependence-type is deprecated in %s"_warn_en_US, - parser::ToUpperCaseLetters( - parser::OmpTaskDependenceType::EnumToString(x)), - ThisVersion(deprecatedIn)); - } else if (version < since) { - context_.Say(GetContext().clauseSource, - "%s task-dependence-type is not supported in %s, %s"_warn_en_US, + "%s task dependence type is not supported in %s, %s"_warn_en_US, parser::ToUpperCaseLetters( parser::OmpTaskDependenceType::EnumToString(x)), ThisVersion(version), TryVersion(since)); } } +void OmpStructureChecker::CheckDependenceType( + const parser::OmpDependenceType::Type &x) { + // Common checks for dependence-type (DEPEND and UPDATE clauses). + unsigned version{context_.langOptions().OpenMPVersion}; + unsigned deprecatedIn{~0u}; + + switch (x) { + case parser::OmpDependenceType::Type::Source: + case parser::OmpDependenceType::Type::Sink: + deprecatedIn = 52; + break; + } + + if (version >= deprecatedIn) { + context_.Say(GetContext().clauseSource, + "%s dependence type is deprecated in %s"_warn_en_US, + parser::ToUpperCaseLetters(parser::OmpDependenceType::EnumToString(x)), + ThisVersion(deprecatedIn)); + } +} + void OmpStructureChecker::Enter( const parser::OpenMPSimpleStandaloneConstruct &x) { const auto &dir{std::get(x.t)}; @@ -3434,41 +3449,50 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Device &x) { void OmpStructureChecker::Enter(const parser::OmpClause::Depend &x) { CheckAllowedClause(llvm::omp::Clause::OMPC_depend); - llvm::omp::Directive directive{GetContext().directive}; + llvm::omp::Directive dir{GetContext().directive}; unsigned version{context_.langOptions().OpenMPVersion}; - using DepType = parser::OmpTaskDependenceType::Type; - DepType depType = x.v.GetDepType(); + auto *doaDep{std::get_if(&x.v.u)}; + auto *taskDep{std::get_if(&x.v.u)}; + assert(((doaDep == nullptr) != (taskDep == nullptr)) && + "Unexpected alternative in update clause"); - CheckTaskDependenceType(depType); + if (doaDep) { + CheckDependenceType(doaDep->GetDepType()); + } else { + CheckTaskDependenceType(taskDep->GetTaskDepType()); + } - if (directive == llvm::omp::OMPD_depobj) { + if (dir == llvm::omp::OMPD_depobj) { // [5.0:255:11], [5.1:288:3] // A depend clause on a depobj construct must not have source, sink [or // depobj](5.0) as dependence-type. if (version >= 50) { - bool invalidDep{depType == DepType::Source || depType == DepType::Sink}; - if (version == 50) { - invalidDep = invalidDep || depType == DepType::Depobj; + bool invalidDep{false}; + if (taskDep) { + if (version == 50) { + invalidDep = taskDep->GetTaskDepType() == + parser::OmpTaskDependenceType::Type::Depobj; + } + } else { + invalidDep = true; } if (invalidDep) { context_.Say(GetContext().clauseSource, - "A DEPEND clause on a DEPOBJ construct must not have SOURCE%s " - "as dependence-type"_err_en_US, - version == 50 ? ", SINK or DEPOBJ" : " or SINK"); + "A DEPEND clause on a DEPOBJ construct must not have %s as dependence type"_err_en_US, + version == 50 ? "SINK, SOURCE or DEPOBJ" : "SINK or SOURCE"); } } - } else if (directive != llvm::omp::OMPD_ordered) { - if (depType == DepType::Source || depType == DepType::Sink) { + } else if (dir != llvm::omp::OMPD_ordered) { + if (doaDep) { context_.Say(GetContext().clauseSource, - "DEPEND(SOURCE) or DEPEND(SINK : vec) can be used only with the " - "ordered directive. Used here in the %s construct."_err_en_US, - parser::ToUpperCaseLetters(getDirectiveName(directive))); + "The SINK and SOURCE dependence types can only be used with the ORDERED directive, used here in the %s construct"_err_en_US, + parser::ToUpperCaseLetters(getDirectiveName(dir))); } } - if (const auto *inOut{std::get_if(&x.v.u)}) { - auto &objList{std::get(inOut->t)}; - if (directive == llvm::omp::OMPD_depobj) { + if (taskDep) { + auto &objList{std::get(taskDep->t)}; + if (dir == llvm::omp::OMPD_depobj) { // [5.0:255:13], [5.1:288:6], [5.2:322:26] // A depend clause on a depobj construct must only specify one locator. if (objList.v.size() != 1) { @@ -3495,14 +3519,14 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Depend &x) { } } } - if (std::get>(inOut->t)) { + if (std::get>(taskDep->t)) { unsigned allowedInVersion{50}; if (version < allowedInVersion) { context_.Say(GetContext().clauseSource, "Iterator modifiers are not supported in %s, %s"_warn_en_US, ThisVersion(version), TryVersion(allowedInVersion)); } else { - if (directive == llvm::omp::OMPD_depobj) { + if (dir == llvm::omp::OMPD_depobj) { context_.Say(GetContext().clauseSource, "An iterator-modifier may specify multiple locators, " "a DEPEND clause on a DEPOBJ construct must only specify " @@ -3624,29 +3648,36 @@ void OmpStructureChecker::CheckStructureElement( void OmpStructureChecker::Enter(const parser::OmpClause::Update &x) { CheckAllowedClause(llvm::omp::Clause::OMPC_update); - llvm::omp::Directive directive{GetContext().directive}; + llvm::omp::Directive dir{GetContext().directive}; unsigned version{context_.langOptions().OpenMPVersion}; - CheckTaskDependenceType(x.v.v.v); + auto *depType{std::get_if(&x.v.u)}; + auto *taskType{std::get_if(&x.v.u)}; + assert(((depType == nullptr) != (taskType == nullptr)) && + "Unexpected alternative in update clause"); + + if (depType) { + CheckDependenceType(depType->v); + } else if (taskType) { + CheckTaskDependenceType(taskType->v); + } // [5.1:288:4-5] // An update clause on a depobj construct must not have source, sink or depobj // as dependence-type. // [5.2:322:3] // task-dependence-type must not be depobj. - if (directive == llvm::omp::OMPD_depobj) { + if (dir == llvm::omp::OMPD_depobj) { if (version >= 51) { - // Update -> OmpUpdateClause -> OmpTaskDependenceType -> Type - switch (x.v.v.v) { - case parser::OmpTaskDependenceType::Type::Source: - case parser::OmpTaskDependenceType::Type::Sink: - case parser::OmpTaskDependenceType::Type::Depobj: + bool invalidDep{false}; + if (taskType) { + invalidDep = taskType->v == parser::OmpTaskDependenceType::Type::Depobj; + } else { + invalidDep = true; + } + if (invalidDep) { context_.Say(GetContext().clauseSource, - "An UPDATE clause on a DEPOBJ construct must not have SOURCE, " - "SINK or DEPOBJ as dependence-type"_err_en_US); - break; - default: - break; + "An UPDATE clause on a DEPOBJ construct must not have SINK, SOURCE or DEPOBJ as dependence type"_err_en_US); } } } diff --git a/flang/lib/Semantics/check-omp-structure.h b/flang/lib/Semantics/check-omp-structure.h index d9236be8bced4..9efacaa971008 100644 --- a/flang/lib/Semantics/check-omp-structure.h +++ b/flang/lib/Semantics/check-omp-structure.h @@ -202,6 +202,7 @@ class OmpStructureChecker void CheckSIMDNest(const parser::OpenMPConstruct &x); void CheckTargetNest(const parser::OpenMPConstruct &x); void CheckTargetUpdate(); + void CheckDependenceType(const parser::OmpDependenceType::Type &x); void CheckTaskDependenceType(const parser::OmpTaskDependenceType::Type &x); void CheckCancellationNest( const parser::CharBlock &source, const parser::OmpCancelType::Type &type); diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index c2b5b9673239b..632d7e918ac64 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -553,7 +553,7 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor { return false; } - void Post(const parser::OmpDependSinkVec &x) { + void Post(const parser::OmpIteration &x) { const auto &name{std::get(x.t)}; ResolveName(&name); } @@ -1138,7 +1138,7 @@ bool AccAttributeVisitor::Pre(const parser::OpenACCCombinedConstruct &x) { static bool IsLastNameArray(const parser::Designator &designator) { const auto &name{GetLastName(designator)}; const evaluate::DataRef dataRef{*(name.symbol)}; - return common::visit( + return common::visit( // common::visitors{ [](const evaluate::SymbolRef &ref) { return ref->Rank() > 0 || diff --git a/flang/test/Parser/OpenMP/depobj-construct.f90 b/flang/test/Parser/OpenMP/depobj-construct.f90 index 7c474071bc1e6..3de190c95bb73 100644 --- a/flang/test/Parser/OpenMP/depobj-construct.f90 +++ b/flang/test/Parser/OpenMP/depobj-construct.f90 @@ -14,7 +14,7 @@ subroutine f00 !PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPStandaloneConstruct -> OpenMPDepobjConstruct !PARSE-TREE: | Verbatim !PARSE-TREE: | OmpObject -> Designator -> DataRef -> Name = 'x' -!PARSE-TREE: | OmpClause -> Depend -> OmpDependClause -> InOut +!PARSE-TREE: | OmpClause -> Depend -> OmpDependClause -> TaskDep !PARSE-TREE: | | OmpTaskDependenceType -> Type = In !PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'y' diff --git a/flang/test/Parser/OpenMP/doacross-clause.f90 b/flang/test/Parser/OpenMP/doacross-clause.f90 new file mode 100644 index 0000000000000..afd27d9d727e0 --- /dev/null +++ b/flang/test/Parser/OpenMP/doacross-clause.f90 @@ -0,0 +1,90 @@ +!RUN: %flang_fc1 -fdebug-unparse -fopenmp -fopenmp-version=52 %s | FileCheck --ignore-case --check-prefix="UNPARSE" %s +!RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp -fopenmp-version=52 %s | FileCheck --check-prefix="PARSE-TREE" %s + +subroutine f00(x) + integer :: x(10, 10) + !$omp do ordered(2) + do i = 1, 10 + do j = 1, 10 + !$omp ordered doacross(source) + x(i, j) = i + j + enddo + enddo + !$omp end do +end + +!UNPARSE: SUBROUTINE f00 (x) +!UNPARSE: INTEGER x(10_4,10_4) +!UNPARSE: !$OMP DO ORDERED(2_4) +!UNPARSE: DO i=1_4,10_4 +!UNPARSE: DO j=1_4,10_4 +!UNPARSE: !$OMP ORDERED DOACROSS(SOURCE) +!UNPARSE: x(int(i,kind=8),int(j,kind=8))=i+j +!UNPARSE: END DO +!UNPARSE: END DO +!UNPARSE: !$OMP END DO +!UNPARSE: END SUBROUTINE + +!PARSE-TREE-LABEL: ProgramUnit -> SubroutineSubprogram +!PARSE-TREE: OmpBeginLoopDirective +!PARSE-TREE: | OmpLoopDirective -> llvm::omp::Directive = do +!PARSE-TREE: | OmpClauseList -> OmpClause -> Ordered -> Scalar -> Integer -> Constant -> Expr = '2_4' +!PARSE-TREE: | | LiteralConstant -> IntLiteralConstant = '2' +![...] +!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPStandaloneConstruct -> OpenMPSimpleStandaloneConstruct +!PARSE-TREE: | OmpSimpleStandaloneDirective -> llvm::omp::Directive = ordered +!PARSE-TREE: | OmpClauseList -> OmpClause -> Doacross -> OmpDoacrossClause -> OmpDoacross -> Source + +subroutine f01(x) + integer :: x(10, 10) + !$omp do ordered(2) + do i = 1, 10 + do j = 1, 10 + !$omp ordered doacross(sink: i+1, j-2), doacross(sink: i, j+3) + x(i, j) = i + j + enddo + enddo + !$omp end do +end + +!UNPARSE: SUBROUTINE f01 (x) +!UNPARSE: INTEGER x(10_4,10_4) +!UNPARSE: !$OMP DO ORDERED(2_4) +!UNPARSE: DO i=1_4,10_4 +!UNPARSE: DO j=1_4,10_4 +!UNPARSE: !$OMP ORDERED DOACROSS(SINK: i+1_4, j-2_4) DOACROSS(SINK: i, j+3_4) +!UNPARSE: x(int(i,kind=8),int(j,kind=8))=i+j +!UNPARSE: END DO +!UNPARSE: END DO +!UNPARSE: !$OMP END DO +!UNPARSE: END SUBROUTINE + +!PARSE-TREE-LABEL: ProgramUnit -> SubroutineSubprogram +!PARSE-TREE: OmpBeginLoopDirective +!PARSE-TREE: | OmpLoopDirective -> llvm::omp::Directive = do +!PARSE-TREE: | OmpClauseList -> OmpClause -> Ordered -> Scalar -> Integer -> Constant -> Expr = '2_4' +!PARSE-TREE: | | LiteralConstant -> IntLiteralConstant = '2' +![...] +!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPStandaloneConstruct -> OpenMPSimpleStandaloneConstruct +!PARSE-TREE: | OmpSimpleStandaloneDirective -> llvm::omp::Directive = ordered +!PARSE-TREE: | OmpClauseList -> OmpClause -> Doacross -> OmpDoacrossClause -> OmpDoacross -> Sink -> OmpIterationVector -> OmpIteration +!PARSE-TREE: | | Name = 'i' +!PARSE-TREE: | | OmpIterationOffset +!PARSE-TREE: | | | DefinedOperator -> IntrinsicOperator = Add +!PARSE-TREE: | | | Scalar -> Integer -> Constant -> Expr = '1_4' +!PARSE-TREE: | | | | LiteralConstant -> IntLiteralConstant = '1' +!PARSE-TREE: | OmpIteration +!PARSE-TREE: | | Name = 'j' +!PARSE-TREE: | | OmpIterationOffset +!PARSE-TREE: | | | DefinedOperator -> IntrinsicOperator = Subtract +!PARSE-TREE: | | | Scalar -> Integer -> Constant -> Expr = '2_4' +!PARSE-TREE: | | | | LiteralConstant -> IntLiteralConstant = '2' +!PARSE-TREE: | OmpClause -> Doacross -> OmpDoacrossClause -> OmpDoacross -> Sink -> OmpIterationVector -> OmpIteration +!PARSE-TREE: | | Name = 'i' +!PARSE-TREE: | OmpIteration +!PARSE-TREE: | | Name = 'j' +!PARSE-TREE: | | OmpIterationOffset +!PARSE-TREE: | | | DefinedOperator -> IntrinsicOperator = Add +!PARSE-TREE: | | | Scalar -> Integer -> Constant -> Expr = '3_4' +!PARSE-TREE: | | | | LiteralConstant -> IntLiteralConstant = '3' + diff --git a/flang/test/Parser/OpenMP/ordered-depend.f90 b/flang/test/Parser/OpenMP/ordered-depend.f90 new file mode 100644 index 0000000000000..9e0946af0f09a --- /dev/null +++ b/flang/test/Parser/OpenMP/ordered-depend.f90 @@ -0,0 +1,90 @@ +!RUN: %flang_fc1 -fdebug-unparse -fopenmp -fopenmp-version=45 %s | FileCheck --ignore-case --check-prefix="UNPARSE" %s +!RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp -fopenmp-version=45 %s | FileCheck --check-prefix="PARSE-TREE" %s + +subroutine f00(x) + integer :: x(10, 10) + !$omp do ordered(2) + do i = 1, 10 + do j = 1, 10 + !$omp ordered depend(source) + x(i, j) = i + j + enddo + enddo + !$omp end do +end + +!UNPARSE: SUBROUTINE f00 (x) +!UNPARSE: INTEGER x(10_4,10_4) +!UNPARSE: !$OMP DO ORDERED(2_4) +!UNPARSE: DO i=1_4,10_4 +!UNPARSE: DO j=1_4,10_4 +!UNPARSE: !$OMP ORDERED DEPEND(SOURCE) +!UNPARSE: x(int(i,kind=8),int(j,kind=8))=i+j +!UNPARSE: END DO +!UNPARSE: END DO +!UNPARSE: !$OMP END DO +!UNPARSE: END SUBROUTINE + +!PARSE-TREE-LABEL: ProgramUnit -> SubroutineSubprogram +!PARSE-TREE: OmpBeginLoopDirective +!PARSE-TREE: | OmpLoopDirective -> llvm::omp::Directive = do +!PARSE-TREE: | OmpClauseList -> OmpClause -> Ordered -> Scalar -> Integer -> Constant -> Expr = '2_4' +!PARSE-TREE: | | LiteralConstant -> IntLiteralConstant = '2' +![...] +!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPStandaloneConstruct -> OpenMPSimpleStandaloneConstruct +!PARSE-TREE: | OmpSimpleStandaloneDirective -> llvm::omp::Directive = ordered +!PARSE-TREE: | OmpClauseList -> OmpClause -> Depend -> OmpDependClause -> OmpDoacross -> Source + +subroutine f01(x) + integer :: x(10, 10) + !$omp do ordered(2) + do i = 1, 10 + do j = 1, 10 + !$omp ordered depend(sink: i+1, j-2), depend(sink: i, j+3) + x(i, j) = i + j + enddo + enddo + !$omp end do +end + +!UNPARSE: SUBROUTINE f01 (x) +!UNPARSE: INTEGER x(10_4,10_4) +!UNPARSE: !$OMP DO ORDERED(2_4) +!UNPARSE: DO i=1_4,10_4 +!UNPARSE: DO j=1_4,10_4 +!UNPARSE: !$OMP ORDERED DEPEND(SINK: i+1_4, j-2_4) DEPEND(SINK: i, j+3_4) +!UNPARSE: x(int(i,kind=8),int(j,kind=8))=i+j +!UNPARSE: END DO +!UNPARSE: END DO +!UNPARSE: !$OMP END DO +!UNPARSE: END SUBROUTINE + +!PARSE-TREE-LABEL: ProgramUnit -> SubroutineSubprogram +!PARSE-TREE: OmpBeginLoopDirective +!PARSE-TREE: | OmpLoopDirective -> llvm::omp::Directive = do +!PARSE-TREE: | OmpClauseList -> OmpClause -> Ordered -> Scalar -> Integer -> Constant -> Expr = '2_4' +!PARSE-TREE: | | LiteralConstant -> IntLiteralConstant = '2' +![...] +!PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPStandaloneConstruct -> OpenMPSimpleStandaloneConstruct +!PARSE-TREE: | OmpSimpleStandaloneDirective -> llvm::omp::Directive = ordered +!PARSE-TREE: | OmpClauseList -> OmpClause -> Depend -> OmpDependClause -> OmpDoacross -> Sink -> OmpIterationVector -> OmpIteration +!PARSE-TREE: | | Name = 'i' +!PARSE-TREE: | | OmpIterationOffset +!PARSE-TREE: | | | DefinedOperator -> IntrinsicOperator = Add +!PARSE-TREE: | | | Scalar -> Integer -> Constant -> Expr = '1_4' +!PARSE-TREE: | | | | LiteralConstant -> IntLiteralConstant = '1' +!PARSE-TREE: | OmpIteration +!PARSE-TREE: | | Name = 'j' +!PARSE-TREE: | | OmpIterationOffset +!PARSE-TREE: | | | DefinedOperator -> IntrinsicOperator = Subtract +!PARSE-TREE: | | | Scalar -> Integer -> Constant -> Expr = '2_4' +!PARSE-TREE: | | | | LiteralConstant -> IntLiteralConstant = '2' +!PARSE-TREE: | OmpClause -> Depend -> OmpDependClause -> OmpDoacross -> Sink -> OmpIterationVector -> OmpIteration +!PARSE-TREE: | | Name = 'i' +!PARSE-TREE: | OmpIteration +!PARSE-TREE: | | Name = 'j' +!PARSE-TREE: | | OmpIterationOffset +!PARSE-TREE: | | | DefinedOperator -> IntrinsicOperator = Add +!PARSE-TREE: | | | Scalar -> Integer -> Constant -> Expr = '3_4' +!PARSE-TREE: | | | | LiteralConstant -> IntLiteralConstant = '3' + diff --git a/flang/test/Semantics/OpenMP/clause-validity01.f90 b/flang/test/Semantics/OpenMP/clause-validity01.f90 index 124f1a02d99fb..406d30b38948e 100644 --- a/flang/test/Semantics/OpenMP/clause-validity01.f90 +++ b/flang/test/Semantics/OpenMP/clause-validity01.f90 @@ -495,7 +495,7 @@ !$omp taskyield !$omp barrier !$omp taskwait - !ERROR: DEPEND(SOURCE) or DEPEND(SINK : vec) can be used only with the ordered directive. Used here in the TASKWAIT construct. + !ERROR: The SINK and SOURCE dependence types can only be used with the ORDERED directive, used here in the TASKWAIT construct !$omp taskwait depend(source) ! !$omp taskwait depend(sink:i-1) ! !$omp target enter data map(to:arrayA) map(alloc:arrayB) diff --git a/flang/test/Semantics/OpenMP/depend06.f90 b/flang/test/Semantics/OpenMP/depend06.f90 index a9668c552f967..d2e6a114676c3 100644 --- a/flang/test/Semantics/OpenMP/depend06.f90 +++ b/flang/test/Semantics/OpenMP/depend06.f90 @@ -2,7 +2,7 @@ subroutine f00(x) integer :: x -!WARNING: INOUTSET task-dependence-type is not supported in OpenMP v4.5, try -fopenmp-version=52 +!WARNING: INOUTSET task dependence type is not supported in OpenMP v4.5, try -fopenmp-version=52 !$omp task depend(inoutset: x) x = x + 1 !$omp end task @@ -10,7 +10,7 @@ subroutine f00(x) subroutine f01(x) integer :: x -!WARNING: MUTEXINOUTSET task-dependence-type is not supported in OpenMP v4.5, try -fopenmp-version=50 +!WARNING: MUTEXINOUTSET task dependence type is not supported in OpenMP v4.5, try -fopenmp-version=50 !$omp task depend(mutexinoutset: x) x = x + 1 !$omp end task diff --git a/flang/test/Semantics/OpenMP/depobj-construct-v50.f90 b/flang/test/Semantics/OpenMP/depobj-construct-v50.f90 index e87d86ca54bee..76661785826b4 100644 --- a/flang/test/Semantics/OpenMP/depobj-construct-v50.f90 +++ b/flang/test/Semantics/OpenMP/depobj-construct-v50.f90 @@ -2,7 +2,7 @@ subroutine f00 integer :: obj -!ERROR: A DEPEND clause on a DEPOBJ construct must not have SOURCE, SINK or DEPOBJ as dependence-type +!ERROR: A DEPEND clause on a DEPOBJ construct must not have SINK, SOURCE or DEPOBJ as dependence type !$omp depobj(obj) depend(source) end diff --git a/flang/test/Semantics/OpenMP/depobj-construct-v51.f90 b/flang/test/Semantics/OpenMP/depobj-construct-v51.f90 index fa0c025a11010..fc403f0b2db22 100644 --- a/flang/test/Semantics/OpenMP/depobj-construct-v51.f90 +++ b/flang/test/Semantics/OpenMP/depobj-construct-v51.f90 @@ -2,12 +2,12 @@ subroutine f04 integer :: obj -!ERROR: An UPDATE clause on a DEPOBJ construct must not have SOURCE, SINK or DEPOBJ as dependence-type +!ERROR: An UPDATE clause on a DEPOBJ construct must not have SINK, SOURCE or DEPOBJ as dependence type !$omp depobj(obj) update(source) end subroutine f05 integer :: obj -!ERROR: An UPDATE clause on a DEPOBJ construct must not have SOURCE, SINK or DEPOBJ as dependence-type +!ERROR: An UPDATE clause on a DEPOBJ construct must not have SINK, SOURCE or DEPOBJ as dependence type !$omp depobj(obj) update(depobj) end diff --git a/flang/test/Semantics/OpenMP/depobj-construct-v52.f90 b/flang/test/Semantics/OpenMP/depobj-construct-v52.f90 index 42a2102500ea7..644090d7f7e8b 100644 --- a/flang/test/Semantics/OpenMP/depobj-construct-v52.f90 +++ b/flang/test/Semantics/OpenMP/depobj-construct-v52.f90 @@ -2,8 +2,8 @@ subroutine f00 integer :: obj -!WARNING: SOURCE task-dependence-type is deprecated in OpenMP v5.2 -!ERROR: A DEPEND clause on a DEPOBJ construct must not have SOURCE or SINK as dependence-type +!WARNING: SOURCE dependence type is deprecated in OpenMP v5.2 +!ERROR: A DEPEND clause on a DEPOBJ construct must not have SINK or SOURCE as dependence type !$omp depobj(obj) depend(source) end diff --git a/flang/test/Semantics/OpenMP/ordered01.f90 b/flang/test/Semantics/OpenMP/ordered01.f90 index 9433120fab10f..9f3a258d470a6 100644 --- a/flang/test/Semantics/OpenMP/ordered01.f90 +++ b/flang/test/Semantics/OpenMP/ordered01.f90 @@ -37,17 +37,16 @@ program main !$omp do ordered(1) do i = 2, N - !ERROR: Only DEPEND(SOURCE) or DEPEND(SINK: vec) are allowed when ORDERED construct is a standalone construct with no ORDERED region - !ERROR: At most one DEPEND(SOURCE) clause can appear on the ORDERED directive + !ERROR: Only SINK or SOURCE dependence types are allowed when ORDERED construct is a standalone construct with no ORDERED region + !ERROR: At most one SOURCE dependence type can appear on the ORDERED directive !$omp ordered depend(source) depend(inout: arrayA) depend(source) arrayA(i) = foo(i) - !ERROR: DEPEND(SOURCE) is not allowed when DEPEND(SINK: vec) is present on ORDERED directive - !ERROR: DEPEND(SOURCE) is not allowed when DEPEND(SINK: vec) is present on ORDERED directive - !ERROR: At most one DEPEND(SOURCE) clause can appear on the ORDERED directive + !ERROR: The SINK and SOURCE dependence types are mutually exclusive + !ERROR: At most one SOURCE dependence type can appear on the ORDERED directive !$omp ordered depend(sink: i - 1) depend(source) depend(source) arrayB(i) = bar(arrayA(i), arrayB(i-1)) - !ERROR: Only DEPEND(SOURCE) or DEPEND(SINK: vec) are allowed when ORDERED construct is a standalone construct with no ORDERED region - !ERROR: Only DEPEND(SOURCE) or DEPEND(SINK: vec) are allowed when ORDERED construct is a standalone construct with no ORDERED region + !ERROR: Only SINK or SOURCE dependence types are allowed when ORDERED construct is a standalone construct with no ORDERED region + !ERROR: Only SINK or SOURCE dependence types are allowed when ORDERED construct is a standalone construct with no ORDERED region !$omp ordered depend(out: arrayC) depend(in: arrayB) arrayC(i) = baz(arrayB(i-1)) end do diff --git a/flang/test/Semantics/OpenMP/ordered03.f90 b/flang/test/Semantics/OpenMP/ordered03.f90 index 18f85fc24a9fb..e96d4557f8f18 100644 --- a/flang/test/Semantics/OpenMP/ordered03.f90 +++ b/flang/test/Semantics/OpenMP/ordered03.f90 @@ -99,7 +99,7 @@ subroutine sub1() !$omp do ordered(1) do i = 1, N - !ERROR: The number of variables in DEPEND(SINK: vec) clause does not match the parameter specified in ORDERED clause + !ERROR: The number of variables in the SINK iteration vector does not match the parameter specified in ORDERED clause !$omp ordered depend(sink: i - 1) depend(sink: i - 1, j) arrayB(i) = bar(i - 1, j) end do @@ -108,7 +108,7 @@ subroutine sub1() !$omp do ordered(2) do i = 1, N do j = 1, N - !ERROR: The number of variables in DEPEND(SINK: vec) clause does not match the parameter specified in ORDERED clause + !ERROR: The number of variables in the SINK iteration vector does not match the parameter specified in ORDERED clause !$omp ordered depend(sink: i - 1) depend(sink: i - 1, j) arrayB(i) = foo(i - 1) + bar(i - 1, j) end do diff --git a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h index 8ff15b51f1abd..f4e089db0080e 100644 --- a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h +++ b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h @@ -238,8 +238,9 @@ struct MapperT { // When used as arguments for other clauses, e.g. `fail`. ENUM(MemoryOrder, AcqRel, Acquire, Relaxed, Release, SeqCst); ENUM(MotionExpectation, Present); +// Union of `dependence-type` and `task-depenence-type`. // V5.2: [15.9.1] `task-dependence-type` modifier -ENUM(TaskDependenceType, Depobj, In, Inout, Inoutset, Mutexinoutset, Out, Sink, +ENUM(DependenceType, Depobj, In, Inout, Inoutset, Mutexinoutset, Out, Sink, Source); template // @@ -502,17 +503,17 @@ template // struct DependT { using Iterator = type::IteratorT; using LocatorList = ObjectListT; - using TaskDependenceType = tomp::type::TaskDependenceType; + using DependenceType = tomp::type::DependenceType; - struct DepType { // The form with task dependence type. + struct TaskDep { // The form with task dependence type. using TupleTrait = std::true_type; // Empty LocatorList means "omp_all_memory". - std::tuple t; + std::tuple t; }; using Doacross = DoacrossT; using UnionTrait = std::true_type; - std::variant u; // Doacross form is legacy + std::variant u; // Doacross form is legacy }; // V5.2: [3.5] `destroy` clause @@ -562,7 +563,7 @@ struct DistScheduleT { template // struct DoacrossT { using Vector = ListT>; - ENUM(DependenceType, Source, Sink); + using DependenceType = tomp::type::DependenceType; using TupleTrait = std::true_type; // Empty Vector means "omp_cur_iteration" std::tuple t; @@ -1162,9 +1163,9 @@ struct UntiedT { // V5.2: [15.9.3] `update` clause template // struct UpdateT { - using TaskDependenceType = tomp::type::TaskDependenceType; + using DependenceType = tomp::type::DependenceType; using WrapperTrait = std::true_type; - OPT(TaskDependenceType) v; + OPT(DependenceType) v; }; // V5.2: [14.1.3] `use` clause diff --git a/llvm/include/llvm/Frontend/OpenMP/OMP.td b/llvm/include/llvm/Frontend/OpenMP/OMP.td index 36834939d9b45..e81cdb681cb99 100644 --- a/llvm/include/llvm/Frontend/OpenMP/OMP.td +++ b/llvm/include/llvm/Frontend/OpenMP/OMP.td @@ -151,6 +151,7 @@ def OMPC_DistSchedule : Clause<"dist_schedule"> { } def OMPC_Doacross : Clause<"doacross"> { let clangClass = "OMPDoacrossClause"; + let flangClass = "OmpDoacrossClause"; } def OMPC_DynamicAllocators : Clause<"dynamic_allocators"> { let clangClass = "OMPDynamicAllocatorsClause"; From 4165254e8c4a7b572e741cb62d632b462537dc0f Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Tue, 5 Nov 2024 12:01:43 -0600 Subject: [PATCH 2/8] [flang][OpenMP] Semantic checks for DOACROSS clause Keep track of loop constructs and OpenMP loop constructs that have been entered. Use the information to validate the variables in the SINK loop iteration vector. --- flang/lib/Lower/OpenMP/Clauses.cpp | 28 ++-- flang/lib/Semantics/check-omp-structure.cpp | 141 ++++++++++++++++++-- flang/lib/Semantics/check-omp-structure.h | 25 +++- flang/lib/Semantics/resolve-directives.cpp | 12 +- flang/test/Lower/OpenMP/Todo/ordered.f90 | 20 +++ flang/test/Semantics/OpenMP/doacross.f90 | 28 ++++ flang/test/Semantics/OpenMP/ordered01.f90 | 4 +- flang/test/Semantics/OpenMP/ordered03.f90 | 2 + 8 files changed, 230 insertions(+), 30 deletions(-) create mode 100644 flang/test/Lower/OpenMP/Todo/ordered.f90 create mode 100644 flang/test/Semantics/OpenMP/doacross.f90 diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp index f6633dd53f6f2..1764b3b79b4e3 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 132fb6484bcfc..67360b983a7d1 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -541,6 +541,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)}; @@ -933,11 +934,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) { @@ -1068,8 +1077,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; } @@ -1618,7 +1626,8 @@ void OmpStructureChecker::CheckBarrierNesting( void OmpStructureChecker::ChecksOnOrderedAsStandalone() { if (FindClause(llvm::omp::Clause::OMPC_threads) || FindClause(llvm::omp::Clause::OMPC_simd)) { - context_.Say(GetContext().clauseSource, + context_.Say( + GetContext().clauseSource, "THREADS, SIMD clauses are not allowed when ORDERED construct is a " "standalone construct with no ORDERED region"_err_en_US); } @@ -1645,8 +1654,9 @@ void OmpStructureChecker::ChecksOnOrderedAsStandalone() { } }; - 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)}) { @@ -1656,6 +1666,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() && @@ -1693,13 +1708,18 @@ void OmpStructureChecker::CheckOrderedDependClause( } } }; - 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() { @@ -2677,7 +2697,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) @@ -3458,6 +3477,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()); @@ -3537,6 +3557,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)) { @@ -4291,6 +4398,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 9efacaa971008..57796ad32de69 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" @@ -156,13 +162,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 = ""); @@ -254,9 +266,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. @@ -264,10 +280,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 632d7e918ac64..9267262d4718e 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..661b3939b3e62 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 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 From ecde8f5cfacdec0daf6e223143dca374bc954cbc Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Thu, 7 Nov 2024 17:34:46 -0600 Subject: [PATCH 3/8] format --- flang/lib/Semantics/check-omp-structure.cpp | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index 67360b983a7d1..cfa6155b5b713 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -1626,10 +1626,8 @@ void OmpStructureChecker::CheckBarrierNesting( 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); + context_.Say(GetContext().clauseSource, + "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}; From 49d5e9c878987c1e553fb4d2837c85a62a99f5a0 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Thu, 7 Nov 2024 17:38:50 -0600 Subject: [PATCH 4/8] restart build From 9ab7cd0b0d359d98d5adf530edd0aee901ca0c23 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Thu, 7 Nov 2024 19:27:04 -0600 Subject: [PATCH 5/8] fix error message in test --- flang/test/Semantics/OpenMP/ordered01.f90 | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flang/test/Semantics/OpenMP/ordered01.f90 b/flang/test/Semantics/OpenMP/ordered01.f90 index 661b3939b3e62..12543acb2916b 100644 --- a/flang/test/Semantics/OpenMP/ordered01.f90 +++ b/flang/test/Semantics/OpenMP/ordered01.f90 @@ -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 From b1fd415dbb1494a68ed729a3070bcd8e60902774 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Fri, 8 Nov 2024 07:47:18 -0600 Subject: [PATCH 6/8] fix example --- flang/examples/FeatureList/FeatureList.cpp | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/flang/examples/FeatureList/FeatureList.cpp b/flang/examples/FeatureList/FeatureList.cpp index 62f8d39a8abaa..dc68f160f5d92 100644 --- a/flang/examples/FeatureList/FeatureList.cpp +++ b/flang/examples/FeatureList/FeatureList.cpp @@ -470,13 +470,17 @@ struct NodeVisitor { READ_FEATURE(OmpDefaultmapClause::ImplicitBehavior) READ_FEATURE(OmpDefaultmapClause::VariableCategory) READ_FEATURE(OmpDependClause) - READ_FEATURE(OmpDependClause::InOut) - READ_FEATURE(OmpDependClause::Sink) - READ_FEATURE(OmpDependClause::Source) + READ_FEATURE(OmpDependClause::TaskDep) + READ_FEATURE(OmpDoacross::Sink) + READ_FEATURE(OmpDoacross::Source) + READ_FEATURE(OmpDoacrossClause) + READ_FEATURE(OmpDependenceType) + READ_FEATURE(OmpDependenceType::Type) READ_FEATURE(OmpTaskDependenceType) READ_FEATURE(OmpTaskDependenceType::Type) - READ_FEATURE(OmpDependSinkVec) - READ_FEATURE(OmpDependSinkVecLength) + READ_FEATURE(OmpIteration) + READ_FEATURE(OmpIterationOffset) + READ_FEATURE(OmpIterationVector) READ_FEATURE(OmpEndAllocators) READ_FEATURE(OmpEndAtomic) READ_FEATURE(OmpEndBlockDirective) From d72089c5f0eba49c05fac95646e59aff0ec3ac4b Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Mon, 11 Nov 2024 06:16:32 -0600 Subject: [PATCH 7/8] Update flang/lib/Semantics/check-omp-structure.cpp Co-authored-by: Tom Eccles --- flang/lib/Semantics/check-omp-structure.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index cfa6155b5b713..6af402e707c3b 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -1664,7 +1664,7 @@ 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); + 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); From 2a94fd732c37d23d09bb6272574eb9d3e789d659 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Mon, 11 Nov 2024 06:35:15 -0600 Subject: [PATCH 8/8] use {} initialization in more places --- flang/lib/Semantics/check-omp-structure.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index 6af402e707c3b..2f8625746d366 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -941,9 +941,9 @@ void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &x) { dirContext_.pop_back(); assert(!loopStack_.empty() && "Expecting non-empty loop stack"); - const LoopConstruct &top = loopStack_.back(); + const LoopConstruct &top{loopStack_.back()}; #ifndef NDEBUG - auto *loopc = std::get_if(&top); + auto *loopc{std::get_if(&top)}; assert(loopc != nullptr && *loopc == &x && "Mismatched loop constructs"); #endif loopStack_.pop_back(); @@ -1633,8 +1633,8 @@ void OmpStructureChecker::ChecksOnOrderedAsStandalone() { 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++; }, @@ -1650,11 +1650,11 @@ void OmpStructureChecker::ChecksOnOrderedAsStandalone() { context_.Say(src, "At most one SOURCE dependence type can appear on the ORDERED directive"_err_en_US); } - }; + }}; // Visit the DEPEND and DOACROSS clauses. - auto depClauses = FindClauses(llvm::omp::Clause::OMPC_depend); - for (auto itr = depClauses.first; itr != depClauses.second; ++itr) { + 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)}) { @@ -1665,7 +1665,7 @@ void OmpStructureChecker::ChecksOnOrderedAsStandalone() { } } auto doaClauses{FindClauses(llvm::omp::Clause::OMPC_doacross)}; - for (auto itr = doaClauses.first; itr != doaClauses.second; ++itr) { + for (auto itr{doaClauses.first}; itr != doaClauses.second; ++itr) { auto &doaClause{std::get(itr->second->u)}; visitDoacross(doaClause.v.v, itr->second->source); } @@ -4405,7 +4405,7 @@ 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); + auto *doc{std::get_if(&top)}; assert(doc != nullptr && *doc == &x && "Mismatched loop constructs"); #endif loopStack_.pop_back();