[flang][OpenMP] Rework LINEAR clause #119278
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
llvmbot
added
flang
|
@llvm/pr-subscribers-flang-semantics @llvm/pr-subscribers-flang-fir-hlfir Author: Krzysztof Parzyszek (kparzysz) ChangesThe 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. 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. Patch is 44.08 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/119278.diff 18 Files Affected:
diff --git a/flang/examples/FeatureList/FeatureList.cpp b/flang/examples/FeatureList/FeatureList.cpp
index 41a6255207976d..3a689c335c81c0 100644
--- a/flang/examples/FeatureList/FeatureList.cpp
+++ b/flang/examples/FeatureList/FeatureList.cpp
@@ -495,8 +495,7 @@ struct NodeVisitor {
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 c6f35a07d81ea5..e34777a6c64bba 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -558,10 +558,11 @@ class ParseTreeDumper {
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 e1ea4d459f4a60..af1d34ae804f3c 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 <typename V>
- 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 <typename M>
- 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 <typename V>
- 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 <typename M>
- 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 8160b095f06dd9..bd8db6b719d0a7 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3698,6 +3698,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
@@ -3925,7 +3941,7 @@ struct OmpDeviceTypeClause {
struct OmpFromClause {
TUPLE_CLASS_BOILERPLATE(OmpFromClause);
MODIFIER_BOILERPLATE(OmpExpectation, OmpIterator, OmpMapper);
- std::tuple<MODIFIERS(), OmpObjectList, bool> t;
+ std::tuple<MODIFIERS(), OmpObjectList, /*CommaSeparated=*/bool> t;
};
// Ref: [4.5:87-91], [5.0:140-146], [5.1:166-171], [5.2:269]
@@ -3969,28 +3985,20 @@ struct OmpLastprivateClause {
std::tuple<MODIFIERS(), OmpObjectList> 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<Name> &&n,
- std::optional<ScalarIntConstantExpr> &&s)
- : modifier(std::move(m)), names(std::move(n)), step(std::move(s)) {}
- OmpLinearModifier modifier;
- std::list<Name> names;
- std::optional<ScalarIntConstantExpr> step;
- };
- struct WithoutModifier {
- BOILERPLATE(WithoutModifier);
- WithoutModifier(
- std::list<Name> &&n, std::optional<ScalarIntConstantExpr> &&s)
- : names(std::move(n)), step(std::move(s)) {}
- std::list<Name> names;
- std::optional<ScalarIntConstantExpr> step;
- };
- std::variant<WithModifier, WithoutModifier> u;
+ TUPLE_CLASS_BOILERPLATE(OmpLinearClause);
+ MODIFIER_BOILERPLATE(
+ OmpLinearModifier, OmpStepSimpleModifier, OmpStepComplexModifier);
+ std::tuple<OmpObjectList, MODIFIERS(), /*PostModified=*/bool> t;
};
// Ref: [4.5:216-219], [5.0:315-324], [5.1:347-355], [5.2:150-158]
@@ -4005,7 +4013,7 @@ struct OmpLinearClause {
struct OmpMapClause {
TUPLE_CLASS_BOILERPLATE(OmpMapClause);
MODIFIER_BOILERPLATE(OmpMapTypeModifier, OmpMapper, OmpIterator, OmpMapType);
- std::tuple<MODIFIERS(), OmpObjectList, bool> t;
+ std::tuple<MODIFIERS(), OmpObjectList, /*CommaSeparated=*/bool> t;
};
// Ref: [4.5:87-91], [5.0:140-146], [5.1:166-171], [5.2:270]
@@ -4083,7 +4091,7 @@ struct OmpScheduleClause {
struct OmpToClause {
TUPLE_CLASS_BOILERPLATE(OmpToClause);
MODIFIER_BOILERPLATE(OmpExpectation, OmpIterator, OmpMapper);
- std::tuple<MODIFIERS(), OmpObjectList, bool> t;
+ std::tuple<MODIFIERS(), OmpObjectList, /*CommaSeparated=*/bool> 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 4025ce112d9cab..5d5c5e97faf413 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 10c31963ec493a..8b1ddc7a7ccf28 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -895,8 +895,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
@@ -906,26 +904,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<parser::OmpStepComplexModifier>(mods);
+ auto *m1 =
+ semantics::OmpGetUniqueModifier<parser::OmpStepSimpleModifier>(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<parser::OmpLinearModifier>(mods);
+ auto &t1 = std::get<parser::OmpObjectList>(inp.v.t);
+
+ auto &&maybeStep = m0 ? maybeApplyToV(makeExprFn(semaCtx), m0)
+ : m1 ? maybeApplyToV(makeExprFn(semaCtx), m1)
+ : std::optional<Linear::StepComplexModifier>{};
+
+ 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 86d475c1a15422..b7baa9759b0f55 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -99,6 +99,26 @@ constexpr ModifierList<Clause, Separator> modifierList(Separator sep) {
return ModifierList<Clause, Separator>(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 <typename SpecificTy, typename ClauseTy>
+struct SpecificModifierParser {
+ using resultType = typename ClauseTy::Modifier;
+ std::optional<resultType> Parse(ParseState &state) const {
+ Parser<resultType> p;
+ if (auto result{attempt(Parser<resultType>{}).Parse(state)}) {
+ if (std::holds_alternative<SpecificTy>(result->u)) {
+ return result;
+ }
+ }
+ return std::nullopt;
+ }
+};
+
// OpenMP Clauses
// [5.0] 2.1.6 iterator-specifier -> type-declaration-stmt = subscript-triple |
@@ -232,6 +252,11 @@ TYPE_PARSER(construct<OmpReductionModifier>(
"TASK" >> pure(OmpReductionModifier::Value::Task) ||
"DEFAULT" >> pure(OmpReductionModifier::Value::Default)))
+TYPE_PARSER(construct<OmpStepComplexModifier>( //
+ "STEP" >> parenthesized(scalarIntExpr)))
+
+TYPE_PARSER(construct<OmpStepSimpleModifier>(scalarIntExpr))
+
TYPE_PARSER(construct<OmpTaskDependenceType>(
"DEPOBJ" >> pure(OmpTaskDependenceType::Value::Depobj) ||
"IN"_id >> pure(OmpTaskDependenceType::Value::In) ||
@@ -285,6 +310,11 @@ TYPE_PARSER(sourced(construct<OmpIfClause::Modifier>(OmpDirectiveNameParser{})))
TYPE_PARSER(sourced(construct<OmpLastprivateClause::Modifier>(
Parser<OmpLastprivateModifier>{})))
+TYPE_PARSER(sourced(
+ construct<OmpLinearClause::Modifier>(Parser<OmpLinearModifier>{}) ||
+ construct<OmpLinearClause::Modifier>(Parser<OmpStepComplexModifier>{}) ||
+ construct<OmpLinearClause::Modifier>(Parser<OmpStepSimpleModifier>{})))
+
TYPE_PARSER(sourced(construct<OmpMapClause::Modifier>(
sourced(construct<OmpMapClause::Modifier>(Parser<OmpMapTypeModifier>{}) ||
construct<OmpMapClause::Modifier>(Parser<OmpMapper>{}) ||
@@ -460,13 +490,33 @@ TYPE_PARSER(construct<OmpToClause>(
applyFunction<OmpToClause>(makeMobClause<false>,
modifierList<OmpToClause>(maybe(","_tok)), Parser<OmpObjectList>{})))
-TYPE_CONTEXT_PARSER("Omp LINEAR clause"_en_US,
- construct<OmpLinearClause>(
- construct<OmpLinearClause>(construct<OmpLinearClause::WithModifier>(
- Parser<OmpLinearModifier>{}, parenthesized(nonemptyList(name)),
- maybe(":" >> scalarIntConstantExpr))) ||
- construct<OmpLinearClause>(construct<OmpLinearClause::WithoutModifier>(
- nonemptyList(name), maybe(":" >> scalarIntConstantExpr)))))
+OmpLinearClause makeLinearFromOldSyntax(OmpLinearClause::Modifier &&lm,
+ OmpObjectList &&objs, std::optional<OmpLinearClause::Modifier> &&ssm) {
+ std::list<OmpLinearClause::Modifier> 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<OmpLinearClause>( //
+ applyFunction(makeLinearFromOldSyntax,
+ SpecificModifierParser<OmpLinearModifier, OmpLinearClause>{},
+ parenthesized(Parser<OmpObjectList>{}),
+ maybe(":"_tok >> SpecificModifierParser<OmpStepSimpleModifier,
+ OmpLinearClause>{}))) ||
+ // LINEAR(list [: modifiers])
+ construct<OmpLinearClause>( //
+ Parser<OmpObjectList>{},
+ maybe(":"_tok >> nonemptyList(Parser<OmpLinearClause::Modifier>{})),
+ /*PostModified=*/pure(true)))
// OpenMPv5.2 12.5.2 detach-clause -> DETACH (event-handle)
TYPE_PARSER(construct<OmpDetachClause>(Parser<OmpObject>{}))
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 4782cc1f2d7d7d..2085c62f74b8d7 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2133,13 +2133,63 @@ class UnparseVisitor {
Walk(std::get<std::optional<std::list<Modifier>>>(x.t), ": ");
Walk(std::get<ScalarLogicalExpr>(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<std::optional<std::list<Modifier>>>(x.t)};
+ if (std::get<bool>(x.t)) { // PostModified
+ Walk(std::get<OmpObjectList>(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<parser::OmpLinearModifier>(&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<OmpObjectList>(x.t));
+ if (HasLinearModifier) {
+ Put(")");
+ }
+
+ if (HasStepModifier) {
+ Put(": ");
+ bool NeedComma{false};
+ for (const Modifier &m : *modifiers) {
+ if (!std::holds_alternative<parser::OmpLinearModifier>(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 3c9c5a02a338a6..e070466f64d452 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<parser::OmpClause::Linear>(clause->u)};
+ auto &linearClause{std::get<parser::OmpClause::Linear>(clause->u)};
std::list<parser::Name> 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<parser::OmpObjectList>(linearClause.v.t), symbols);
+ llvm::transform(symbols, std::back_inserter(nameList), [&](auto &&pair) {
+ return parser::Name{pair.second, const_cast<Symbol *>(pair.first)};
+ });
CheckMultipleOccurrence(listVars, nameList, clause->source, "LINEAR");
}
}
@@ -958,28 +958,13 @@ void OmpStructureChecker::CheckDistLinear(
const auto &beginLoopDir{std::get<parser::OmpBeginLoopDirective>(x.t)};
const auto &clauses{std::get<parser::OmpClauseList>(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<parser::OmpClause::Linear>(&clause.u)}) {
-
- std::list<parser::Name> values;
- // Get the variant type
- if (std::holds_alternative<parser::OmpLinearClause::WithModifier>(
- linearClause->v.u)) {
- const auto &withM{
- std::get<parser::OmpLinearClause::WithModifier>(linearClause->v.u)};
- values = withM.names;
- } else {
- const auto &withOutM{std::get<parser::OmpLinearClause::WithoutModifier>(
- 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<parser::OmpClause::Linear>(&clause.u)}) {
+ auto &objects{std::get<parser::OmpObjectList>(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());
}
}
@@ -3619,17 +3602,63 @@ void OmpStructureChecker::Enter(con...
[truncated]
|
|
@llvm/pr-subscribers-flang-parser Author: Krzysztof Parzyszek (kparzysz) ChangesThe 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. 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. Patch is 44.08 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/119278.diff 18 Files Affected:
diff --git a/flang/examples/FeatureList/FeatureList.cpp b/flang/examples/FeatureList/FeatureList.cpp
index 41a6255207976d..3a689c335c81c0 100644
--- a/flang/examples/FeatureList/FeatureList.cpp
+++ b/flang/examples/FeatureList/FeatureList.cpp
@@ -495,8 +495,7 @@ struct NodeVisitor {
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 c6f35a07d81ea5..e34777a6c64bba 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -558,10 +558,11 @@ class ParseTreeDumper {
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 e1ea4d459f4a60..af1d34ae804f3c 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 <typename V>
- 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 <typename M>
- 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 <typename V>
- 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 <typename M>
- 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 8160b095f06dd9..bd8db6b719d0a7 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3698,6 +3698,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
@@ -3925,7 +3941,7 @@ struct OmpDeviceTypeClause {
struct OmpFromClause {
TUPLE_CLASS_BOILERPLATE(OmpFromClause);
MODIFIER_BOILERPLATE(OmpExpectation, OmpIterator, OmpMapper);
- std::tuple<MODIFIERS(), OmpObjectList, bool> t;
+ std::tuple<MODIFIERS(), OmpObjectList, /*CommaSeparated=*/bool> t;
};
// Ref: [4.5:87-91], [5.0:140-146], [5.1:166-171], [5.2:269]
@@ -3969,28 +3985,20 @@ struct OmpLastprivateClause {
std::tuple<MODIFIERS(), OmpObjectList> 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<Name> &&n,
- std::optional<ScalarIntConstantExpr> &&s)
- : modifier(std::move(m)), names(std::move(n)), step(std::move(s)) {}
- OmpLinearModifier modifier;
- std::list<Name> names;
- std::optional<ScalarIntConstantExpr> step;
- };
- struct WithoutModifier {
- BOILERPLATE(WithoutModifier);
- WithoutModifier(
- std::list<Name> &&n, std::optional<ScalarIntConstantExpr> &&s)
- : names(std::move(n)), step(std::move(s)) {}
- std::list<Name> names;
- std::optional<ScalarIntConstantExpr> step;
- };
- std::variant<WithModifier, WithoutModifier> u;
+ TUPLE_CLASS_BOILERPLATE(OmpLinearClause);
+ MODIFIER_BOILERPLATE(
+ OmpLinearModifier, OmpStepSimpleModifier, OmpStepComplexModifier);
+ std::tuple<OmpObjectList, MODIFIERS(), /*PostModified=*/bool> t;
};
// Ref: [4.5:216-219], [5.0:315-324], [5.1:347-355], [5.2:150-158]
@@ -4005,7 +4013,7 @@ struct OmpLinearClause {
struct OmpMapClause {
TUPLE_CLASS_BOILERPLATE(OmpMapClause);
MODIFIER_BOILERPLATE(OmpMapTypeModifier, OmpMapper, OmpIterator, OmpMapType);
- std::tuple<MODIFIERS(), OmpObjectList, bool> t;
+ std::tuple<MODIFIERS(), OmpObjectList, /*CommaSeparated=*/bool> t;
};
// Ref: [4.5:87-91], [5.0:140-146], [5.1:166-171], [5.2:270]
@@ -4083,7 +4091,7 @@ struct OmpScheduleClause {
struct OmpToClause {
TUPLE_CLASS_BOILERPLATE(OmpToClause);
MODIFIER_BOILERPLATE(OmpExpectation, OmpIterator, OmpMapper);
- std::tuple<MODIFIERS(), OmpObjectList, bool> t;
+ std::tuple<MODIFIERS(), OmpObjectList, /*CommaSeparated=*/bool> 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 4025ce112d9cab..5d5c5e97faf413 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 10c31963ec493a..8b1ddc7a7ccf28 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -895,8 +895,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
@@ -906,26 +904,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<parser::OmpStepComplexModifier>(mods);
+ auto *m1 =
+ semantics::OmpGetUniqueModifier<parser::OmpStepSimpleModifier>(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<parser::OmpLinearModifier>(mods);
+ auto &t1 = std::get<parser::OmpObjectList>(inp.v.t);
+
+ auto &&maybeStep = m0 ? maybeApplyToV(makeExprFn(semaCtx), m0)
+ : m1 ? maybeApplyToV(makeExprFn(semaCtx), m1)
+ : std::optional<Linear::StepComplexModifier>{};
+
+ 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 86d475c1a15422..b7baa9759b0f55 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -99,6 +99,26 @@ constexpr ModifierList<Clause, Separator> modifierList(Separator sep) {
return ModifierList<Clause, Separator>(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 <typename SpecificTy, typename ClauseTy>
+struct SpecificModifierParser {
+ using resultType = typename ClauseTy::Modifier;
+ std::optional<resultType> Parse(ParseState &state) const {
+ Parser<resultType> p;
+ if (auto result{attempt(Parser<resultType>{}).Parse(state)}) {
+ if (std::holds_alternative<SpecificTy>(result->u)) {
+ return result;
+ }
+ }
+ return std::nullopt;
+ }
+};
+
// OpenMP Clauses
// [5.0] 2.1.6 iterator-specifier -> type-declaration-stmt = subscript-triple |
@@ -232,6 +252,11 @@ TYPE_PARSER(construct<OmpReductionModifier>(
"TASK" >> pure(OmpReductionModifier::Value::Task) ||
"DEFAULT" >> pure(OmpReductionModifier::Value::Default)))
+TYPE_PARSER(construct<OmpStepComplexModifier>( //
+ "STEP" >> parenthesized(scalarIntExpr)))
+
+TYPE_PARSER(construct<OmpStepSimpleModifier>(scalarIntExpr))
+
TYPE_PARSER(construct<OmpTaskDependenceType>(
"DEPOBJ" >> pure(OmpTaskDependenceType::Value::Depobj) ||
"IN"_id >> pure(OmpTaskDependenceType::Value::In) ||
@@ -285,6 +310,11 @@ TYPE_PARSER(sourced(construct<OmpIfClause::Modifier>(OmpDirectiveNameParser{})))
TYPE_PARSER(sourced(construct<OmpLastprivateClause::Modifier>(
Parser<OmpLastprivateModifier>{})))
+TYPE_PARSER(sourced(
+ construct<OmpLinearClause::Modifier>(Parser<OmpLinearModifier>{}) ||
+ construct<OmpLinearClause::Modifier>(Parser<OmpStepComplexModifier>{}) ||
+ construct<OmpLinearClause::Modifier>(Parser<OmpStepSimpleModifier>{})))
+
TYPE_PARSER(sourced(construct<OmpMapClause::Modifier>(
sourced(construct<OmpMapClause::Modifier>(Parser<OmpMapTypeModifier>{}) ||
construct<OmpMapClause::Modifier>(Parser<OmpMapper>{}) ||
@@ -460,13 +490,33 @@ TYPE_PARSER(construct<OmpToClause>(
applyFunction<OmpToClause>(makeMobClause<false>,
modifierList<OmpToClause>(maybe(","_tok)), Parser<OmpObjectList>{})))
-TYPE_CONTEXT_PARSER("Omp LINEAR clause"_en_US,
- construct<OmpLinearClause>(
- construct<OmpLinearClause>(construct<OmpLinearClause::WithModifier>(
- Parser<OmpLinearModifier>{}, parenthesized(nonemptyList(name)),
- maybe(":" >> scalarIntConstantExpr))) ||
- construct<OmpLinearClause>(construct<OmpLinearClause::WithoutModifier>(
- nonemptyList(name), maybe(":" >> scalarIntConstantExpr)))))
+OmpLinearClause makeLinearFromOldSyntax(OmpLinearClause::Modifier &&lm,
+ OmpObjectList &&objs, std::optional<OmpLinearClause::Modifier> &&ssm) {
+ std::list<OmpLinearClause::Modifier> 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<OmpLinearClause>( //
+ applyFunction(makeLinearFromOldSyntax,
+ SpecificModifierParser<OmpLinearModifier, OmpLinearClause>{},
+ parenthesized(Parser<OmpObjectList>{}),
+ maybe(":"_tok >> SpecificModifierParser<OmpStepSimpleModifier,
+ OmpLinearClause>{}))) ||
+ // LINEAR(list [: modifiers])
+ construct<OmpLinearClause>( //
+ Parser<OmpObjectList>{},
+ maybe(":"_tok >> nonemptyList(Parser<OmpLinearClause::Modifier>{})),
+ /*PostModified=*/pure(true)))
// OpenMPv5.2 12.5.2 detach-clause -> DETACH (event-handle)
TYPE_PARSER(construct<OmpDetachClause>(Parser<OmpObject>{}))
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 4782cc1f2d7d7d..2085c62f74b8d7 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2133,13 +2133,63 @@ class UnparseVisitor {
Walk(std::get<std::optional<std::list<Modifier>>>(x.t), ": ");
Walk(std::get<ScalarLogicalExpr>(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<std::optional<std::list<Modifier>>>(x.t)};
+ if (std::get<bool>(x.t)) { // PostModified
+ Walk(std::get<OmpObjectList>(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<parser::OmpLinearModifier>(&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<OmpObjectList>(x.t));
+ if (HasLinearModifier) {
+ Put(")");
+ }
+
+ if (HasStepModifier) {
+ Put(": ");
+ bool NeedComma{false};
+ for (const Modifier &m : *modifiers) {
+ if (!std::holds_alternative<parser::OmpLinearModifier>(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 3c9c5a02a338a6..e070466f64d452 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<parser::OmpClause::Linear>(clause->u)};
+ auto &linearClause{std::get<parser::OmpClause::Linear>(clause->u)};
std::list<parser::Name> 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<parser::OmpObjectList>(linearClause.v.t), symbols);
+ llvm::transform(symbols, std::back_inserter(nameList), [&](auto &&pair) {
+ return parser::Name{pair.second, const_cast<Symbol *>(pair.first)};
+ });
CheckMultipleOccurrence(listVars, nameList, clause->source, "LINEAR");
}
}
@@ -958,28 +958,13 @@ void OmpStructureChecker::CheckDistLinear(
const auto &beginLoopDir{std::get<parser::OmpBeginLoopDirective>(x.t)};
const auto &clauses{std::get<parser::OmpClauseList>(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<parser::OmpClause::Linear>(&clause.u)}) {
-
- std::list<parser::Name> values;
- // Get the variant type
- if (std::holds_alternative<parser::OmpLinearClause::WithModifier>(
- linearClause->v.u)) {
- const auto &withM{
- std::get<parser::OmpLinearClause::WithModifier>(linearClause->v.u)};
- values = withM.names;
- } else {
- const auto &withOutM{std::get<parser::OmpLinearClause::WithoutModifier>(
- 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<parser::OmpClause::Linear>(&clause.u)}) {
+ auto &objects{std::get<parser::OmpObjectList>(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());
}
}
@@ -3619,17 +3602,63 @@ void OmpStructureChecker::Enter(con...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
tblah
approved these changes
Dec 12, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.