From 0c5c452840a32490bdac36ef48e27d50cc1d8e39 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Wed, 23 Oct 2024 15:57:35 -0500 Subject: [PATCH 01/10] [flang][OpenMP] Update handling of DEPEND clause Parse the locator list in OmpDependClause as an OmpObjectList (instead of a list of Designators). When a common block appears in the locator list, show an informative message. Implement resolving symbols in DependSinkVec in a dedicated visitor instead of having a visitor for OmpDependClause. Resolve unresolved names common blocks in OmpObjectList. Minor changes to the code organization: - rename OmpDependenceType to OmpTaskDependenceType (to follow 5.2 terminology), - rename Depend::WithLocators to Depend::DepType, - add comments with more detailed spec references to parse-tree.h. --- .../FlangOmpReport/FlangOmpReportVisitor.cpp | 4 +-- .../FlangOmpReport/FlangOmpReportVisitor.h | 2 +- flang/include/flang/Parser/dump-parse-tree.h | 4 +-- flang/include/flang/Parser/parse-tree.h | 32 +++++++++++++------ flang/include/flang/Semantics/symbol.h | 11 +++++-- flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 10 +++--- flang/lib/Lower/OpenMP/Clauses.cpp | 15 ++++----- flang/lib/Parser/openmp-parsers.cpp | 10 +++--- flang/lib/Parser/unparse.cpp | 6 ++-- flang/lib/Semantics/check-omp-structure.cpp | 24 ++++++++------ flang/lib/Semantics/resolve-directives.cpp | 27 ++++++++++------ flang/test/Semantics/OpenMP/depend04.f90 | 10 ++++++ llvm/include/llvm/Frontend/OpenMP/ClauseT.h | 4 +-- 13 files changed, 98 insertions(+), 61 deletions(-) create mode 100644 flang/test/Semantics/OpenMP/depend04.f90 diff --git a/flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp b/flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp index 5d3c5cd72eef0..d28ed0534d600 100644 --- a/flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp +++ b/flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp @@ -222,9 +222,9 @@ void OpenMPCounterVisitor::Post(const OmpLinearModifier::Type &c) { clauseDetails += "modifier=" + std::string{OmpLinearModifier::EnumToString(c)} + ";"; } -void OpenMPCounterVisitor::Post(const OmpDependenceType::Type &c) { +void OpenMPCounterVisitor::Post(const OmpTaskDependenceType::Type &c) { clauseDetails += - "type=" + std::string{OmpDependenceType::EnumToString(c)} + ";"; + "type=" + std::string{OmpTaskDependenceType::EnumToString(c)} + ";"; } void OpenMPCounterVisitor::Post(const OmpMapClause::Type &c) { clauseDetails += "type=" + std::string{OmpMapClause::EnumToString(c)} + ";"; diff --git a/flang/examples/FlangOmpReport/FlangOmpReportVisitor.h b/flang/examples/FlangOmpReport/FlangOmpReportVisitor.h index 380534ebbfd70..68c52db46e2f0 100644 --- a/flang/examples/FlangOmpReport/FlangOmpReportVisitor.h +++ b/flang/examples/FlangOmpReport/FlangOmpReportVisitor.h @@ -73,7 +73,7 @@ struct OpenMPCounterVisitor { void Post(const OmpDeviceTypeClause::Type &c); void Post(const OmpScheduleModifierType::ModType &c); void Post(const OmpLinearModifier::Type &c); - void Post(const OmpDependenceType::Type &c); + void Post(const OmpTaskDependenceType::Type &c); void Post(const OmpMapClause::Type &c); void Post(const OmpScheduleClause::ScheduleType &c); void Post(const OmpIfClause::DirectiveNameModifier &c); diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h index 76d2f164fc4bf..1517539ff85f3 100644 --- a/flang/include/flang/Parser/dump-parse-tree.h +++ b/flang/include/flang/Parser/dump-parse-tree.h @@ -513,8 +513,8 @@ class ParseTreeDumper { 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, OmpEndAllocators) diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index c1884f6e88d1e..25e2f42ca29bd 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -3439,6 +3439,18 @@ struct OmpObject { WRAPPER_CLASS(OmpObjectList, std::list); +// 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, Source, Sink) + WRAPPER_CLASS_BOILERPLATE(OmpTaskDependenceType, Type); +}; + // [5.0] 2.1.6 iterator-specifier -> type-declaration-stmt = subscript-triple // iterator-modifier -> iterator-specifier-list struct OmpIteratorSpecifier { @@ -3534,27 +3546,27 @@ struct OmpDependSinkVecLength { std::tuple t; }; -// 2.13.9 depend-vec -> iterator [+/- depend-vec-length],...,iterator[...] +// 2.13.9 depend-vec -> induction-variable [depend-vec-length], ... struct OmpDependSinkVec { TUPLE_CLASS_BOILERPLATE(OmpDependSinkVec); std::tuple> t; }; -// 2.13.9 depend-type -> IN | OUT | INOUT | SOURCE | SINK -struct OmpDependenceType { - ENUM_CLASS(Type, In, Out, Inout, Source, Sink) - WRAPPER_CLASS_BOILERPLATE(OmpDependenceType, Type); -}; - -// 2.13.9 depend-clause -> DEPEND (((IN | OUT | INOUT) : variable-name-list) | -// SOURCE | SINK : depend-vec) +// 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-modifier -> iterator-modifier // since 5.0 struct OmpDependClause { UNION_CLASS_BOILERPLATE(OmpDependClause); EMPTY_CLASS(Source); WRAPPER_CLASS(Sink, std::list); struct InOut { TUPLE_CLASS_BOILERPLATE(InOut); - std::tuple> t; + std::tuple t; }; std::variant u; }; diff --git a/flang/include/flang/Semantics/symbol.h b/flang/include/flang/Semantics/symbol.h index 0767d8ea84bc6..db1c25285c080 100644 --- a/flang/include/flang/Semantics/symbol.h +++ b/flang/include/flang/Semantics/symbol.h @@ -24,6 +24,8 @@ #include #include +#include "llvm/Support/Signals.h" + namespace llvm { class raw_ostream; } @@ -753,9 +755,9 @@ class Symbol { // OpenMP miscellaneous flags OmpCommonBlock, OmpReduction, OmpAligned, OmpNontemporal, OmpAllocate, OmpDeclarativeAllocateDirective, OmpExecutableAllocateDirective, - OmpDeclareSimd, OmpDeclareTarget, OmpThreadprivate, OmpDeclareReduction, - OmpFlushed, OmpCriticalLock, OmpIfSpecified, OmpNone, OmpPreDetermined, - OmpImplicit); + OmpDependClause, OmpDeclareSimd, OmpDeclareTarget, OmpThreadprivate, + OmpDeclareReduction, OmpFlushed, OmpCriticalLock, OmpIfSpecified, OmpNone, + OmpPreDetermined, OmpImplicit); using Flags = common::EnumSet; const Scope &owner() const { return *owner_; } @@ -976,6 +978,7 @@ template class Symbols { } private: + static int counter; using blockType = std::array; std::list blocks_; std::size_t nextIndex_{0}; @@ -994,6 +997,8 @@ template class Symbols { } }; +template int Symbols::counter; + // Define a few member functions here in the header so that they // can be used by lib/Evaluate without inducing a dependence cycle // between the two shared libraries. diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp index fbc031f3a93d7..12fe3b572244f 100644 --- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp +++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp @@ -798,11 +798,11 @@ bool ClauseProcessor::processDepend(mlir::omp::DependClauseOps &result) const { return findRepeatableClause( [&](const omp::clause::Depend &clause, const parser::CharBlock &) { using Depend = omp::clause::Depend; - assert(std::holds_alternative(clause.u) && - "Only the modern form is handled at the moment"); - auto &modern = std::get(clause.u); - auto kind = std::get(modern.t); - auto &objects = std::get(modern.t); + assert(std::holds_alternative(clause.u) && + "Only the form with depenence type is handled at the moment"); + auto &depType = std::get(clause.u); + auto kind = std::get(depType.t); + auto &objects = std::get(depType.t); mlir::omp::ClauseTaskDependAttr dependTypeOperand = genDependKindAttr(firOpBuilder, kind); diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp index ee3d74a7c631a..57667fbbf7d4a 100644 --- a/flang/lib/Lower/OpenMP/Clauses.cpp +++ b/flang/lib/Lower/OpenMP/Clauses.cpp @@ -555,7 +555,7 @@ Depend make(const parser::OmpClause::Depend &inp, using Iteration = Doacross::Vector::value_type; // LoopIterationT CLAUSET_ENUM_CONVERT( // - convert1, parser::OmpDependenceType::Type, Depend::TaskDependenceType, + convert1, parser::OmpTaskDependenceType::Type, Depend::TaskDependenceType, // clang-format off MS(In, In) MS(Out, Out) @@ -593,17 +593,14 @@ Depend make(const parser::OmpClause::Depend &inp, return Doacross{{/*DependenceType=*/Doacross::DependenceType::Sink, /*Vector=*/makeList(s.v, convert2)}}; }, - // Depend::WithLocators + // Depend::DepType [&](const wrapped::InOut &s) -> Variant { - auto &t0 = std::get(s.t); - auto &t1 = std::get>(s.t); - auto convert4 = [&](const parser::Designator &t) { - return makeObject(t, semaCtx); - }; - return Depend::WithLocators{ + auto &t0 = std::get(s.t); + auto &t1 = std::get(s.t); + return Depend::DepType{ {/*TaskDependenceType=*/convert1(t0.v), /*Iterator=*/std::nullopt, - /*LocatorList=*/makeList(t1, convert4)}}; + /*LocatorList=*/makeObjects(t1, semaCtx)}}; }, }, inp.v.u)}; diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp index 59a8757e58e8c..3a19d44559dcc 100644 --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -365,10 +365,10 @@ TYPE_PARSER(construct( TYPE_PARSER( construct(name, maybe(Parser{}))) -TYPE_PARSER( - construct("IN"_id >> pure(OmpDependenceType::Type::In) || - "INOUT" >> pure(OmpDependenceType::Type::Inout) || - "OUT" >> pure(OmpDependenceType::Type::Out))) +TYPE_PARSER(construct( + "IN"_id >> pure(OmpTaskDependenceType::Type::In) || + "INOUT" >> pure(OmpTaskDependenceType::Type::Inout) || + "OUT" >> pure(OmpTaskDependenceType::Type::Out))) TYPE_CONTEXT_PARSER("Omp Depend clause"_en_US, construct(construct( @@ -376,7 +376,7 @@ TYPE_CONTEXT_PARSER("Omp Depend clause"_en_US, construct( construct("SOURCE"_tok)) || construct(construct( - Parser{}, ":" >> nonemptyList(designator)))) + Parser{}, ":" >> Parser{}))) // 2.15.3.7 LINEAR (linear-list: linear-step) // linear-list -> list | modifier(list) diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp index 04df988223e8f..1445468b4fb73 100644 --- a/flang/lib/Parser/unparse.cpp +++ b/flang/lib/Parser/unparse.cpp @@ -2206,9 +2206,9 @@ class UnparseVisitor { } void Unparse(const OmpDependClause::InOut &x) { Put("("); - Walk(std::get(x.t)); + Walk(std::get(x.t)); Put(":"); - Walk(std::get>(x.t), ","); + Walk(std::get(x.t)); Put(")"); } bool Pre(const OmpDependClause &x) { @@ -2816,7 +2816,7 @@ class UnparseVisitor { OmpLastprivateClause, LastprivateModifier) // OMP lastprivate-modifier WALK_NESTED_ENUM(OmpScheduleModifierType, ModType) // OMP schedule-modifier WALK_NESTED_ENUM(OmpLinearModifier, Type) // OMP linear-modifier - WALK_NESTED_ENUM(OmpDependenceType, Type) // OMP dependence-type + WALK_NESTED_ENUM(OmpTaskDependenceType, Type) // OMP task-dependence-type WALK_NESTED_ENUM(OmpScheduleClause, ScheduleType) // OMP schedule-type WALK_NESTED_ENUM(OmpDeviceClause, DeviceModifier) // OMP device modifier WALK_NESTED_ENUM(OmpDeviceTypeClause, Type) // OMP DEVICE_TYPE diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index 46486907ceb9e..892745c9c36b6 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -3283,15 +3283,21 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Depend &x) { parser::ToUpperCaseLetters(getDirectiveName(GetContext().directive))); } if (const auto *inOut{std::get_if(&x.v.u)}) { - const auto &designators{std::get>(inOut->t)}; - for (const auto &ele : designators) { - if (const auto *dataRef{std::get_if(&ele.u)}) { - CheckDependList(*dataRef); - if (const auto *arr{ - std::get_if>( - &dataRef->u)}) { - CheckArraySection(arr->value(), GetLastName(*dataRef), - llvm::omp::Clause::OMPC_depend); + for (const auto &object : std::get(inOut->t).v) { + if (const auto *name{std::get_if(&object.u)}) { + context_.Say(GetContext().clauseSource, + "Common block name ('%s') cannot appear in a DEPEND " + "clause"_err_en_US, + name->ToString()); + } else if (auto *designator{std::get_if(&object.u)}) { + if (auto *dataRef{std::get_if(&designator->u)}) { + CheckDependList(*dataRef); + if (const auto *arr{ + std::get_if>( + &dataRef->u)}) { + CheckArraySection(arr->value(), GetLastName(*dataRef), + llvm::omp::Clause::OMPC_depend); + } } } } diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index 33936ba4c2b34..1c3359cc9199d 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -25,6 +25,7 @@ #include #include +#include "llvm/Support/Signals.h" template static Fortran::semantics::Scope *GetScope( Fortran::semantics::SemanticsContext &context, const T &x) { @@ -435,6 +436,19 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor { bool Pre(const parser::OpenMPAllocatorsConstruct &); void Post(const parser::OpenMPAllocatorsConstruct &); + void Post(const parser::OmpObjectList &x) { + // The objects from OMP clauses should have already been resolved, + // except common blocks (the ResolveNamesVisitor does not visit + // parser::Name, those are dealt with as members of other structures). + // Iterate over elements of x, and resolve any common blocks that + // are still unresolved. + for (const parser::OmpObject &obj : x.v) { + if (auto *name{std::get_if(&obj.u)}) { + Resolve(*name, currScope().MakeCommonBlock(name->source)); + } + } + } + // 2.15.3 Data-Sharing Attribute Clauses void Post(const parser::OmpDefaultClause &); bool Pre(const parser::OmpClause::Shared &x) { @@ -531,16 +545,9 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor { return false; } - bool Pre(const parser::OmpDependClause &x) { - if (const auto *dependSink{ - std::get_if(&x.u)}) { - const auto &dependSinkVec{dependSink->v}; - for (const auto &dependSinkElement : dependSinkVec) { - const auto &name{std::get(dependSinkElement.t)}; - ResolveName(&name); - } - } - return false; + void Post(const parser::OmpDependSinkVec &x) { + const auto &name{std::get(x.t)}; + ResolveName(&name); } bool Pre(const parser::OmpClause::UseDevicePtr &x) { diff --git a/flang/test/Semantics/OpenMP/depend04.f90 b/flang/test/Semantics/OpenMP/depend04.f90 new file mode 100644 index 0000000000000..8bdddb017d2c9 --- /dev/null +++ b/flang/test/Semantics/OpenMP/depend04.f90 @@ -0,0 +1,10 @@ +!RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=50 + +subroutine f00 + integer :: x + common /cc/ x +!ERROR: Common block name ('cc') cannot appear in a DEPEND clause + !$omp task depend(in: /cc/) + x = 0 + !$omp end task +end diff --git a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h index ac34ddafc5e72..2b8e56ed246e5 100644 --- a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h +++ b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h @@ -503,7 +503,7 @@ struct DependT { using LocatorList = ObjectListT; using TaskDependenceType = tomp::type::TaskDependenceType; - struct WithLocators { // Modern form + struct DepType { // The form with task dependence type. using TupleTrait = std::true_type; // Empty LocatorList means "omp_all_memory". std::tuple t; @@ -511,7 +511,7 @@ struct DependT { 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 From eff3d013058c2125bc24b45d704fa1b434909152 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Thu, 24 Oct 2024 14:48:51 -0500 Subject: [PATCH 02/10] [flang][OpenMP] Extract OMP version hint into helper functions, NFC --- flang/lib/Semantics/check-omp-structure.cpp | 24 +++++++++++++-------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index 892745c9c36b6..e9a2fcecd9efe 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -38,6 +38,16 @@ namespace Fortran::semantics { CheckAllowedClause(llvm::omp::Y); \ } +std::string ThisVersion(unsigned version) { + std::string tv{ + std::to_string(version / 10) + "." + std::to_string(version % 10)}; + return "OpenMP v" + tv; +} + +std::string TryVersion(unsigned version) { + return "try -fopenmp-version=" + std::to_string(version); +} + // 'OmpWorkshareBlockChecker' is used to check the validity of the assignment // statements and the expressions enclosed in an OpenMP Workshare construct class OmpWorkshareBlockChecker { @@ -200,14 +210,10 @@ bool OmpStructureChecker::CheckAllowedClause(llvmOmpClause clause) { auto clauseName{parser::ToUpperCaseLetters(getClauseName(clause).str())}; auto dirName{parser::ToUpperCaseLetters(getDirectiveName(dir).str())}; - std::string thisVersion{ - std::to_string(version / 10) + "." + std::to_string(version % 10)}; - std::string goodVersion{std::to_string(allowedInVersion)}; - context_.Say(dirCtx.clauseSource, - "%s clause is not allowed on directive %s in OpenMP v%s, " - "try -fopenmp-version=%d"_err_en_US, - clauseName, dirName, thisVersion, allowedInVersion); + "%s clause is not allowed on directive %s in %s, %s"_err_en_US, + clauseName, dirName, ThisVersion(version), + TryVersion(allowedInVersion)); } } return CheckAllowed(clause); @@ -3373,8 +3379,8 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Lastprivate &x) { std::to_string(version / 10) + "." + std::to_string(version % 10)}; context_.Say(GetContext().clauseSource, "LASTPRIVATE clause with CONDITIONAL modifier is not " - "allowed in OpenMP v%s, try -fopenmp-version=%d"_err_en_US, - thisVersion, allowedInVersion); + "allowed in %s, %s"_err_en_US, + ThisVersion(version), TryVersion(allowedInVersion)); } } } From c9cd78239a05a4741d0f15c49cecd464b7943b47 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Thu, 24 Oct 2024 14:58:39 -0500 Subject: [PATCH 03/10] [flang][OpenMP] Parsing support for iterator in DEPEND clause Warn about use of iterators OpenMP versions that didn't have them (support added in 5.0). Emit a TODO error in lowering. --- flang/include/flang/Parser/parse-tree.h | 4 +- flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 62 ++++++++++--------- flang/lib/Lower/OpenMP/Clauses.cpp | 16 +++-- flang/lib/Parser/openmp-parsers.cpp | 3 +- flang/lib/Semantics/check-omp-structure.cpp | 9 +++ .../test/Lower/OpenMP/Todo/depend-clause.f90 | 10 +++ flang/test/Semantics/OpenMP/depend05.f90 | 9 +++ 7 files changed, 77 insertions(+), 36 deletions(-) create mode 100644 flang/test/Lower/OpenMP/Todo/depend-clause.f90 create mode 100644 flang/test/Semantics/OpenMP/depend05.f90 diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index 25e2f42ca29bd..227fb50fcf7e7 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -3566,7 +3566,9 @@ struct OmpDependClause { WRAPPER_CLASS(Sink, std::list); struct InOut { TUPLE_CLASS_BOILERPLATE(InOut); - std::tuple t; + std::tuple, OmpTaskDependenceType, + OmpObjectList> + t; }; std::variant u; }; diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp index 12fe3b572244f..c71fbded443de 100644 --- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp +++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp @@ -795,35 +795,41 @@ bool ClauseProcessor::processCopyprivate( bool ClauseProcessor::processDepend(mlir::omp::DependClauseOps &result) const { fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder(); - return findRepeatableClause( - [&](const omp::clause::Depend &clause, const parser::CharBlock &) { - using Depend = omp::clause::Depend; - assert(std::holds_alternative(clause.u) && - "Only the form with depenence type is handled at the moment"); - auto &depType = std::get(clause.u); - auto kind = std::get(depType.t); - auto &objects = std::get(depType.t); - - mlir::omp::ClauseTaskDependAttr dependTypeOperand = - genDependKindAttr(firOpBuilder, kind); - result.dependKinds.append(objects.size(), dependTypeOperand); - - for (const omp::Object &object : objects) { - assert(object.ref() && "Expecting designator"); - - if (evaluate::ExtractSubstring(*object.ref())) { - TODO(converter.getCurrentLocation(), - "substring not supported for task depend"); - } else if (evaluate::IsArrayElement(*object.ref())) { - TODO(converter.getCurrentLocation(), - "array sections not supported for task depend"); - } + auto process = [&](const omp::clause::Depend &clause, + const parser::CharBlock &) { + using Depend = omp::clause::Depend; + assert(std::holds_alternative(clause.u) && + "Only the form with depenence type is handled at the moment"); + auto &depType = std::get(clause.u); + auto kind = std::get(depType.t); + auto &objects = std::get(depType.t); + + if (std::get>(depType.t)) { + TODO(converter.getCurrentLocation(), + "Support for iterator modifiers is not implemented yet"); + } + mlir::omp::ClauseTaskDependAttr dependTypeOperand = + genDependKindAttr(firOpBuilder, kind); + result.dependKinds.append(objects.size(), dependTypeOperand); + + for (const omp::Object &object : objects) { + assert(object.ref() && "Expecting designator"); + + if (evaluate::ExtractSubstring(*object.ref())) { + TODO(converter.getCurrentLocation(), + "substring not supported for task depend"); + } else if (evaluate::IsArrayElement(*object.ref())) { + TODO(converter.getCurrentLocation(), + "array sections not supported for task depend"); + } - semantics::Symbol *sym = object.sym(); - const mlir::Value variable = converter.getSymbolAddress(*sym); - result.dependVars.push_back(variable); - } - }); + semantics::Symbol *sym = object.sym(); + const mlir::Value variable = converter.getSymbolAddress(*sym); + result.dependVars.push_back(variable); + } + }; + + return findRepeatableClause(process); } bool ClauseProcessor::processHasDeviceAddr( diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp index 57667fbbf7d4a..21a246815782c 100644 --- a/flang/lib/Lower/OpenMP/Clauses.cpp +++ b/flang/lib/Lower/OpenMP/Clauses.cpp @@ -595,12 +595,16 @@ Depend make(const parser::OmpClause::Depend &inp, }, // Depend::DepType [&](const wrapped::InOut &s) -> Variant { - auto &t0 = std::get(s.t); - auto &t1 = std::get(s.t); - return Depend::DepType{ - {/*TaskDependenceType=*/convert1(t0.v), - /*Iterator=*/std::nullopt, - /*LocatorList=*/makeObjects(t1, semaCtx)}}; + 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=*/convert1(t1.v), + /*Iterator=*/std::move(maybeIter), + /*LocatorList=*/makeObjects(t2, semaCtx)}}; }, }, inp.v.u)}; diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp index 3a19d44559dcc..869419b00bf86 100644 --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -376,7 +376,8 @@ TYPE_CONTEXT_PARSER("Omp Depend clause"_en_US, construct( construct("SOURCE"_tok)) || construct(construct( - Parser{}, ":" >> Parser{}))) + maybe(Parser{} / ","_tok), + Parser{} / ":", Parser{}))) // 2.15.3.7 LINEAR (linear-list: linear-step) // linear-list -> list | modifier(list) diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index e9a2fcecd9efe..99f208f1280c2 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -3307,6 +3307,15 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Depend &x) { } } } + if (std::get>(inOut->t)) { + unsigned version{context_.langOptions().OpenMPVersion}; + 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)); + } + } } } diff --git a/flang/test/Lower/OpenMP/Todo/depend-clause.f90 b/flang/test/Lower/OpenMP/Todo/depend-clause.f90 new file mode 100644 index 0000000000000..74525888c91d6 --- /dev/null +++ b/flang/test/Lower/OpenMP/Todo/depend-clause.f90 @@ -0,0 +1,10 @@ +!RUN: %not_todo_cmd bbc -emit-hlfir -fopenmp -fopenmp-version=50 -o - %s 2>&1 | FileCheck %s +!RUN: %not_todo_cmd %flang_fc1 -emit-hlfir -fopenmp -fopenmp-version=50 -o - %s 2>&1 | FileCheck %s + +!CHECK: Support for iterator modifiers is not implemented yet +subroutine f00(x) + integer :: x(10) + !$omp task depend(iterator(i = 1:10), in: x(i)) + x = 0 + !$omp end task +end diff --git a/flang/test/Semantics/OpenMP/depend05.f90 b/flang/test/Semantics/OpenMP/depend05.f90 new file mode 100644 index 0000000000000..53fd82bd08a9e --- /dev/null +++ b/flang/test/Semantics/OpenMP/depend05.f90 @@ -0,0 +1,9 @@ +!RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=45 -Werror + +subroutine f00(x) + integer :: x(10) +!WARNING: Iterator modifiers are not supported in OpenMP v4.5, try -fopenmp-version=50 + !$omp task depend(iterator(i = 1:10), in: x(i)) + x = 0 + !$omp end task +end From 72c8f7df8e993ffa8d4432b10197885b03c540d8 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Thu, 24 Oct 2024 16:47:08 -0500 Subject: [PATCH 04/10] undo leftover changes in symbol.h --- flang/include/flang/Semantics/symbol.h | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/flang/include/flang/Semantics/symbol.h b/flang/include/flang/Semantics/symbol.h index db1c25285c080..0767d8ea84bc6 100644 --- a/flang/include/flang/Semantics/symbol.h +++ b/flang/include/flang/Semantics/symbol.h @@ -24,8 +24,6 @@ #include #include -#include "llvm/Support/Signals.h" - namespace llvm { class raw_ostream; } @@ -755,9 +753,9 @@ class Symbol { // OpenMP miscellaneous flags OmpCommonBlock, OmpReduction, OmpAligned, OmpNontemporal, OmpAllocate, OmpDeclarativeAllocateDirective, OmpExecutableAllocateDirective, - OmpDependClause, OmpDeclareSimd, OmpDeclareTarget, OmpThreadprivate, - OmpDeclareReduction, OmpFlushed, OmpCriticalLock, OmpIfSpecified, OmpNone, - OmpPreDetermined, OmpImplicit); + OmpDeclareSimd, OmpDeclareTarget, OmpThreadprivate, OmpDeclareReduction, + OmpFlushed, OmpCriticalLock, OmpIfSpecified, OmpNone, OmpPreDetermined, + OmpImplicit); using Flags = common::EnumSet; const Scope &owner() const { return *owner_; } @@ -978,7 +976,6 @@ template class Symbols { } private: - static int counter; using blockType = std::array; std::list blocks_; std::size_t nextIndex_{0}; @@ -997,8 +994,6 @@ template class Symbols { } }; -template int Symbols::counter; - // Define a few member functions here in the header so that they // can be used by lib/Evaluate without inducing a dependence cycle // between the two shared libraries. From 09e6720758749e0f9c607f418e7c58979c2abe89 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Thu, 24 Oct 2024 16:47:27 -0500 Subject: [PATCH 05/10] format --- flang/lib/Lower/OpenMP/Clauses.cpp | 7 +++---- llvm/include/llvm/Frontend/OpenMP/ClauseT.h | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp index 57667fbbf7d4a..bcff82c2c1787 100644 --- a/flang/lib/Lower/OpenMP/Clauses.cpp +++ b/flang/lib/Lower/OpenMP/Clauses.cpp @@ -597,10 +597,9 @@ Depend make(const parser::OmpClause::Depend &inp, [&](const wrapped::InOut &s) -> Variant { auto &t0 = std::get(s.t); auto &t1 = std::get(s.t); - return Depend::DepType{ - {/*TaskDependenceType=*/convert1(t0.v), - /*Iterator=*/std::nullopt, - /*LocatorList=*/makeObjects(t1, semaCtx)}}; + return Depend::DepType{{/*TaskDependenceType=*/convert1(t0.v), + /*Iterator=*/std::nullopt, + /*LocatorList=*/makeObjects(t1, semaCtx)}}; }, }, inp.v.u)}; diff --git a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h index 2b8e56ed246e5..2a890905dc632 100644 --- a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h +++ b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h @@ -503,7 +503,7 @@ struct DependT { using LocatorList = ObjectListT; using TaskDependenceType = tomp::type::TaskDependenceType; - struct DepType { // The form with task dependence type. + struct DepType { // The form with task dependence type. using TupleTrait = std::true_type; // Empty LocatorList means "omp_all_memory". std::tuple t; From 952e3736fab15a043b65a7f782e7a7d33f185d63 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Thu, 24 Oct 2024 16:50:13 -0500 Subject: [PATCH 06/10] remove leftover include --- flang/lib/Semantics/resolve-directives.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index 1c3359cc9199d..7ffbcbacdc96b 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -25,7 +25,6 @@ #include #include -#include "llvm/Support/Signals.h" template static Fortran::semantics::Scope *GetScope( Fortran::semantics::SemanticsContext &context, const T &x) { From 5c99093aede560d08101b940354653c8e2a32db8 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Thu, 24 Oct 2024 17:04:38 -0500 Subject: [PATCH 07/10] only call ResolveName for unresolved common block names --- flang/lib/Semantics/resolve-directives.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index 7ffbcbacdc96b..a403aedc238ab 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -442,7 +442,8 @@ class OmpAttributeVisitor : DirectiveAttributeVisitor { // Iterate over elements of x, and resolve any common blocks that // are still unresolved. for (const parser::OmpObject &obj : x.v) { - if (auto *name{std::get_if(&obj.u)}) { + auto *name{std::get_if(&obj.u)}; + if (name && !name->symbol) { Resolve(*name, currScope().MakeCommonBlock(name->source)); } } From 1672f0be95e69818f4bf4e6c34c7d60fe8b68d92 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Thu, 24 Oct 2024 18:03:20 -0500 Subject: [PATCH 08/10] missed OmpDependenceType -> OmpTaskDependenceType --- flang/examples/FeatureList/FeatureList.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/flang/examples/FeatureList/FeatureList.cpp b/flang/examples/FeatureList/FeatureList.cpp index 06ca12a492d29..d791b9af818c7 100644 --- a/flang/examples/FeatureList/FeatureList.cpp +++ b/flang/examples/FeatureList/FeatureList.cpp @@ -473,8 +473,8 @@ struct NodeVisitor { READ_FEATURE(OmpDependClause::InOut) READ_FEATURE(OmpDependClause::Sink) READ_FEATURE(OmpDependClause::Source) - READ_FEATURE(OmpDependenceType) - READ_FEATURE(OmpDependenceType::Type) + READ_FEATURE(OmpTaskDependenceType) + READ_FEATURE(OmpTaskDependenceType::Type) READ_FEATURE(OmpDependSinkVec) READ_FEATURE(OmpDependSinkVecLength) READ_FEATURE(OmpEndAllocators) From 97be5c65b3f7fc3ef7d99c8ba6f25c2a7f4f0415 Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Tue, 29 Oct 2024 06:57:37 -0500 Subject: [PATCH 09/10] convert assertion to todo --- flang/lib/Lower/OpenMP/ClauseProcessor.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp index c71fbded443de..7c254ce673855 100644 --- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp +++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp @@ -798,8 +798,10 @@ bool ClauseProcessor::processDepend(mlir::omp::DependClauseOps &result) const { auto process = [&](const omp::clause::Depend &clause, const parser::CharBlock &) { using Depend = omp::clause::Depend; - assert(std::holds_alternative(clause.u) && - "Only the form with depenence type is handled at the moment"); + 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); From 1e0a085ca29e69b285c295778c333d0889411b8e Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Tue, 29 Oct 2024 06:58:13 -0500 Subject: [PATCH 10/10] use brace initialization --- 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 99f208f1280c2..368c6dee44e5c 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -3309,7 +3309,7 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Depend &x) { } if (std::get>(inOut->t)) { unsigned version{context_.langOptions().OpenMPVersion}; - unsigned allowedInVersion = 50; + unsigned allowedInVersion{50}; if (version < allowedInVersion) { context_.Say(GetContext().clauseSource, "Iterator modifiers are not supported in %s, %s"_warn_en_US,