Skip to content

Commit 03cbe42

Browse files
kparzyszkparzysz
authored
[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.
1 parent 58f9c4f commit 03cbe42

File tree

18 files changed

+448
-233
lines changed

18 files changed

+448
-233
lines changed

‎flang/examples/FeatureList/FeatureList.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -495,8 +495,7 @@ struct NodeVisitor {
495495
READ_FEATURE(OmpIfClause::Modifier)
496496
READ_FEATURE(OmpDirectiveNameModifier)
497497
READ_FEATURE(OmpLinearClause)
498-
READ_FEATURE(OmpLinearClause::WithModifier)
499-
READ_FEATURE(OmpLinearClause::WithoutModifier)
498+
READ_FEATURE(OmpLinearClause::Modifier)
500499
READ_FEATURE(OmpLinearModifier)
501500
READ_FEATURE(OmpLinearModifier::Value)
502501
READ_FEATURE(OmpLoopDirective)

‎flang/include/flang/Parser/dump-parse-tree.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -559,10 +559,11 @@ class ParseTreeDumper {
559559
NODE(parser, OmpLastprivateModifier)
560560
NODE_ENUM(OmpLastprivateModifier, Value)
561561
NODE(parser, OmpLinearClause)
562-
NODE(OmpLinearClause, WithModifier)
563-
NODE(OmpLinearClause, WithoutModifier)
562+
NODE(OmpLinearClause, Modifier)
564563
NODE(parser, OmpLinearModifier)
565564
NODE_ENUM(OmpLinearModifier, Value)
565+
NODE(parser, OmpStepComplexModifier)
566+
NODE(parser, OmpStepSimpleModifier)
566567
NODE(parser, OmpLoopDirective)
567568
NODE(parser, OmpMapClause)
568569
NODE(OmpMapClause, Modifier)

‎flang/include/flang/Parser/parse-tree-visitor.h

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -897,40 +897,6 @@ struct ParseTreeVisitorLookupScope {
897897
mutator.Post(x);
898898
}
899899
}
900-
template <typename V>
901-
static void Walk(const OmpLinearClause::WithModifier &x, V &visitor) {
902-
if (visitor.Pre(x)) {
903-
Walk(x.modifier, visitor);
904-
Walk(x.names, visitor);
905-
Walk(x.step, visitor);
906-
visitor.Post(x);
907-
}
908-
}
909-
template <typename M>
910-
static void Walk(OmpLinearClause::WithModifier &x, M &mutator) {
911-
if (mutator.Pre(x)) {
912-
Walk(x.modifier, mutator);
913-
Walk(x.names, mutator);
914-
Walk(x.step, mutator);
915-
mutator.Post(x);
916-
}
917-
}
918-
template <typename V>
919-
static void Walk(const OmpLinearClause::WithoutModifier &x, V &visitor) {
920-
if (visitor.Pre(x)) {
921-
Walk(x.names, visitor);
922-
Walk(x.step, visitor);
923-
visitor.Post(x);
924-
}
925-
}
926-
template <typename M>
927-
static void Walk(OmpLinearClause::WithoutModifier &x, M &mutator) {
928-
if (mutator.Pre(x)) {
929-
Walk(x.names, mutator);
930-
Walk(x.step, mutator);
931-
mutator.Post(x);
932-
}
933-
}
934900
};
935901
} // namespace detail
936902

‎flang/include/flang/Parser/parse-tree.h

Lines changed: 32 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3699,6 +3699,22 @@ struct OmpReductionModifier {
36993699
WRAPPER_CLASS_BOILERPLATE(OmpReductionModifier, Value);
37003700
};
37013701

3702+
// Ref: [5.2:117-120]
3703+
//
3704+
// step-complex-modifier ->
3705+
// STEP(integer-expression) // since 5.2
3706+
struct OmpStepComplexModifier {
3707+
WRAPPER_CLASS_BOILERPLATE(OmpStepComplexModifier, ScalarIntExpr);
3708+
};
3709+
3710+
// Ref: [4.5:207-210], [5.0:290-293], [5.1:323-325], [5.2:117-120]
3711+
//
3712+
// step-simple-modifier ->
3713+
// integer-expresion // since 4.5
3714+
struct OmpStepSimpleModifier {
3715+
WRAPPER_CLASS_BOILERPLATE(OmpStepSimpleModifier, ScalarIntExpr);
3716+
};
3717+
37023718
// Ref: [4.5:169-170], [5.0:254-256], [5.1:287-289], [5.2:321]
37033719
//
37043720
// task-dependence-type -> // "dependence-type" in 5.1 and before
@@ -3934,7 +3950,7 @@ struct OmpFailClause {
39343950
struct OmpFromClause {
39353951
TUPLE_CLASS_BOILERPLATE(OmpFromClause);
39363952
MODIFIER_BOILERPLATE(OmpExpectation, OmpIterator, OmpMapper);
3937-
std::tuple<MODIFIERS(), OmpObjectList, bool> t;
3953+
std::tuple<MODIFIERS(), OmpObjectList, /*CommaSeparated=*/bool> t;
39383954
};
39393955

39403956
// Ref: [4.5:87-91], [5.0:140-146], [5.1:166-171], [5.2:269]
@@ -3981,28 +3997,20 @@ struct OmpLastprivateClause {
39813997
std::tuple<MODIFIERS(), OmpObjectList> t;
39823998
};
39833999

3984-
// 2.15.3.7 linear-clause -> LINEAR (linear-list[ : linear-step])
3985-
// linear-list -> list | linear-modifier(list)
4000+
// Ref: [4.5:207-210], [5.0:290-293], [5.1:323-325], [5.2:117-120]
4001+
//
4002+
// linear-clause ->
4003+
// LINEAR(list [: step-simple-modifier]) | // since 4.5
4004+
// LINEAR(linear-modifier(list)
4005+
// [: step-simple-modifier]) | // since 4.5, until 5.2[*]
4006+
// LINEAR(list [: linear-modifier,
4007+
// step-complex-modifier]) // since 5.2
4008+
// [*] Still allowed in 5.2 when on DECLARE SIMD, but deprecated.
39864009
struct OmpLinearClause {
3987-
UNION_CLASS_BOILERPLATE(OmpLinearClause);
3988-
struct WithModifier {
3989-
BOILERPLATE(WithModifier);
3990-
WithModifier(OmpLinearModifier &&m, std::list<Name> &&n,
3991-
std::optional<ScalarIntConstantExpr> &&s)
3992-
: modifier(std::move(m)), names(std::move(n)), step(std::move(s)) {}
3993-
OmpLinearModifier modifier;
3994-
std::list<Name> names;
3995-
std::optional<ScalarIntConstantExpr> step;
3996-
};
3997-
struct WithoutModifier {
3998-
BOILERPLATE(WithoutModifier);
3999-
WithoutModifier(
4000-
std::list<Name> &&n, std::optional<ScalarIntConstantExpr> &&s)
4001-
: names(std::move(n)), step(std::move(s)) {}
4002-
std::list<Name> names;
4003-
std::optional<ScalarIntConstantExpr> step;
4004-
};
4005-
std::variant<WithModifier, WithoutModifier> u;
4010+
TUPLE_CLASS_BOILERPLATE(OmpLinearClause);
4011+
MODIFIER_BOILERPLATE(
4012+
OmpLinearModifier, OmpStepSimpleModifier, OmpStepComplexModifier);
4013+
std::tuple<OmpObjectList, MODIFIERS(), /*PostModified=*/bool> t;
40064014
};
40074015

40084016
// Ref: [4.5:216-219], [5.0:315-324], [5.1:347-355], [5.2:150-158]
@@ -4017,7 +4025,7 @@ struct OmpLinearClause {
40174025
struct OmpMapClause {
40184026
TUPLE_CLASS_BOILERPLATE(OmpMapClause);
40194027
MODIFIER_BOILERPLATE(OmpMapTypeModifier, OmpMapper, OmpIterator, OmpMapType);
4020-
std::tuple<MODIFIERS(), OmpObjectList, bool> t;
4028+
std::tuple<MODIFIERS(), OmpObjectList, /*CommaSeparated=*/bool> t;
40214029
};
40224030

40234031
// Ref: [4.5:87-91], [5.0:140-146], [5.1:166-171], [5.2:270]
@@ -4105,7 +4113,7 @@ struct OmpTaskReductionClause {
41054113
struct OmpToClause {
41064114
TUPLE_CLASS_BOILERPLATE(OmpToClause);
41074115
MODIFIER_BOILERPLATE(OmpExpectation, OmpIterator, OmpMapper);
4108-
std::tuple<MODIFIERS(), OmpObjectList, bool> t;
4116+
std::tuple<MODIFIERS(), OmpObjectList, /*CommaSeparated=*/bool> t;
41094117
};
41104118

41114119
// Ref: [5.0:254-255], [5.1:287-288], [5.2:321-322]

‎flang/include/flang/Semantics/openmp-modifiers.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,8 @@ DECLARE_DESCRIPTOR(parser::OmpOrderingModifier);
8787
DECLARE_DESCRIPTOR(parser::OmpPrescriptiveness);
8888
DECLARE_DESCRIPTOR(parser::OmpReductionIdentifier);
8989
DECLARE_DESCRIPTOR(parser::OmpReductionModifier);
90+
DECLARE_DESCRIPTOR(parser::OmpStepComplexModifier);
91+
DECLARE_DESCRIPTOR(parser::OmpStepSimpleModifier);
9092
DECLARE_DESCRIPTOR(parser::OmpTaskDependenceType);
9193
DECLARE_DESCRIPTOR(parser::OmpVariableCategory);
9294

‎flang/lib/Lower/OpenMP/Clauses.cpp

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -899,8 +899,6 @@ Lastprivate make(const parser::OmpClause::Lastprivate &inp,
899899
Linear make(const parser::OmpClause::Linear &inp,
900900
semantics::SemanticsContext &semaCtx) {
901901
// inp.v -> parser::OmpLinearClause
902-
using wrapped = parser::OmpLinearClause;
903-
904902
CLAUSET_ENUM_CONVERT( //
905903
convert, parser::OmpLinearModifier::Value, Linear::LinearModifier,
906904
// clang-format off
@@ -910,26 +908,23 @@ Linear make(const parser::OmpClause::Linear &inp,
910908
// clang-format on
911909
);
912910

913-
using Tuple = decltype(Linear::t);
911+
auto &mods = semantics::OmpGetModifiers(inp.v);
912+
auto *m0 =
913+
semantics::OmpGetUniqueModifier<parser::OmpStepComplexModifier>(mods);
914+
auto *m1 =
915+
semantics::OmpGetUniqueModifier<parser::OmpStepSimpleModifier>(mods);
916+
assert((!m0 || !m1) && "Simple and complex modifiers both present");
914917

915-
return Linear{Fortran::common::visit(
916-
common::visitors{
917-
[&](const wrapped::WithModifier &s) -> Tuple {
918-
return {
919-
/*StepSimpleModifier=*/std::nullopt,
920-
/*StepComplexModifier=*/maybeApply(makeExprFn(semaCtx), s.step),
921-
/*LinearModifier=*/convert(s.modifier.v),
922-
/*List=*/makeList(s.names, makeObjectFn(semaCtx))};
923-
},
924-
[&](const wrapped::WithoutModifier &s) -> Tuple {
925-
return {
926-
/*StepSimpleModifier=*/maybeApply(makeExprFn(semaCtx), s.step),
927-
/*StepComplexModifier=*/std::nullopt,
928-
/*LinearModifier=*/std::nullopt,
929-
/*List=*/makeList(s.names, makeObjectFn(semaCtx))};
930-
},
931-
},
932-
inp.v.u)};
918+
auto *m2 = semantics::OmpGetUniqueModifier<parser::OmpLinearModifier>(mods);
919+
auto &t1 = std::get<parser::OmpObjectList>(inp.v.t);
920+
921+
auto &&maybeStep = m0 ? maybeApplyToV(makeExprFn(semaCtx), m0)
922+
: m1 ? maybeApplyToV(makeExprFn(semaCtx), m1)
923+
: std::optional<Linear::StepComplexModifier>{};
924+
925+
return Linear{{/*StepComplexModifier=*/std::move(maybeStep),
926+
/*LinearModifier=*/maybeApplyToV(convert, m2),
927+
/*List=*/makeObjects(t1, semaCtx)}};
933928
}
934929

935930
Link make(const parser::OmpClause::Link &inp,

‎flang/lib/Parser/openmp-parsers.cpp

Lines changed: 56 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,25 @@ constexpr ModifierList<Clause, Separator> modifierList(Separator sep) {
9999
return ModifierList<Clause, Separator>(sep);
100100
}
101101

102+
// Parse the input as any modifier from ClauseTy, but only succeed if
103+
// the result was the SpecificTy. It requires that SpecificTy is one
104+
// of the alternatives in ClauseTy::Modifier.
105+
// The reason to have this is that ClauseTy::Modifier has "source",
106+
// while specific modifiers don't. This class allows to parse a specific
107+
// modifier together with obtaining its location.
108+
template <typename SpecificTy, typename ClauseTy>
109+
struct SpecificModifierParser {
110+
using resultType = typename ClauseTy::Modifier;
111+
std::optional<resultType> Parse(ParseState &state) const {
112+
if (auto result{attempt(Parser<resultType>{}).Parse(state)}) {
113+
if (std::holds_alternative<SpecificTy>(result->u)) {
114+
return result;
115+
}
116+
}
117+
return std::nullopt;
118+
}
119+
};
120+
102121
// OpenMP Clauses
103122

104123
// [5.0] 2.1.6 iterator-specifier -> type-declaration-stmt = subscript-triple |
@@ -232,6 +251,11 @@ TYPE_PARSER(construct<OmpReductionModifier>(
232251
"TASK" >> pure(OmpReductionModifier::Value::Task) ||
233252
"DEFAULT" >> pure(OmpReductionModifier::Value::Default)))
234253

254+
TYPE_PARSER(construct<OmpStepComplexModifier>( //
255+
"STEP" >> parenthesized(scalarIntExpr)))
256+
257+
TYPE_PARSER(construct<OmpStepSimpleModifier>(scalarIntExpr))
258+
235259
TYPE_PARSER(construct<OmpTaskDependenceType>(
236260
"DEPOBJ" >> pure(OmpTaskDependenceType::Value::Depobj) ||
237261
"IN"_id >> pure(OmpTaskDependenceType::Value::In) ||
@@ -288,6 +312,11 @@ TYPE_PARSER(sourced(construct<OmpInReductionClause::Modifier>(
288312
TYPE_PARSER(sourced(construct<OmpLastprivateClause::Modifier>(
289313
Parser<OmpLastprivateModifier>{})))
290314

315+
TYPE_PARSER(sourced(
316+
construct<OmpLinearClause::Modifier>(Parser<OmpLinearModifier>{}) ||
317+
construct<OmpLinearClause::Modifier>(Parser<OmpStepComplexModifier>{}) ||
318+
construct<OmpLinearClause::Modifier>(Parser<OmpStepSimpleModifier>{})))
319+
291320
TYPE_PARSER(sourced(construct<OmpMapClause::Modifier>(
292321
sourced(construct<OmpMapClause::Modifier>(Parser<OmpMapTypeModifier>{}) ||
293322
construct<OmpMapClause::Modifier>(Parser<OmpMapper>{}) ||
@@ -471,13 +500,33 @@ TYPE_PARSER(construct<OmpToClause>(
471500
applyFunction<OmpToClause>(makeMobClause<false>,
472501
modifierList<OmpToClause>(maybe(","_tok)), Parser<OmpObjectList>{})))
473502

474-
TYPE_CONTEXT_PARSER("Omp LINEAR clause"_en_US,
475-
construct<OmpLinearClause>(
476-
construct<OmpLinearClause>(construct<OmpLinearClause::WithModifier>(
477-
Parser<OmpLinearModifier>{}, parenthesized(nonemptyList(name)),
478-
maybe(":" >> scalarIntConstantExpr))) ||
479-
construct<OmpLinearClause>(construct<OmpLinearClause::WithoutModifier>(
480-
nonemptyList(name), maybe(":" >> scalarIntConstantExpr)))))
503+
OmpLinearClause makeLinearFromOldSyntax(OmpLinearClause::Modifier &&lm,
504+
OmpObjectList &&objs, std::optional<OmpLinearClause::Modifier> &&ssm) {
505+
std::list<OmpLinearClause::Modifier> mods;
506+
mods.emplace_back(std::move(lm));
507+
if (ssm) {
508+
mods.emplace_back(std::move(*ssm));
509+
}
510+
return OmpLinearClause{std::move(objs),
511+
mods.empty() ? decltype(mods){} : std::move(mods),
512+
/*PostModified=*/false};
513+
}
514+
515+
TYPE_PARSER(
516+
// Parse the "modifier(x)" first, because syntacticaly it will match
517+
// an array element (i.e. a list item).
518+
// LINEAR(linear-modifier(list) [: step-simple-modifier])
519+
construct<OmpLinearClause>( //
520+
applyFunction<OmpLinearClause>(makeLinearFromOldSyntax,
521+
SpecificModifierParser<OmpLinearModifier, OmpLinearClause>{},
522+
parenthesized(Parser<OmpObjectList>{}),
523+
maybe(":"_tok >> SpecificModifierParser<OmpStepSimpleModifier,
524+
OmpLinearClause>{}))) ||
525+
// LINEAR(list [: modifiers])
526+
construct<OmpLinearClause>( //
527+
Parser<OmpObjectList>{},
528+
maybe(":"_tok >> nonemptyList(Parser<OmpLinearClause::Modifier>{})),
529+
/*PostModified=*/pure(true)))
481530

482531
// OpenMPv5.2 12.5.2 detach-clause -> DETACH (event-handle)
483532
TYPE_PARSER(construct<OmpDetachClause>(Parser<OmpObject>{}))

‎flang/lib/Parser/unparse.cpp

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2133,13 +2133,63 @@ class UnparseVisitor {
21332133
Walk(std::get<std::optional<std::list<Modifier>>>(x.t), ": ");
21342134
Walk(std::get<ScalarLogicalExpr>(x.t));
21352135
}
2136-
void Unparse(const OmpLinearClause::WithoutModifier &x) {
2137-
Walk(x.names, ", ");
2138-
Walk(":", x.step);
2136+
void Unparse(const OmpStepSimpleModifier &x) { Walk(x.v); }
2137+
void Unparse(const OmpStepComplexModifier &x) {
2138+
Word("STEP(");
2139+
Walk(x.v);
2140+
Put(")");
21392141
}
2140-
void Unparse(const OmpLinearClause::WithModifier &x) {
2141-
Walk(x.modifier), Put("("), Walk(x.names, ","), Put(")");
2142-
Walk(":", x.step);
2142+
void Unparse(const OmpLinearClause &x) {
2143+
using Modifier = OmpLinearClause::Modifier;
2144+
auto &modifiers{std::get<std::optional<std::list<Modifier>>>(x.t)};
2145+
if (std::get<bool>(x.t)) { // PostModified
2146+
Walk(std::get<OmpObjectList>(x.t));
2147+
Walk(": ", modifiers);
2148+
} else {
2149+
// Unparse using pre-5.2 syntax.
2150+
bool HasStepModifier{false}, HasLinearModifier{false};
2151+
2152+
if (modifiers) {
2153+
bool NeedComma{false};
2154+
for (const Modifier &m : *modifiers) {
2155+
// Print all linear modifiers in case we need to unparse an
2156+
// incorrect tree.
2157+
if (auto *lmod{std::get_if<parser::OmpLinearModifier>(&m.u)}) {
2158+
if (NeedComma) {
2159+
Put(",");
2160+
}
2161+
Walk(*lmod);
2162+
HasLinearModifier = true;
2163+
NeedComma = true;
2164+
} else {
2165+
// If not linear-modifier, then it has to be step modifier.
2166+
HasStepModifier = true;
2167+
}
2168+
}
2169+
}
2170+
2171+
if (HasLinearModifier) {
2172+
Put("(");
2173+
}
2174+
Walk(std::get<OmpObjectList>(x.t));
2175+
if (HasLinearModifier) {
2176+
Put(")");
2177+
}
2178+
2179+
if (HasStepModifier) {
2180+
Put(": ");
2181+
bool NeedComma{false};
2182+
for (const Modifier &m : *modifiers) {
2183+
if (!std::holds_alternative<parser::OmpLinearModifier>(m.u)) {
2184+
if (NeedComma) {
2185+
Put(",");
2186+
}
2187+
common::visit([&](auto &&s) { Walk(s); }, m.u);
2188+
NeedComma = true;
2189+
}
2190+
}
2191+
}
2192+
}
21432193
}
21442194
void Unparse(const OmpReductionClause &x) {
21452195
using Modifier = OmpReductionClause::Modifier;

0 commit comments

Comments
 (0)