From 03cbe42627c7a7940b47cc1a2cda0120bc9c6d5e Mon Sep 17 00:00:00 2001 From: Krzysztof Parzyszek Date: Thu, 12 Dec 2024 12:19:35 -0600 Subject: [PATCH] [flang][OpenMP] Rework LINEAR clause (#119278) The OmpLinearClause class was a variant of two classes, one for when the linear modifier was present, and one for when it was absent. These two classes did not follow the conventions for parse tree nodes, (i.e. tuple/wrapper/union formats), which necessitated specialization of the parse tree visitor. The new form of OmpLinearClause is the standard tuple with a list of modifiers and an object list. The specialization of parse tree visitor for it has been removed. Parsing and unparsing of the new form bears additional complexity due to syntactical differences between OpenMP 5.2 and prior versions: in OpenMP 5.2 the argument list is post-modified, while in the prior versions, the step modifier was a post-modifier while the linear modifier had an unusual syntax of `modifier(list)`. With this change the LINEAR clause is no different from any other clauses in terms of its structure and use of modifiers. Modifier validation and all other checks work the same as with other clauses. --- flang/examples/FeatureList/FeatureList.cpp | 3 +- flang/include/flang/Parser/dump-parse-tree.h | 5 +- .../include/flang/Parser/parse-tree-visitor.h | 34 ---- flang/include/flang/Parser/parse-tree.h | 56 +++--- .../flang/Semantics/openmp-modifiers.h | 2 + flang/lib/Lower/OpenMP/Clauses.cpp | 37 ++-- flang/lib/Parser/openmp-parsers.cpp | 63 +++++- flang/lib/Parser/unparse.cpp | 62 +++++- flang/lib/Semantics/check-omp-structure.cpp | 186 ++++++++---------- flang/lib/Semantics/openmp-modifiers.cpp | 33 ++++ flang/lib/Semantics/resolve-directives.cpp | 15 +- flang/test/Parser/OpenMP/linear-clause.f90 | 117 +++++++++++ .../Semantics/OpenMP/clause-validity01.f90 | 8 + .../test/Semantics/OpenMP/linear-clause01.f90 | 12 +- .../test/Semantics/OpenMP/linear-clause02.f90 | 13 ++ flang/test/Semantics/OpenMP/linear-iter.f90 | 14 +- llvm/include/llvm/Frontend/OpenMP/ClauseT.h | 6 +- .../Frontend/OpenMPDecompositionTest.cpp | 15 +- 18 files changed, 448 insertions(+), 233 deletions(-) create mode 100644 flang/test/Parser/OpenMP/linear-clause.f90 create mode 100644 flang/test/Semantics/OpenMP/linear-clause02.f90 diff --git a/flang/examples/FeatureList/FeatureList.cpp b/flang/examples/FeatureList/FeatureList.cpp index 41a625520797..3a689c335c81 100644 --- a/flang/examples/FeatureList/FeatureList.cpp +++ b/flang/examples/FeatureList/FeatureList.cpp @@ -495,8 +495,7 @@ public: READ_FEATURE(OmpIfClause::Modifier) READ_FEATURE(OmpDirectiveNameModifier) READ_FEATURE(OmpLinearClause) - READ_FEATURE(OmpLinearClause::WithModifier) - READ_FEATURE(OmpLinearClause::WithoutModifier) + READ_FEATURE(OmpLinearClause::Modifier) READ_FEATURE(OmpLinearModifier) READ_FEATURE(OmpLinearModifier::Value) READ_FEATURE(OmpLoopDirective) diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h index 6321ce02e78d..7c0f04091362 100644 --- a/flang/include/flang/Parser/dump-parse-tree.h +++ b/flang/include/flang/Parser/dump-parse-tree.h @@ -559,10 +559,11 @@ public: NODE(parser, OmpLastprivateModifier) NODE_ENUM(OmpLastprivateModifier, Value) NODE(parser, OmpLinearClause) - NODE(OmpLinearClause, WithModifier) - NODE(OmpLinearClause, WithoutModifier) + NODE(OmpLinearClause, Modifier) NODE(parser, OmpLinearModifier) NODE_ENUM(OmpLinearModifier, Value) + NODE(parser, OmpStepComplexModifier) + NODE(parser, OmpStepSimpleModifier) NODE(parser, OmpLoopDirective) NODE(parser, OmpMapClause) NODE(OmpMapClause, Modifier) diff --git a/flang/include/flang/Parser/parse-tree-visitor.h b/flang/include/flang/Parser/parse-tree-visitor.h index e1ea4d459f4a..af1d34ae804f 100644 --- a/flang/include/flang/Parser/parse-tree-visitor.h +++ b/flang/include/flang/Parser/parse-tree-visitor.h @@ -897,40 +897,6 @@ struct ParseTreeVisitorLookupScope { mutator.Post(x); } } - template - static void Walk(const OmpLinearClause::WithModifier &x, V &visitor) { - if (visitor.Pre(x)) { - Walk(x.modifier, visitor); - Walk(x.names, visitor); - Walk(x.step, visitor); - visitor.Post(x); - } - } - template - static void Walk(OmpLinearClause::WithModifier &x, M &mutator) { - if (mutator.Pre(x)) { - Walk(x.modifier, mutator); - Walk(x.names, mutator); - Walk(x.step, mutator); - mutator.Post(x); - } - } - template - static void Walk(const OmpLinearClause::WithoutModifier &x, V &visitor) { - if (visitor.Pre(x)) { - Walk(x.names, visitor); - Walk(x.step, visitor); - visitor.Post(x); - } - } - template - static void Walk(OmpLinearClause::WithoutModifier &x, M &mutator) { - if (mutator.Pre(x)) { - Walk(x.names, mutator); - Walk(x.step, mutator); - mutator.Post(x); - } - } }; } // namespace detail diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h index db0d73fb6be1..8086c3103101 100644 --- a/flang/include/flang/Parser/parse-tree.h +++ b/flang/include/flang/Parser/parse-tree.h @@ -3699,6 +3699,22 @@ struct OmpReductionModifier { WRAPPER_CLASS_BOILERPLATE(OmpReductionModifier, Value); }; +// Ref: [5.2:117-120] +// +// step-complex-modifier -> +// STEP(integer-expression) // since 5.2 +struct OmpStepComplexModifier { + WRAPPER_CLASS_BOILERPLATE(OmpStepComplexModifier, ScalarIntExpr); +}; + +// Ref: [4.5:207-210], [5.0:290-293], [5.1:323-325], [5.2:117-120] +// +// step-simple-modifier -> +// integer-expresion // since 4.5 +struct OmpStepSimpleModifier { + WRAPPER_CLASS_BOILERPLATE(OmpStepSimpleModifier, ScalarIntExpr); +}; + // 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 @@ -3934,7 +3950,7 @@ struct OmpFailClause { struct OmpFromClause { TUPLE_CLASS_BOILERPLATE(OmpFromClause); MODIFIER_BOILERPLATE(OmpExpectation, OmpIterator, OmpMapper); - std::tuple t; + std::tuple t; }; // Ref: [4.5:87-91], [5.0:140-146], [5.1:166-171], [5.2:269] @@ -3981,28 +3997,20 @@ struct OmpLastprivateClause { std::tuple t; }; -// 2.15.3.7 linear-clause -> LINEAR (linear-list[ : linear-step]) -// linear-list -> list | linear-modifier(list) +// Ref: [4.5:207-210], [5.0:290-293], [5.1:323-325], [5.2:117-120] +// +// linear-clause -> +// LINEAR(list [: step-simple-modifier]) | // since 4.5 +// LINEAR(linear-modifier(list) +// [: step-simple-modifier]) | // since 4.5, until 5.2[*] +// LINEAR(list [: linear-modifier, +// step-complex-modifier]) // since 5.2 +// [*] Still allowed in 5.2 when on DECLARE SIMD, but deprecated. struct OmpLinearClause { - UNION_CLASS_BOILERPLATE(OmpLinearClause); - struct WithModifier { - BOILERPLATE(WithModifier); - WithModifier(OmpLinearModifier &&m, std::list &&n, - std::optional &&s) - : modifier(std::move(m)), names(std::move(n)), step(std::move(s)) {} - OmpLinearModifier modifier; - std::list names; - std::optional step; - }; - struct WithoutModifier { - BOILERPLATE(WithoutModifier); - WithoutModifier( - std::list &&n, std::optional &&s) - : names(std::move(n)), step(std::move(s)) {} - std::list names; - std::optional step; - }; - std::variant u; + TUPLE_CLASS_BOILERPLATE(OmpLinearClause); + MODIFIER_BOILERPLATE( + OmpLinearModifier, OmpStepSimpleModifier, OmpStepComplexModifier); + std::tuple t; }; // Ref: [4.5:216-219], [5.0:315-324], [5.1:347-355], [5.2:150-158] @@ -4017,7 +4025,7 @@ struct OmpLinearClause { struct OmpMapClause { TUPLE_CLASS_BOILERPLATE(OmpMapClause); MODIFIER_BOILERPLATE(OmpMapTypeModifier, OmpMapper, OmpIterator, OmpMapType); - std::tuple t; + std::tuple t; }; // Ref: [4.5:87-91], [5.0:140-146], [5.1:166-171], [5.2:270] @@ -4105,7 +4113,7 @@ struct OmpTaskReductionClause { struct OmpToClause { TUPLE_CLASS_BOILERPLATE(OmpToClause); MODIFIER_BOILERPLATE(OmpExpectation, OmpIterator, OmpMapper); - std::tuple t; + std::tuple t; }; // Ref: [5.0:254-255], [5.1:287-288], [5.2:321-322] diff --git a/flang/include/flang/Semantics/openmp-modifiers.h b/flang/include/flang/Semantics/openmp-modifiers.h index 4025ce112d9c..5d5c5e97faf4 100644 --- a/flang/include/flang/Semantics/openmp-modifiers.h +++ b/flang/include/flang/Semantics/openmp-modifiers.h @@ -87,6 +87,8 @@ DECLARE_DESCRIPTOR(parser::OmpOrderingModifier); DECLARE_DESCRIPTOR(parser::OmpPrescriptiveness); DECLARE_DESCRIPTOR(parser::OmpReductionIdentifier); DECLARE_DESCRIPTOR(parser::OmpReductionModifier); +DECLARE_DESCRIPTOR(parser::OmpStepComplexModifier); +DECLARE_DESCRIPTOR(parser::OmpStepSimpleModifier); DECLARE_DESCRIPTOR(parser::OmpTaskDependenceType); DECLARE_DESCRIPTOR(parser::OmpVariableCategory); diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp index a0dc1be0afc5..b424e209d56d 100644 --- a/flang/lib/Lower/OpenMP/Clauses.cpp +++ b/flang/lib/Lower/OpenMP/Clauses.cpp @@ -899,8 +899,6 @@ Lastprivate make(const parser::OmpClause::Lastprivate &inp, Linear make(const parser::OmpClause::Linear &inp, semantics::SemanticsContext &semaCtx) { // inp.v -> parser::OmpLinearClause - using wrapped = parser::OmpLinearClause; - CLAUSET_ENUM_CONVERT( // convert, parser::OmpLinearModifier::Value, Linear::LinearModifier, // clang-format off @@ -910,26 +908,23 @@ Linear make(const parser::OmpClause::Linear &inp, // clang-format on ); - using Tuple = decltype(Linear::t); + auto &mods = semantics::OmpGetModifiers(inp.v); + auto *m0 = + semantics::OmpGetUniqueModifier(mods); + auto *m1 = + semantics::OmpGetUniqueModifier(mods); + assert((!m0 || !m1) && "Simple and complex modifiers both present"); - return Linear{Fortran::common::visit( - common::visitors{ - [&](const wrapped::WithModifier &s) -> Tuple { - return { - /*StepSimpleModifier=*/std::nullopt, - /*StepComplexModifier=*/maybeApply(makeExprFn(semaCtx), s.step), - /*LinearModifier=*/convert(s.modifier.v), - /*List=*/makeList(s.names, makeObjectFn(semaCtx))}; - }, - [&](const wrapped::WithoutModifier &s) -> Tuple { - return { - /*StepSimpleModifier=*/maybeApply(makeExprFn(semaCtx), s.step), - /*StepComplexModifier=*/std::nullopt, - /*LinearModifier=*/std::nullopt, - /*List=*/makeList(s.names, makeObjectFn(semaCtx))}; - }, - }, - inp.v.u)}; + auto *m2 = semantics::OmpGetUniqueModifier(mods); + auto &t1 = std::get(inp.v.t); + + auto &&maybeStep = m0 ? maybeApplyToV(makeExprFn(semaCtx), m0) + : m1 ? maybeApplyToV(makeExprFn(semaCtx), m1) + : std::optional{}; + + return Linear{{/*StepComplexModifier=*/std::move(maybeStep), + /*LinearModifier=*/maybeApplyToV(convert, m2), + /*List=*/makeObjects(t1, semaCtx)}}; } Link make(const parser::OmpClause::Link &inp, diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp index 36d0a0240399..7d10de8c6097 100644 --- a/flang/lib/Parser/openmp-parsers.cpp +++ b/flang/lib/Parser/openmp-parsers.cpp @@ -99,6 +99,25 @@ constexpr ModifierList modifierList(Separator sep) { return ModifierList(sep); } +// Parse the input as any modifier from ClauseTy, but only succeed if +// the result was the SpecificTy. It requires that SpecificTy is one +// of the alternatives in ClauseTy::Modifier. +// The reason to have this is that ClauseTy::Modifier has "source", +// while specific modifiers don't. This class allows to parse a specific +// modifier together with obtaining its location. +template +struct SpecificModifierParser { + using resultType = typename ClauseTy::Modifier; + std::optional Parse(ParseState &state) const { + if (auto result{attempt(Parser{}).Parse(state)}) { + if (std::holds_alternative(result->u)) { + return result; + } + } + return std::nullopt; + } +}; + // OpenMP Clauses // [5.0] 2.1.6 iterator-specifier -> type-declaration-stmt = subscript-triple | @@ -232,6 +251,11 @@ TYPE_PARSER(construct( "TASK" >> pure(OmpReductionModifier::Value::Task) || "DEFAULT" >> pure(OmpReductionModifier::Value::Default))) +TYPE_PARSER(construct( // + "STEP" >> parenthesized(scalarIntExpr))) + +TYPE_PARSER(construct(scalarIntExpr)) + TYPE_PARSER(construct( "DEPOBJ" >> pure(OmpTaskDependenceType::Value::Depobj) || "IN"_id >> pure(OmpTaskDependenceType::Value::In) || @@ -288,6 +312,11 @@ TYPE_PARSER(sourced(construct( TYPE_PARSER(sourced(construct( Parser{}))) +TYPE_PARSER(sourced( + construct(Parser{}) || + construct(Parser{}) || + construct(Parser{}))) + TYPE_PARSER(sourced(construct( sourced(construct(Parser{}) || construct(Parser{}) || @@ -471,13 +500,33 @@ TYPE_PARSER(construct( applyFunction(makeMobClause, modifierList(maybe(","_tok)), Parser{}))) -TYPE_CONTEXT_PARSER("Omp LINEAR clause"_en_US, - construct( - construct(construct( - Parser{}, parenthesized(nonemptyList(name)), - maybe(":" >> scalarIntConstantExpr))) || - construct(construct( - nonemptyList(name), maybe(":" >> scalarIntConstantExpr))))) +OmpLinearClause makeLinearFromOldSyntax(OmpLinearClause::Modifier &&lm, + OmpObjectList &&objs, std::optional &&ssm) { + std::list mods; + mods.emplace_back(std::move(lm)); + if (ssm) { + mods.emplace_back(std::move(*ssm)); + } + return OmpLinearClause{std::move(objs), + mods.empty() ? decltype(mods){} : std::move(mods), + /*PostModified=*/false}; +} + +TYPE_PARSER( + // Parse the "modifier(x)" first, because syntacticaly it will match + // an array element (i.e. a list item). + // LINEAR(linear-modifier(list) [: step-simple-modifier]) + construct( // + applyFunction(makeLinearFromOldSyntax, + SpecificModifierParser{}, + parenthesized(Parser{}), + maybe(":"_tok >> SpecificModifierParser{}))) || + // LINEAR(list [: modifiers]) + construct( // + Parser{}, + maybe(":"_tok >> nonemptyList(Parser{})), + /*PostModified=*/pure(true))) // OpenMPv5.2 12.5.2 detach-clause -> DETACH (event-handle) TYPE_PARSER(construct(Parser{})) diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp index 95985569eee8..4b8e2624e36c 100644 --- a/flang/lib/Parser/unparse.cpp +++ b/flang/lib/Parser/unparse.cpp @@ -2133,13 +2133,63 @@ public: Walk(std::get>>(x.t), ": "); Walk(std::get(x.t)); } - void Unparse(const OmpLinearClause::WithoutModifier &x) { - Walk(x.names, ", "); - Walk(":", x.step); + void Unparse(const OmpStepSimpleModifier &x) { Walk(x.v); } + void Unparse(const OmpStepComplexModifier &x) { + Word("STEP("); + Walk(x.v); + Put(")"); } - void Unparse(const OmpLinearClause::WithModifier &x) { - Walk(x.modifier), Put("("), Walk(x.names, ","), Put(")"); - Walk(":", x.step); + void Unparse(const OmpLinearClause &x) { + using Modifier = OmpLinearClause::Modifier; + auto &modifiers{std::get>>(x.t)}; + if (std::get(x.t)) { // PostModified + Walk(std::get(x.t)); + Walk(": ", modifiers); + } else { + // Unparse using pre-5.2 syntax. + bool HasStepModifier{false}, HasLinearModifier{false}; + + if (modifiers) { + bool NeedComma{false}; + for (const Modifier &m : *modifiers) { + // Print all linear modifiers in case we need to unparse an + // incorrect tree. + if (auto *lmod{std::get_if(&m.u)}) { + if (NeedComma) { + Put(","); + } + Walk(*lmod); + HasLinearModifier = true; + NeedComma = true; + } else { + // If not linear-modifier, then it has to be step modifier. + HasStepModifier = true; + } + } + } + + if (HasLinearModifier) { + Put("("); + } + Walk(std::get(x.t)); + if (HasLinearModifier) { + Put(")"); + } + + if (HasStepModifier) { + Put(": "); + bool NeedComma{false}; + for (const Modifier &m : *modifiers) { + if (!std::holds_alternative(m.u)) { + if (NeedComma) { + Put(","); + } + common::visit([&](auto &&s) { Walk(s); }, m.u); + NeedComma = true; + } + } + } + } } void Unparse(const OmpReductionClause &x) { using Modifier = OmpReductionClause::Modifier; diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp index 0c50e14620fe..d63f7a5aea3a 100644 --- a/flang/lib/Semantics/check-omp-structure.cpp +++ b/flang/lib/Semantics/check-omp-structure.cpp @@ -414,14 +414,14 @@ void OmpStructureChecker::CheckMultListItems() { // Linear clause for (auto [_, clause] : FindClauses(llvm::omp::Clause::OMPC_linear)) { - const auto &linearClause{std::get(clause->u)}; + auto &linearClause{std::get(clause->u)}; std::list nameList; - common::visit( - [&](const auto &u) { - std::copy( - u.names.begin(), u.names.end(), std::back_inserter(nameList)); - }, - linearClause.v.u); + SymbolSourceMap symbols; + GetSymbolsInObjectList( + std::get(linearClause.v.t), symbols); + llvm::transform(symbols, std::back_inserter(nameList), [&](auto &&pair) { + return parser::Name{pair.second, const_cast(pair.first)}; + }); CheckMultipleOccurrence(listVars, nameList, clause->source, "LINEAR"); } } @@ -958,28 +958,13 @@ void OmpStructureChecker::CheckDistLinear( const auto &beginLoopDir{std::get(x.t)}; const auto &clauses{std::get(beginLoopDir.t)}; - semantics::UnorderedSymbolSet indexVars; + SymbolSourceMap indexVars; // Collect symbols of all the variables from linear clauses - for (const auto &clause : clauses.v) { - if (const auto *linearClause{ - std::get_if(&clause.u)}) { - - std::list values; - // Get the variant type - if (std::holds_alternative( - linearClause->v.u)) { - const auto &withM{ - std::get(linearClause->v.u)}; - values = withM.names; - } else { - const auto &withOutM{std::get( - linearClause->v.u)}; - values = withOutM.names; - } - for (auto const &v : values) { - indexVars.insert(*(v.symbol)); - } + for (auto &clause : clauses.v) { + if (auto *linearClause{std::get_if(&clause.u)}) { + auto &objects{std::get(linearClause->v.t)}; + GetSymbolsInObjectList(objects, indexVars); } } @@ -999,8 +984,8 @@ void OmpStructureChecker::CheckDistLinear( if (loop->IsDoNormal()) { const parser::Name &itrVal{GetLoopIndex(loop)}; if (itrVal.symbol) { - // Remove the symbol from the collcted set - indexVars.erase(*(itrVal.symbol)); + // Remove the symbol from the collected set + indexVars.erase(&itrVal.symbol->GetUltimate()); } collapseVal--; if (collapseVal == 0) { @@ -1016,12 +1001,10 @@ void OmpStructureChecker::CheckDistLinear( } // Show error for the remaining variables - for (auto var : indexVars) { - const Symbol &root{GetAssociationRoot(var)}; - context_.Say(parser::FindSourceLocation(x), - "Variable '%s' not allowed in `LINEAR` clause, only loop iterator " - "can be specified in `LINEAR` clause of a construct combined with " - "`DISTRIBUTE`"_err_en_US, + for (auto &[symbol, source] : indexVars) { + const Symbol &root{GetAssociationRoot(*symbol)}; + context_.Say(source, + "Variable '%s' not allowed in LINEAR clause, only loop iterator can be specified in LINEAR clause of a construct combined with DISTRIBUTE"_err_en_US, root.name()); } } @@ -3669,17 +3652,63 @@ void OmpStructureChecker::Enter(const parser::OmpClause::If &x) { void OmpStructureChecker::Enter(const parser::OmpClause::Linear &x) { CheckAllowedClause(llvm::omp::Clause::OMPC_linear); + unsigned version{context_.langOptions().OpenMPVersion}; + llvm::omp::Directive dir{GetContext().directive}; + parser::CharBlock clauseSource{GetContext().clauseSource}; + const parser::OmpLinearModifier *linearMod{nullptr}; - parser::CharBlock source{GetContext().clauseSource}; - // 2.7 Loop Construct Restriction - if ((llvm::omp::allDoSet | llvm::omp::allSimdSet) - .test(GetContext().directive)) { - if (std::holds_alternative(x.v.u)) { + SymbolSourceMap symbols; + auto &objects{std::get(x.v.t)}; + GetSymbolsInObjectList(objects, symbols); + + auto CheckIntegerNoRef{[&](const Symbol *symbol, parser::CharBlock source) { + if (!symbol->GetType()->IsNumeric(TypeCategory::Integer)) { + auto &desc{OmpGetDescriptor()}; context_.Say(source, - "A modifier may not be specified in a LINEAR clause " - "on the %s directive"_err_en_US, - ContextDirectiveAsFortran()); - return; + "The list item '%s' specified without the REF '%s' must be of INTEGER type"_err_en_US, + symbol->name(), desc.name.str()); + } + }}; + + if (OmpVerifyModifiers(x.v, llvm::omp::OMPC_linear, clauseSource, context_)) { + auto &modifiers{OmpGetModifiers(x.v)}; + linearMod = OmpGetUniqueModifier(modifiers); + if (linearMod) { + // 2.7 Loop Construct Restriction + if ((llvm::omp::allDoSet | llvm::omp::allSimdSet).test(dir)) { + context_.Say(clauseSource, + "A modifier may not be specified in a LINEAR clause on the %s directive"_err_en_US, + ContextDirectiveAsFortran()); + return; + } + + auto &desc{OmpGetDescriptor()}; + for (auto &[symbol, source] : symbols) { + if (linearMod->v != parser::OmpLinearModifier::Value::Ref) { + CheckIntegerNoRef(symbol, source); + } else { + if (!IsAllocatable(*symbol) && !IsAssumedShape(*symbol) && + !IsPolymorphic(*symbol)) { + context_.Say(source, + "The list item `%s` specified with the REF '%s' must be polymorphic variable, assumed-shape array, or a variable with the `ALLOCATABLE` attribute"_err_en_US, + symbol->name(), desc.name.str()); + } + } + if (linearMod->v == parser::OmpLinearModifier::Value::Ref || + linearMod->v == parser::OmpLinearModifier::Value::Uval) { + if (!IsDummy(*symbol) || IsValue(*symbol)) { + context_.Say(source, + "If the `%s` is REF or UVAL, the list item '%s' must be a dummy argument without the VALUE attribute"_err_en_US, + desc.name.str(), symbol->name()); + } + } + } // for (symbol, source) + + if (version >= 52 && !std::get(x.v.t)) { + context_.Say(OmpGetModifierSource(modifiers, linearMod), + "The 'modifier()' syntax is deprecated in %s, use ' : modifier' instead"_warn_en_US, + ThisVersion(version)); + } } } @@ -3692,73 +3721,28 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Linear &x) { } } - auto checkForValidLinearClause_01 = [&](const parser::Name &name, - bool is_ref) { - std::string listItemName{name.ToString()}; - if (!is_ref && !name.symbol->GetType()->IsNumeric(TypeCategory::Integer)) { - context_.Say(source, - "The list item `%s` specified with other than linear-modifier `REF` must be of type INTEGER"_err_en_US, - listItemName); + // OpenMP 5.2: Linear clause Restrictions + for (auto &[symbol, source] : symbols) { + if (!linearMod) { + // Already checked this with the modifier present. + CheckIntegerNoRef(symbol, source); } - if (GetContext().directive == llvm::omp::Directive::OMPD_declare_simd && - !IsDummy(*name.symbol)) { + if (dir == llvm::omp::Directive::OMPD_declare_simd && !IsDummy(*symbol)) { context_.Say(source, "The list item `%s` must be a dummy argument"_err_en_US, - listItemName); + symbol->name()); } - if (IsPointer(*name.symbol) || - name.symbol->test(Symbol::Flag::CrayPointer)) { + if (IsPointer(*symbol) || symbol->test(Symbol::Flag::CrayPointer)) { context_.Say(source, "The list item `%s` in a LINEAR clause must not be Cray Pointer or a variable with POINTER attribute"_err_en_US, - listItemName); + symbol->name()); } - if (FindCommonBlockContaining(*name.symbol)) { + if (FindCommonBlockContaining(*symbol)) { context_.Say(source, "'%s' is a common block name and must not appear in an LINEAR clause"_err_en_US, - listItemName); + symbol->name()); } - }; - - auto checkForValidLinearClause_02 = [&](const parser::Name &name, - const parser::OmpLinearModifier::Value - &modifierValue) { - std::string listItemName{name.ToString()}; - checkForValidLinearClause_01( - name, (modifierValue == parser::OmpLinearModifier::Value::Ref)); - if (modifierValue != parser::OmpLinearModifier::Value::Val && - IsDummy(*name.symbol) && IsValue(*name.symbol)) { - context_.Say(source, - "The list item `%s` specified with the linear-modifier `REF` or `UVAL` must be a dummy argument without `VALUE` attribute"_err_en_US, - listItemName); - } - if (modifierValue == parser::OmpLinearModifier::Value::Ref && - !(IsAllocatable(*name.symbol) || IsAssumedShape(*name.symbol) || - IsPolymorphic(*name.symbol))) { - context_.Say(source, - "The list item `%s` specified with the linear-modifier `REF` must be polymorphic variable, assumed-shape array, or a variable with the `ALLOCATABLE` attribute"_err_en_US, - listItemName); - } - }; - - // OpenMP 5.2: Linear clause Restrictions - common::visit( - common::visitors{ - [&](const parser::OmpLinearClause::WithoutModifier &withoutModifier) { - for (const auto &name : withoutModifier.names) { - if (name.symbol) { - checkForValidLinearClause_01(name, false); - } - } - }, - [&](const parser::OmpLinearClause::WithModifier &withModifier) { - for (const auto &name : withModifier.names) { - if (name.symbol) { - checkForValidLinearClause_02(name, withModifier.modifier.v); - } - } - }, - }, - x.v.u); + } } void OmpStructureChecker::CheckAllowedMapTypes( diff --git a/flang/lib/Semantics/openmp-modifiers.cpp b/flang/lib/Semantics/openmp-modifiers.cpp index f8f81e6c6ffa..9f2896229bb7 100644 --- a/flang/lib/Semantics/openmp-modifiers.cpp +++ b/flang/lib/Semantics/openmp-modifiers.cpp @@ -407,6 +407,39 @@ const OmpModifierDescriptor &OmpGetDescriptor() { return desc; } +template <> +const OmpModifierDescriptor & +OmpGetDescriptor() { + static const OmpModifierDescriptor desc{ + /*name=*/"step-complex-modifier", + /*props=*/ + { + {52, {OmpProperty::Unique}}, + }, + /*clauses=*/ + { + {52, {Clause::OMPC_linear}}, + }, + }; + return desc; +} + +template <> +const OmpModifierDescriptor &OmpGetDescriptor() { + static const OmpModifierDescriptor desc{ + /*name=*/"step-simple-modifier", + /*props=*/ + { + {45, {OmpProperty::Unique, OmpProperty::Exclusive}}, + }, + /*clauses=*/ + { + {45, {Clause::OMPC_linear}}, + }, + }; + return desc; +} + template <> const OmpModifierDescriptor &OmpGetDescriptor() { static const OmpModifierDescriptor desc{ diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp index 80a086acebba..39478b58a907 100644 --- a/flang/lib/Semantics/resolve-directives.cpp +++ b/flang/lib/Semantics/resolve-directives.cpp @@ -502,19 +502,8 @@ public: return false; } bool Pre(const parser::OmpLinearClause &x) { - common::visit(common::visitors{ - [&](const parser::OmpLinearClause::WithoutModifier - &linearWithoutModifier) { - ResolveOmpNameList(linearWithoutModifier.names, - Symbol::Flag::OmpLinear); - }, - [&](const parser::OmpLinearClause::WithModifier - &linearWithModifier) { - ResolveOmpNameList( - linearWithModifier.names, Symbol::Flag::OmpLinear); - }, - }, - x.u); + auto &objects{std::get(x.t)}; + ResolveOmpObjectList(objects, Symbol::Flag::OmpLinear); return false; } diff --git a/flang/test/Parser/OpenMP/linear-clause.f90 b/flang/test/Parser/OpenMP/linear-clause.f90 new file mode 100644 index 000000000000..5f031b069414 --- /dev/null +++ b/flang/test/Parser/OpenMP/linear-clause.f90 @@ -0,0 +1,117 @@ +!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 + !$omp do linear(x) + do x = 1, 10 + enddo + !$omp end do +end + +!UNPARSE: SUBROUTINE f00 (x) +!UNPARSE: INTEGER x +!UNPARSE: !$OMP DO LINEAR(x) +!UNPARSE: DO x=1_4,10_4 +!UNPARSE: END DO +!UNPARSE: !$OMP END DO +!UNPARSE: END SUBROUTINE + +!PARSE-TREE: OmpBeginLoopDirective +!PARSE-TREE: | OmpLoopDirective -> llvm::omp::Directive = do +!PARSE-TREE: | OmpClauseList -> OmpClause -> Linear -> OmpLinearClause +!PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x' +!PARSE-TREE: | | bool = 'true' +!PARSE-TREE: DoConstruct + +subroutine f01(x) + integer :: x + !$omp do linear(x : 2) + do x = 1, 10 + enddo + !$omp end do +end + +!UNPARSE: SUBROUTINE f01 (x) +!UNPARSE: INTEGER x +!UNPARSE: !$OMP DO LINEAR(x: 2_4) +!UNPARSE: DO x=1_4,10_4 +!UNPARSE: END DO +!UNPARSE: !$OMP END DO +!UNPARSE: END SUBROUTINE + +!PARSE-TREE: OmpBeginLoopDirective +!PARSE-TREE: | OmpLoopDirective -> llvm::omp::Directive = do +!PARSE-TREE: | OmpClauseList -> OmpClause -> Linear -> OmpLinearClause +!PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x' +!PARSE-TREE: | | Modifier -> OmpStepSimpleModifier -> Scalar -> Integer -> Expr = '2_4' +!PARSE-TREE: | | | LiteralConstant -> IntLiteralConstant = '2' +!PARSE-TREE: | | bool = 'true' +!PARSE-TREE: DoConstruct + +subroutine f02(x) + integer :: x + !$omp do linear(x : step(3)) + do x = 1, 10 + enddo + !$omp end do +end + +!UNPARSE: SUBROUTINE f02 (x) +!UNPARSE: INTEGER x +!UNPARSE: !$OMP DO LINEAR(x: STEP(3_4)) +!UNPARSE: DO x=1_4,10_4 +!UNPARSE: END DO +!UNPARSE: !$OMP END DO +!UNPARSE: END SUBROUTINE + +!PARSE-TREE: OmpBeginLoopDirective +!PARSE-TREE: | OmpLoopDirective -> llvm::omp::Directive = do +!PARSE-TREE: | OmpClauseList -> OmpClause -> Linear -> OmpLinearClause +!PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x' +!PARSE-TREE: | | Modifier -> OmpStepComplexModifier -> Scalar -> Integer -> Expr = '3_4' +!PARSE-TREE: | | | LiteralConstant -> IntLiteralConstant = '3' +!PARSE-TREE: | | bool = 'true' +!PARSE-TREE: DoConstruct + +subroutine f03(x) + integer :: x + !$omp declare simd linear(x : uval) +end + +!UNPARSE: SUBROUTINE f03 (x) +!UNPARSE: INTEGER x +!UNPARSE: !$OMP DECLARE SIMD LINEAR(x: UVAL) +!UNPARSE: END SUBROUTINE + +!PARSE-TREE: SpecificationPart +![...] +!PARSE-TREE: | DeclarationConstruct -> SpecificationConstruct -> OpenMPDeclarativeConstruct -> OpenMPDeclareSimdConstruct +!PARSE-TREE: | | Verbatim +!PARSE-TREE: | | OmpClauseList -> OmpClause -> Linear -> OmpLinearClause +!PARSE-TREE: | | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x' +!PARSE-TREE: | | | Modifier -> OmpLinearModifier -> Value = Uval +!PARSE-TREE: | | | bool = 'true' +!PARSE-TREE: ExecutionPart -> Block + +subroutine f04(x) + integer :: x + !$omp declare simd linear(x : uval, step(3)) +end + +!UNPARSE: SUBROUTINE f04 (x) +!UNPARSE: INTEGER x +!UNPARSE: !$OMP DECLARE SIMD LINEAR(x: UVAL, STEP(3_4)) +!UNPARSE: END SUBROUTINE + +!PARSE-TREE: SpecificationPart +![...] +!PARSE-TREE: | DeclarationConstruct -> SpecificationConstruct -> OpenMPDeclarativeConstruct -> OpenMPDeclareSimdConstruct +!PARSE-TREE: | | Verbatim +!PARSE-TREE: | | OmpClauseList -> OmpClause -> Linear -> OmpLinearClause +!PARSE-TREE: | | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x' +!PARSE-TREE: | | | Modifier -> OmpLinearModifier -> Value = Uval +!PARSE-TREE: | | | Modifier -> OmpStepComplexModifier -> Scalar -> Integer -> Expr = '3_4' +!PARSE-TREE: | | | | LiteralConstant -> IntLiteralConstant = '3' +!PARSE-TREE: | | | bool = 'true' +!PARSE-TREE: ExecutionPart -> Block diff --git a/flang/test/Semantics/OpenMP/clause-validity01.f90 b/flang/test/Semantics/OpenMP/clause-validity01.f90 index 66e11e4b540f..e8114154a809 100644 --- a/flang/test/Semantics/OpenMP/clause-validity01.f90 +++ b/flang/test/Semantics/OpenMP/clause-validity01.f90 @@ -221,11 +221,19 @@ use omp_lib !ERROR: Clause LINEAR is not allowed if clause ORDERED appears on the DO directive !ERROR: The parameter of the ORDERED clause must be a constant positive integer expression + !ERROR: 'b' appears in more than one data-sharing clause on the same OpenMP directive !$omp do ordered(1-1) private(b) linear(b) linear(a) do i = 1, N a = 3.14 enddo + !ERROR: Clause LINEAR is not allowed if clause ORDERED appears on the DO directive + !ERROR: The parameter of the ORDERED clause must be a constant positive integer expression + !$omp do ordered(1-1) linear(a) + do i = 1, N + a = 3.14 + enddo + !ERROR: The parameter of the ORDERED clause must be greater than or equal to the parameter of the COLLAPSE clause !$omp do collapse(num-14) ordered(1) do i = 1, N diff --git a/flang/test/Semantics/OpenMP/linear-clause01.f90 b/flang/test/Semantics/OpenMP/linear-clause01.f90 index 654aa07f5bd4..f95e834c9026 100644 --- a/flang/test/Semantics/OpenMP/linear-clause01.f90 +++ b/flang/test/Semantics/OpenMP/linear-clause01.f90 @@ -16,25 +16,31 @@ end subroutine linear_clause_01 ! Case 2 subroutine linear_clause_02(arg_01, arg_02) - !ERROR: The list item `arg_01` specified with other than linear-modifier `REF` must be of type INTEGER + !ERROR: The list item 'arg_01' specified without the REF 'linear-modifier' must be of INTEGER type !$omp declare simd linear(val(arg_01)) real, intent(in) :: arg_01(:) - !ERROR: The list item `arg_02` specified with the linear-modifier `REF` or `UVAL` must be a dummy argument without `VALUE` attribute + !ERROR: The list item 'arg_02' specified without the REF 'linear-modifier' must be of INTEGER type + !ERROR: If the `linear-modifier` is REF or UVAL, the list item 'arg_02' must be a dummy argument without the VALUE attribute !$omp declare simd linear(uval(arg_02)) + !ERROR: The type of 'arg_02' has already been implicitly declared integer, value, intent(in) :: arg_02 + !ERROR: The list item 'var' specified without the REF 'linear-modifier' must be of INTEGER type + !ERROR: If the `linear-modifier` is REF or UVAL, the list item 'var' must be a dummy argument without the VALUE attribute !ERROR: The list item `var` must be a dummy argument !ERROR: The list item `var` in a LINEAR clause must not be Cray Pointer or a variable with POINTER attribute !$omp declare simd linear(uval(var)) + !ERROR: The type of 'var' has already been implicitly declared integer, pointer :: var end subroutine linear_clause_02 ! Case 3 subroutine linear_clause_03(arg) integer, intent(in) :: arg - !ERROR: The list item `arg` specified with the linear-modifier `REF` must be polymorphic variable, assumed-shape array, or a variable with the `ALLOCATABLE` attribute + !ERROR: The list item `arg` specified with the REF 'linear-modifier' must be polymorphic variable, assumed-shape array, or a variable with the `ALLOCATABLE` attribute !ERROR: List item 'arg' present at multiple LINEAR clauses + !ERROR: 'arg' appears in more than one data-sharing clause on the same OpenMP directive !$omp declare simd linear(ref(arg)) linear(arg) integer :: i diff --git a/flang/test/Semantics/OpenMP/linear-clause02.f90 b/flang/test/Semantics/OpenMP/linear-clause02.f90 new file mode 100644 index 000000000000..695d61715820 --- /dev/null +++ b/flang/test/Semantics/OpenMP/linear-clause02.f90 @@ -0,0 +1,13 @@ +!RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=52 + +subroutine f00(x) + integer :: x + !WARNING: The 'modifier()' syntax is deprecated in OpenMP v5.2, use ' : modifier' instead + !$omp declare simd linear(uval(x)) +end + +subroutine f01(x) + integer :: x + !ERROR: An exclusive 'step-simple-modifier' modifier cannot be specified together with a modifier of a different type + !$omp declare simd linear(uval(x) : 2) +end diff --git a/flang/test/Semantics/OpenMP/linear-iter.f90 b/flang/test/Semantics/OpenMP/linear-iter.f90 index 8102c1a03cd3..1f40228be92a 100644 --- a/flang/test/Semantics/OpenMP/linear-iter.f90 +++ b/flang/test/Semantics/OpenMP/linear-iter.f90 @@ -20,7 +20,7 @@ SUBROUTINE LINEAR_BAD(N) !$omp target !$omp teams - !ERROR: Variable 'j' not allowed in `LINEAR` clause, only loop iterator can be specified in `LINEAR` clause of a construct combined with `DISTRIBUTE` + !ERROR: Variable 'j' not allowed in LINEAR clause, only loop iterator can be specified in LINEAR clause of a construct combined with DISTRIBUTE !$omp distribute parallel do simd linear(j) do i = 1, N a = 3.14 @@ -31,8 +31,8 @@ SUBROUTINE LINEAR_BAD(N) !$omp target !$omp teams - !ERROR: Variable 'j' not allowed in `LINEAR` clause, only loop iterator can be specified in `LINEAR` clause of a construct combined with `DISTRIBUTE` - !ERROR: Variable 'b' not allowed in `LINEAR` clause, only loop iterator can be specified in `LINEAR` clause of a construct combined with `DISTRIBUTE` + !ERROR: Variable 'j' not allowed in LINEAR clause, only loop iterator can be specified in LINEAR clause of a construct combined with DISTRIBUTE + !ERROR: Variable 'b' not allowed in LINEAR clause, only loop iterator can be specified in LINEAR clause of a construct combined with DISTRIBUTE !$omp distribute parallel do simd linear(j) linear(b) do i = 1, N a = 3.14 @@ -43,8 +43,8 @@ SUBROUTINE LINEAR_BAD(N) !$omp target !$omp teams - !ERROR: Variable 'j' not allowed in `LINEAR` clause, only loop iterator can be specified in `LINEAR` clause of a construct combined with `DISTRIBUTE` - !ERROR: Variable 'b' not allowed in `LINEAR` clause, only loop iterator can be specified in `LINEAR` clause of a construct combined with `DISTRIBUTE` + !ERROR: Variable 'j' not allowed in LINEAR clause, only loop iterator can be specified in LINEAR clause of a construct combined with DISTRIBUTE + !ERROR: Variable 'b' not allowed in LINEAR clause, only loop iterator can be specified in LINEAR clause of a construct combined with DISTRIBUTE !$omp distribute parallel do simd linear(j, b) do i = 1, N a = 3.14 @@ -54,7 +54,7 @@ SUBROUTINE LINEAR_BAD(N) !$omp end target !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region. - !ERROR: Variable 'j' not allowed in `LINEAR` clause, only loop iterator can be specified in `LINEAR` clause of a construct combined with `DISTRIBUTE` + !ERROR: Variable 'j' not allowed in LINEAR clause, only loop iterator can be specified in LINEAR clause of a construct combined with DISTRIBUTE !$omp distribute simd linear(i,j) do i = 1, N do j = 1, N @@ -64,7 +64,7 @@ SUBROUTINE LINEAR_BAD(N) !$omp end distribute simd !ERROR: `DISTRIBUTE` region has to be strictly nested inside `TEAMS` region. - !ERROR: Variable 'j' not allowed in `LINEAR` clause, only loop iterator can be specified in `LINEAR` clause of a construct combined with `DISTRIBUTE` + !ERROR: Variable 'j' not allowed in LINEAR clause, only loop iterator can be specified in LINEAR clause of a construct combined with DISTRIBUTE !$omp distribute simd linear(i,j) collapse(1) do i = 1, N do j = 1, N diff --git a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h index 67632fb79f8a..3e22b6ff71c8 100644 --- a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h +++ b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h @@ -758,15 +758,13 @@ template // struct LinearT { // std::get won't work here due to duplicate types in the tuple. using List = ObjectListT; - using StepSimpleModifier = E; + // StepSimpleModifier is same as StepComplexModifier. using StepComplexModifier = E; ENUM(LinearModifier, Ref, Val, Uval); using TupleTrait = std::true_type; // Step == nullopt means 1. - std::tuple - t; + std::tuple t; }; // V5.2: [5.8.5] `link` clause diff --git a/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp b/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp index 8f195c4f6032..d218de225c36 100644 --- a/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp +++ b/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp @@ -748,8 +748,7 @@ TEST_F(OpenMPDecompositionTest, Allocate3) { // Allocate + linear omp::List Clauses{ {OMPC_allocate, omp::clause::Allocate{{std::nullopt, std::nullopt, {x}}}}, - {OMPC_linear, - omp::clause::Linear{{std::nullopt, std::nullopt, std::nullopt, {x}}}}, + {OMPC_linear, omp::clause::Linear{{std::nullopt, std::nullopt, {x}}}}, }; omp::ConstructDecomposition Dec(AnyVersion, Helper, OMPD_parallel_for, @@ -761,7 +760,7 @@ TEST_F(OpenMPDecompositionTest, Allocate3) { // The "shared" clause is duplicated---this isn't harmful, but it // should be fixed eventually. ASSERT_EQ(Dir0, "parallel shared(x) shared(x)"); // (33) - ASSERT_EQ(Dir1, "for linear(, , , (x)) firstprivate(x) lastprivate(, (x)) " + ASSERT_EQ(Dir1, "for linear(, , (x)) firstprivate(x) lastprivate(, (x)) " "allocate(, , (x))"); // (33) } @@ -1059,8 +1058,7 @@ TEST_F(OpenMPDecompositionTest, Linear1) { omp::Object x{"x"}; omp::List Clauses{ - {OMPC_linear, - omp::clause::Linear{{std::nullopt, std::nullopt, std::nullopt, {x}}}}, + {OMPC_linear, omp::clause::Linear{{std::nullopt, std::nullopt, {x}}}}, }; omp::ConstructDecomposition Dec(AnyVersion, Helper, OMPD_for_simd, Clauses); @@ -1068,7 +1066,7 @@ TEST_F(OpenMPDecompositionTest, Linear1) { std::string Dir0 = stringify(Dec.output[0]); std::string Dir1 = stringify(Dec.output[1]); ASSERT_EQ(Dir0, "for firstprivate(x) lastprivate(, (x))"); // (15.1), (15.2) - ASSERT_EQ(Dir1, "simd linear(, , , (x)) lastprivate(, (x))"); // (15.1) + ASSERT_EQ(Dir1, "simd linear(, , (x)) lastprivate(, (x))"); // (15.1) } // NOWAIT @@ -1102,13 +1100,12 @@ TEST_F(OpenMPDecompositionTest, Nowait1) { TEST_F(OpenMPDecompositionTest, Misc1) { omp::Object x{"x"}; omp::List Clauses{ - {OMPC_linear, - omp::clause::Linear{{std::nullopt, std::nullopt, std::nullopt, {x}}}}, + {OMPC_linear, omp::clause::Linear{{std::nullopt, std::nullopt, {x}}}}, }; omp::ConstructDecomposition Dec(AnyVersion, Helper, OMPD_simd, Clauses); ASSERT_EQ(Dec.output.size(), 1u); std::string Dir0 = stringify(Dec.output[0]); - ASSERT_EQ(Dir0, "simd linear(, , , (x)) lastprivate(, (x))"); + ASSERT_EQ(Dir0, "simd linear(, , (x)) lastprivate(, (x))"); } } // namespace