-
Notifications
You must be signed in to change notification settings - Fork 12.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[flang][OpenMP] Apply modifier representation to semantic checks #116658
Conversation
This is the first part of the effort to make parsing of clause modifiers more uniform and robust. Currently, when multiple modifiers are allowed, the parser will expect them to appear in a hard-coded order. Additionally, modifier properties (such as "ultimate") are checked separately for each case. The overall plan is 1. Extract all modifiers into their own top-level classes, and then equip them with sets of common properties that will allow performing the property checks generically, without refering to the specific kind of the modifier. 2. Define a parser (as a separate class) for each modifier. 3. For each clause define a union (std::variant) of all allowable modifiers, and parse the modifiers as a list of these unions. The intent is also to isolate parts of the code that could eventually be auto-generated. OpenMP modifier overhaul: #1/3
The main issue to solve is that OpenMP modifiers can be specified in any order, so the parser cannot expect any specific modifier at a given position. To solve that, define modifier to be a union of all allowable specific modifiers for a given clause. Additionally, implement modifier descriptors: for each modifier the corresponding descriptor contains a set of properties of the modifier that allow a common set of semantic checks. Start with the syntactic properties defined in the spec: Required, Unique, Exclusive, Ultimate, and implement common checks to verify each of them. OpenMP modifier overhaul: #2/3
Also, define helper macros in parse-tree.h. Apply the new modifier representation to the DEFAULTMAP and REDUCTION clauses, with testcases utilizing the new modifier validation. OpenMP modifier overhaul: #3/3
@llvm/pr-subscribers-flang-semantics @llvm/pr-subscribers-flang-openmp Author: Krzysztof Parzyszek (kparzysz) ChangesAlso, define helper macros in parse-tree.h. Apply the new modifier representation to the DEFAULTMAP and REDUCTION clauses, with testcases utilizing the new modifier validation. OpenMP modifier overhaul: #3/3 Patch is 37.74 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/116658.diff 15 Files Affected:
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index df5bf1d8d3200e..9c59ce520a31aa 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -509,9 +509,11 @@ class ParseTreeDumper {
NODE(parser, OmpDeclareMapperSpecifier)
NODE(parser, OmpDefaultClause)
NODE_ENUM(OmpDefaultClause, Type)
+ NODE(parser, OmpVariableCategory)
+ NODE_ENUM(OmpVariableCategory, Value)
NODE(parser, OmpDefaultmapClause)
NODE_ENUM(OmpDefaultmapClause, ImplicitBehavior)
- NODE_ENUM(OmpDefaultmapClause, VariableCategory)
+ NODE(OmpDefaultmapClause, Modifier)
NODE(parser, OmpDependenceType)
NODE_ENUM(OmpDependenceType, Value)
NODE(parser, OmpTaskDependenceType)
@@ -567,8 +569,10 @@ class ParseTreeDumper {
NODE_ENUM(OmpBindClause, Type)
NODE(parser, OmpProcBindClause)
NODE_ENUM(OmpProcBindClause, Type)
- NODE_ENUM(OmpReductionClause, ReductionModifier)
+ NODE(parser, OmpReductionModifier)
+ NODE_ENUM(OmpReductionModifier, Value)
NODE(parser, OmpReductionClause)
+ NODE(OmpReductionClause, Modifier)
NODE(parser, OmpInReductionClause)
NODE(parser, OmpReductionCombiner)
NODE(OmpReductionCombiner, FunctionCombiner)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index ef49a36578270e..5b28bcd4e21b80 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3440,6 +3440,16 @@ struct OmpObject {
WRAPPER_CLASS(OmpObjectList, std::list<OmpObject>);
+#define MODIFIER_BOILERPLATE(...) \
+ struct Modifier { \
+ using Variant = std::variant<__VA_ARGS__>; \
+ UNION_CLASS_BOILERPLATE(Modifier); \
+ CharBlock source; \
+ Variant u; \
+ }
+
+#define MODIFIERS() std::optional<std::list<Modifier>>
+
inline namespace modifier {
// For uniformity, in all keyword modifiers the name of the type defined
// by ENUM_CLASS is "Value", e.g.
@@ -3505,12 +3515,20 @@ struct OmpLinearModifier {
// - | // since 4.5, until 5.2
// + | * | .AND. | .OR. | .EQV. | .NEQV. | // since 4.5
// MIN | MAX | IAND | IOR | IEOR // since 4.5
-//
struct OmpReductionIdentifier {
UNION_CLASS_BOILERPLATE(OmpReductionIdentifier);
std::variant<DefinedOperator, ProcedureDesignator> u;
};
+// Ref: [5.0:300-302], [5.1:332-334], [5.2:134-137]
+//
+// reduction-modifier ->
+// DEFAULT | INSCAN | TASK // since 5.0
+struct OmpReductionModifier {
+ ENUM_CLASS(Value, Default, Inscan, Task);
+ WRAPPER_CLASS_BOILERPLATE(OmpReductionModifier, Value);
+};
+
// 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
@@ -3521,6 +3539,17 @@ struct OmpTaskDependenceType {
ENUM_CLASS(Value, In, Out, Inout, Inoutset, Mutexinoutset, Depobj)
WRAPPER_CLASS_BOILERPLATE(OmpTaskDependenceType, Value);
};
+
+// Ref: [4.5:229-230], [5.0:324-325], [5.1:357-358], [5.2:161-162]
+//
+// variable-category ->
+// SCALAR | // since 4.5
+// AGGREGATE | ALLOCATABLE | POINTER | // since 5.0
+// ALL // since 5.2
+struct OmpVariableCategory {
+ ENUM_CLASS(Value, Aggregate, All, Allocatable, Pointer, Scalar)
+ WRAPPER_CLASS_BOILERPLATE(OmpVariableCategory, Value);
+};
} // namespace modifier
// --- Clauses
@@ -3578,8 +3607,8 @@ struct OmpDefaultmapClause {
TUPLE_CLASS_BOILERPLATE(OmpDefaultmapClause);
ENUM_CLASS(
ImplicitBehavior, Alloc, To, From, Tofrom, Firstprivate, None, Default)
- ENUM_CLASS(VariableCategory, All, Scalar, Aggregate, Allocatable, Pointer)
- std::tuple<ImplicitBehavior, std::optional<VariableCategory>> t;
+ MODIFIER_BOILERPLATE(OmpVariableCategory);
+ std::tuple<ImplicitBehavior, MODIFIERS()> t;
};
// 2.13.9 iteration-offset -> +/- non-negative-constant
@@ -3771,14 +3800,16 @@ struct OmpProcBindClause {
WRAPPER_CLASS_BOILERPLATE(OmpProcBindClause, Type);
};
-// 2.15.3.6 reduction-clause -> REDUCTION (reduction-identifier:
-// variable-name-list)
+// Ref: [4.5:201-207], [5.0:300-302], [5.1:332-334], [5.2:134-137]
+//
+// reduction-clause ->
+// REDUCTION(reduction-identifier: list) | // since 4.5
+// REDUCTION([reduction-modifier,]
+// reduction-identifier: list) // since 5.0
struct OmpReductionClause {
TUPLE_CLASS_BOILERPLATE(OmpReductionClause);
- ENUM_CLASS(ReductionModifier, Inscan, Task, Default)
- std::tuple<std::optional<ReductionModifier>, OmpReductionIdentifier,
- OmpObjectList>
- t;
+ MODIFIER_BOILERPLATE(OmpReductionModifier, OmpReductionIdentifier);
+ std::tuple<MODIFIERS(), OmpObjectList> t;
};
// 2.7.1 sched-modifier -> MONOTONIC | NONMONOTONIC | SIMD
diff --git a/flang/include/flang/Semantics/openmp-modifiers.h b/flang/include/flang/Semantics/openmp-modifiers.h
index 6be582761ed687..28fec7314cd8b5 100644
--- a/flang/include/flang/Semantics/openmp-modifiers.h
+++ b/flang/include/flang/Semantics/openmp-modifiers.h
@@ -70,7 +70,11 @@ const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpLinearModifier>();
template <>
const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpReductionIdentifier>();
template <>
+const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpReductionModifier>();
+template <>
const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpTaskDependenceType>();
+template <>
+const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpVariableCategory>();
// Explanation of terminology:
//
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index 5e514b4ba9f0da..3e0a11bc785e33 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -12,6 +12,7 @@
#include "flang/Evaluate/expression.h"
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/expression.h"
+#include "flang/Semantics/openmp-modifiers.h"
#include "flang/Semantics/symbol.h"
#include "llvm/Frontend/OpenMP/OMPConstants.h"
@@ -571,7 +572,8 @@ Defaultmap make(const parser::OmpClause::Defaultmap &inp,
);
CLAUSET_ENUM_CONVERT( //
- convert2, wrapped::VariableCategory, Defaultmap::VariableCategory,
+ convert2, parser::OmpVariableCategory::Value,
+ Defaultmap::VariableCategory,
// clang-format off
MS(Aggregate, Aggregate)
MS(All, All)
@@ -581,10 +583,11 @@ Defaultmap make(const parser::OmpClause::Defaultmap &inp,
// clang-format on
);
+ auto &mods{semantics::OmpGetModifiers(inp.v)};
auto &t0 = std::get<wrapped::ImplicitBehavior>(inp.v.t);
- auto &t1 = std::get<std::optional<wrapped::VariableCategory>>(inp.v.t);
+ auto *t1 = semantics::OmpGetUniqueModifier<parser::OmpVariableCategory>(mods);
- auto category = t1 ? convert2(*t1) : Defaultmap::VariableCategory::All;
+ auto category = t1 ? convert2(t1->v) : Defaultmap::VariableCategory::All;
return Defaultmap{{/*ImplicitBehavior=*/convert1(t0),
/*VariableCategory=*/category}};
}
@@ -1173,10 +1176,9 @@ ProcBind make(const parser::OmpClause::ProcBind &inp,
Reduction make(const parser::OmpClause::Reduction &inp,
semantics::SemanticsContext &semaCtx) {
// inp.v -> parser::OmpReductionClause
- using wrapped = parser::OmpReductionClause;
-
CLAUSET_ENUM_CONVERT( //
- convert, wrapped::ReductionModifier, Reduction::ReductionModifier,
+ convert, parser::OmpReductionModifier::Value,
+ Reduction::ReductionModifier,
// clang-format off
MS(Inscan, Inscan)
MS(Task, Task)
@@ -1184,16 +1186,17 @@ Reduction make(const parser::OmpClause::Reduction &inp,
// clang-format on
);
- auto &t0 =
- std::get<std::optional<parser::OmpReductionClause::ReductionModifier>>(
- inp.v.t);
- auto &t1 = std::get<parser::OmpReductionIdentifier>(inp.v.t);
+ auto &mods = semantics::OmpGetModifiers(inp.v);
+ auto *t0 =
+ semantics::OmpGetUniqueModifier<parser::OmpReductionModifier>(mods);
+ auto *t1 =
+ semantics::OmpGetUniqueModifier<parser::OmpReductionIdentifier>(mods);
auto &t2 = std::get<parser::OmpObjectList>(inp.v.t);
return Reduction{
{/*ReductionModifier=*/t0
- ? std::make_optional<Reduction::ReductionModifier>(convert(*t0))
+ ? std::make_optional<Reduction::ReductionModifier>(convert(t0->v))
: std::nullopt,
- /*ReductionIdentifiers=*/{makeReductionOperator(t1, semaCtx)},
+ /*ReductionIdentifiers=*/{makeReductionOperator(*t1, semaCtx)},
/*List=*/makeObjects(t2, semaCtx)}};
}
@@ -1314,10 +1317,12 @@ Permutation make(const parser::OmpClause::Permutation &inp,
TaskReduction make(const parser::OmpClause::TaskReduction &inp,
semantics::SemanticsContext &semaCtx) {
// inp.v -> parser::OmpReductionClause
- auto &t0 = std::get<parser::OmpReductionIdentifier>(inp.v.t);
+ auto &mods = semantics::OmpGetModifiers(inp.v);
+ auto *t0 =
+ semantics::OmpGetUniqueModifier<parser::OmpReductionIdentifier>(mods);
auto &t1 = std::get<parser::OmpObjectList>(inp.v.t);
return TaskReduction{
- {/*ReductionIdentifiers=*/{makeReductionOperator(t0, semaCtx)},
+ {/*ReductionIdentifiers=*/{makeReductionOperator(*t0, semaCtx)},
/*List=*/makeObjects(t1, semaCtx)}};
}
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index b4d45873abb3ec..063201fc86ca41 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -229,6 +229,11 @@ TYPE_PARSER(construct<OmpLinearModifier>( //
TYPE_PARSER(construct<OmpReductionIdentifier>(Parser<DefinedOperator>{}) ||
construct<OmpReductionIdentifier>(Parser<ProcedureDesignator>{}))
+TYPE_PARSER(construct<OmpReductionModifier>(
+ "INSCAN" >> pure(OmpReductionModifier::Value::Inscan) ||
+ "TASK" >> pure(OmpReductionModifier::Value::Task) ||
+ "DEFAULT" >> pure(OmpReductionModifier::Value::Default)))
+
TYPE_PARSER(construct<OmpTaskDependenceType>(
"DEPOBJ" >> pure(OmpTaskDependenceType::Value::Depobj) ||
"IN"_id >> pure(OmpTaskDependenceType::Value::In) ||
@@ -237,6 +242,22 @@ TYPE_PARSER(construct<OmpTaskDependenceType>(
"MUTEXINOUTSET" >> pure(OmpTaskDependenceType::Value::Mutexinoutset) ||
"OUT" >> pure(OmpTaskDependenceType::Value::Out)))
+// This could be auto-generated.
+TYPE_PARSER(sourced(construct<OmpReductionClause::Modifier>(sourced(
+ construct<OmpReductionClause::Modifier>(Parser<OmpReductionModifier>{}) ||
+ construct<OmpReductionClause::Modifier>(
+ Parser<OmpReductionIdentifier>{})))))
+
+TYPE_PARSER(construct<OmpVariableCategory>(
+ "AGGREGATE" >> pure(OmpVariableCategory::Value::Aggregate) ||
+ "ALL"_id >> pure(OmpVariableCategory::Value::All) ||
+ "ALLOCATABLE" >> pure(OmpVariableCategory::Value::Allocatable) ||
+ "POINTER" >> pure(OmpVariableCategory::Value::Pointer) ||
+ "SCALAR" >> pure(OmpVariableCategory::Value::Scalar)))
+
+TYPE_PARSER(sourced(construct<OmpDefaultmapClause::Modifier>(
+ Parser<OmpVariableCategory>{})))
+
// --- Parsers for clauses --------------------------------------------
// [5.0] 2.10.1 affinity([aff-modifier:] locator-list)
@@ -309,16 +330,7 @@ TYPE_PARSER(construct<OmpDefaultmapClause>(
pure(OmpDefaultmapClause::ImplicitBehavior::Firstprivate) ||
"NONE" >> pure(OmpDefaultmapClause::ImplicitBehavior::None) ||
"DEFAULT" >> pure(OmpDefaultmapClause::ImplicitBehavior::Default)),
- maybe(":" >>
- construct<OmpDefaultmapClause::VariableCategory>(
- "ALL"_id >> pure(OmpDefaultmapClause::VariableCategory::All) ||
- "SCALAR" >> pure(OmpDefaultmapClause::VariableCategory::Scalar) ||
- "AGGREGATE" >>
- pure(OmpDefaultmapClause::VariableCategory::Aggregate) ||
- "ALLOCATABLE" >>
- pure(OmpDefaultmapClause::VariableCategory::Allocatable) ||
- "POINTER" >>
- pure(OmpDefaultmapClause::VariableCategory::Pointer)))))
+ maybe(":" >> nonemptyList(Parser<OmpDefaultmapClause::Modifier>{}))))
// 2.7.1 SCHEDULE ([modifier1 [, modifier2]:]kind[, chunk_size])
// Modifier -> MONITONIC | NONMONOTONIC | SIMD
@@ -375,12 +387,8 @@ TYPE_PARSER(construct<OmpIfClause>(
scalarLogicalExpr))
TYPE_PARSER(construct<OmpReductionClause>(
- maybe(
- ("INSCAN" >> pure(OmpReductionClause::ReductionModifier::Inscan) ||
- "TASK" >> pure(OmpReductionClause::ReductionModifier::Task) ||
- "DEFAULT" >> pure(OmpReductionClause::ReductionModifier::Default)) /
- ","),
- Parser<OmpReductionIdentifier>{} / ":", Parser<OmpObjectList>{}))
+ maybe(nonemptyList(Parser<OmpReductionClause::Modifier>{}) / ":"),
+ Parser<OmpObjectList>{}))
// OMP 5.0 2.19.5.6 IN_REDUCTION (reduction-identifier: variable-name-list)
TYPE_PARSER(construct<OmpInReductionClause>(
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index b2a0256cb58d0c..e06f4dea80f488 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2179,10 +2179,8 @@ class UnparseVisitor {
Walk(":", x.step);
}
void Unparse(const OmpReductionClause &x) {
- Walk(std::get<std::optional<OmpReductionClause::ReductionModifier>>(x.t),
- ",");
- Walk(std::get<OmpReductionIdentifier>(x.t));
- Put(":");
+ using Modifier = OmpReductionClause::Modifier;
+ Walk(std::get<std::optional<std::list<Modifier>>>(x.t), ":");
Walk(std::get<OmpObjectList>(x.t));
}
void Unparse(const OmpDetachClause &x) { Walk(x.v); }
@@ -2246,9 +2244,9 @@ class UnparseVisitor {
Walk(std::get<OmpObjectList>(x.t));
}
void Unparse(const OmpDefaultmapClause &x) {
+ using Modifier = OmpDefaultmapClause::Modifier;
Walk(std::get<OmpDefaultmapClause::ImplicitBehavior>(x.t));
- Walk(":",
- std::get<std::optional<OmpDefaultmapClause::VariableCategory>>(x.t));
+ Walk(":", std::get<std::optional<std::list<Modifier>>>(x.t));
}
void Unparse(const OmpToClause &x) {
auto &expect{
@@ -2896,7 +2894,7 @@ class UnparseVisitor {
WALK_NESTED_ENUM(OmpProcBindClause, Type) // OMP PROC_BIND
WALK_NESTED_ENUM(OmpDefaultClause, Type) // OMP DEFAULT
WALK_NESTED_ENUM(OmpDefaultmapClause, ImplicitBehavior) // OMP DEFAULTMAP
- WALK_NESTED_ENUM(OmpDefaultmapClause, VariableCategory) // OMP DEFAULTMAP
+ WALK_NESTED_ENUM(OmpVariableCategory, Value) // OMP variable-category
WALK_NESTED_ENUM(
OmpLastprivateClause, LastprivateModifier) // OMP lastprivate-modifier
WALK_NESTED_ENUM(OmpScheduleModifierType, ModType) // OMP schedule-modifier
@@ -2905,8 +2903,7 @@ class UnparseVisitor {
WALK_NESTED_ENUM(OmpScheduleClause, ScheduleType) // OMP schedule-type
WALK_NESTED_ENUM(OmpDeviceClause, DeviceModifier) // OMP device modifier
WALK_NESTED_ENUM(OmpDeviceTypeClause, Type) // OMP DEVICE_TYPE
- WALK_NESTED_ENUM(
- OmpReductionClause, ReductionModifier) // OMP reduction-modifier
+ WALK_NESTED_ENUM(OmpReductionModifier, Value) // OMP reduction-modifier
WALK_NESTED_ENUM(OmpFromClause, Expectation) // OMP motion-expectation
WALK_NESTED_ENUM(OmpIfClause, DirectiveNameModifier) // OMP directive-modifier
WALK_NESTED_ENUM(OmpCancelType, Type) // OMP cancel-type
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 9cac652216fcf2..079d0fd17bfac1 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -11,6 +11,7 @@
#include "flang/Evaluate/check-expression.h"
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/expression.h"
+#include "flang/Semantics/openmp-modifiers.h"
#include "flang/Semantics/tools.h"
#include <variant>
@@ -195,7 +196,7 @@ bool OmpStructureChecker::CheckAllowedClause(llvmOmpClause clause) {
if (!llvm::omp::isAllowedClauseForDirective(dir, clause, version)) {
unsigned allowedInVersion{[&] {
- for (unsigned v : {45, 50, 51, 52, 60}) {
+ for (unsigned v : llvm::omp::getOpenMPVersions()) {
if (v <= version) {
continue;
}
@@ -979,12 +980,14 @@ void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &x) {
// constructs inside LOOP may add the relevant information. Scan reduction is
// supported only in loop constructs, so same checks are not applicable to
// other directives.
+ using ReductionModifier = parser::OmpReductionModifier;
for (const auto &clause : clauseList.v) {
if (const auto *reductionClause{
std::get_if<parser::OmpClause::Reduction>(&clause.u)}) {
- const auto &maybeModifier{
- std::get<std::optional<ReductionModifier>>(reductionClause->v.t)};
- if (maybeModifier && *maybeModifier == ReductionModifier::Inscan) {
+ auto &modifiers{OmpGetModifiers(reductionClause->v)};
+ auto *maybeModifier{OmpGetUniqueModifier<ReductionModifier>(modifiers)};
+ if (maybeModifier &&
+ maybeModifier->v == ReductionModifier::Value::Inscan) {
const auto &objectList{
std::get<parser::OmpObjectList>(reductionClause->v.t)};
auto checkReductionSymbolInScan = [&](const parser::Name *name) {
@@ -2850,20 +2853,27 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Destroy &x) {
void OmpStructureChecker::Enter(const parser::OmpClause::Reduction &x) {
CheckAllowedClause(llvm::omp::Clause::OMPC_reduction);
- if (CheckReductionOperators(x)) {
- CheckReductionTypeList(x);
- }
- if (const auto &maybeModifier{
- std::get<std::optional<ReductionModifier>>(x.v.t)}) {
- const ReductionModifier modifier{*maybeModifier};
- CheckReductionModifier(modifier);
+ if (OmpVerifyModifiers(x.v, GetContext().clauseSource, context_)) {
+ if (CheckReductionOperators(x)) {
+ CheckReductionTypeList(x);
+ }
+ auto &modifiers{OmpGetModifiers(x.v)};
+ using ReductionModifier = parser::OmpReductionModifier;
+ if (auto *maybeModifier{
+ OmpGetUniqueModifier<ReductionModifier>(modifiers)}) {
+ CheckReductionModifier(*maybeModifier);
+ }
}
}
bool OmpStructureChecker::CheckReductionOperators(
const parser::OmpClause::Reduction &x) {
-
- const auto &definedOp{std::get<parser::OmpReductionIdentifier>(x.v.t)};
+ auto &modifiers{OmpGetModifiers(x.v)};
+ const auto *definedOp{
+ OmpGetUniqueModifier<parser::OmpReductionIdentifier>(modifiers)};
+ if (!definedOp) {
+ return false;
+ }
bool ok = false;
common::visit(
common::visitors{
@@ -2896,7 +2906,7 @@ bool OmpStructureChecker::CheckReductionOperators(
}
},
},
- definedOp.u);
+ definedOp->u);
return ok;
}
@@ -2928,7 +2938,12 @@ bool OmpStructureChecker::CheckIntrinsicOperator(
static bool IsReductionAllowedForType(
const parser::OmpClause::Reduction &x, const DeclTypeSpec &type) {
- const auto &definedOp{std::get<parser::OmpReductionIdentifier>(x.v.t)};
+ auto &modifiers{OmpGetModifiers(x.v)};
+ const auto *definedOp{
+ OmpGetUniqueModifier<parser::OmpReductionIdentifier>(modifiers)};
+ if (!definedOp) {
+ return false;
+ }
// TODO: user defined reduction operators. Just allow everything for now.
bool ok{true};
@@ -3002,7 +3017,7 @@ static bool IsReductionAllowedForType(
}
},
},
- definedOp.u);
+ definedOp->u);
return ok;
}
@@ -3035,8 +3050,9 @@ void OmpStructureChecker::CheckReductionTypeList(
}
void OmpStructureChecker::CheckReductionModifier(
- const ReductionModifier &modifier) {
- if (modifier == ReductionModifier::Default) {
+ const parser::OmpReductionModifier &modifier) {
+ using ReductionModifier = parser::OmpReductionModifier;
+ if (modifier.v == ReductionModifier::Value::Default) {
// The default one is always ok.
return;
}
@@ -3049,7 +3065,7 @@ void OmpStructureChecker::CheckReductionModifier(
context_.Say(GetContext().clauseSource,
"REDUCTION modifier on LOOP directive must be DEFAULT"_err_en_US);
}
- if (modifier == ReductionModifier::Task) {
+ if (modifier.v == ReductionModifier::Value::Task) {
// "Task" is only allowed on...
[truncated]
|
@llvm/pr-subscribers-flang-parser Author: Krzysztof Parzyszek (kparzysz) ChangesAlso, define helper macros in parse-tree.h. Apply the new modifier representation to the DEFAULTMAP and REDUCTION clauses, with testcases utilizing the new modifier validation. OpenMP modifier overhaul: #3/3 Patch is 37.74 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/116658.diff 15 Files Affected:
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index df5bf1d8d3200e..9c59ce520a31aa 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -509,9 +509,11 @@ class ParseTreeDumper {
NODE(parser, OmpDeclareMapperSpecifier)
NODE(parser, OmpDefaultClause)
NODE_ENUM(OmpDefaultClause, Type)
+ NODE(parser, OmpVariableCategory)
+ NODE_ENUM(OmpVariableCategory, Value)
NODE(parser, OmpDefaultmapClause)
NODE_ENUM(OmpDefaultmapClause, ImplicitBehavior)
- NODE_ENUM(OmpDefaultmapClause, VariableCategory)
+ NODE(OmpDefaultmapClause, Modifier)
NODE(parser, OmpDependenceType)
NODE_ENUM(OmpDependenceType, Value)
NODE(parser, OmpTaskDependenceType)
@@ -567,8 +569,10 @@ class ParseTreeDumper {
NODE_ENUM(OmpBindClause, Type)
NODE(parser, OmpProcBindClause)
NODE_ENUM(OmpProcBindClause, Type)
- NODE_ENUM(OmpReductionClause, ReductionModifier)
+ NODE(parser, OmpReductionModifier)
+ NODE_ENUM(OmpReductionModifier, Value)
NODE(parser, OmpReductionClause)
+ NODE(OmpReductionClause, Modifier)
NODE(parser, OmpInReductionClause)
NODE(parser, OmpReductionCombiner)
NODE(OmpReductionCombiner, FunctionCombiner)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index ef49a36578270e..5b28bcd4e21b80 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3440,6 +3440,16 @@ struct OmpObject {
WRAPPER_CLASS(OmpObjectList, std::list<OmpObject>);
+#define MODIFIER_BOILERPLATE(...) \
+ struct Modifier { \
+ using Variant = std::variant<__VA_ARGS__>; \
+ UNION_CLASS_BOILERPLATE(Modifier); \
+ CharBlock source; \
+ Variant u; \
+ }
+
+#define MODIFIERS() std::optional<std::list<Modifier>>
+
inline namespace modifier {
// For uniformity, in all keyword modifiers the name of the type defined
// by ENUM_CLASS is "Value", e.g.
@@ -3505,12 +3515,20 @@ struct OmpLinearModifier {
// - | // since 4.5, until 5.2
// + | * | .AND. | .OR. | .EQV. | .NEQV. | // since 4.5
// MIN | MAX | IAND | IOR | IEOR // since 4.5
-//
struct OmpReductionIdentifier {
UNION_CLASS_BOILERPLATE(OmpReductionIdentifier);
std::variant<DefinedOperator, ProcedureDesignator> u;
};
+// Ref: [5.0:300-302], [5.1:332-334], [5.2:134-137]
+//
+// reduction-modifier ->
+// DEFAULT | INSCAN | TASK // since 5.0
+struct OmpReductionModifier {
+ ENUM_CLASS(Value, Default, Inscan, Task);
+ WRAPPER_CLASS_BOILERPLATE(OmpReductionModifier, Value);
+};
+
// 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
@@ -3521,6 +3539,17 @@ struct OmpTaskDependenceType {
ENUM_CLASS(Value, In, Out, Inout, Inoutset, Mutexinoutset, Depobj)
WRAPPER_CLASS_BOILERPLATE(OmpTaskDependenceType, Value);
};
+
+// Ref: [4.5:229-230], [5.0:324-325], [5.1:357-358], [5.2:161-162]
+//
+// variable-category ->
+// SCALAR | // since 4.5
+// AGGREGATE | ALLOCATABLE | POINTER | // since 5.0
+// ALL // since 5.2
+struct OmpVariableCategory {
+ ENUM_CLASS(Value, Aggregate, All, Allocatable, Pointer, Scalar)
+ WRAPPER_CLASS_BOILERPLATE(OmpVariableCategory, Value);
+};
} // namespace modifier
// --- Clauses
@@ -3578,8 +3607,8 @@ struct OmpDefaultmapClause {
TUPLE_CLASS_BOILERPLATE(OmpDefaultmapClause);
ENUM_CLASS(
ImplicitBehavior, Alloc, To, From, Tofrom, Firstprivate, None, Default)
- ENUM_CLASS(VariableCategory, All, Scalar, Aggregate, Allocatable, Pointer)
- std::tuple<ImplicitBehavior, std::optional<VariableCategory>> t;
+ MODIFIER_BOILERPLATE(OmpVariableCategory);
+ std::tuple<ImplicitBehavior, MODIFIERS()> t;
};
// 2.13.9 iteration-offset -> +/- non-negative-constant
@@ -3771,14 +3800,16 @@ struct OmpProcBindClause {
WRAPPER_CLASS_BOILERPLATE(OmpProcBindClause, Type);
};
-// 2.15.3.6 reduction-clause -> REDUCTION (reduction-identifier:
-// variable-name-list)
+// Ref: [4.5:201-207], [5.0:300-302], [5.1:332-334], [5.2:134-137]
+//
+// reduction-clause ->
+// REDUCTION(reduction-identifier: list) | // since 4.5
+// REDUCTION([reduction-modifier,]
+// reduction-identifier: list) // since 5.0
struct OmpReductionClause {
TUPLE_CLASS_BOILERPLATE(OmpReductionClause);
- ENUM_CLASS(ReductionModifier, Inscan, Task, Default)
- std::tuple<std::optional<ReductionModifier>, OmpReductionIdentifier,
- OmpObjectList>
- t;
+ MODIFIER_BOILERPLATE(OmpReductionModifier, OmpReductionIdentifier);
+ std::tuple<MODIFIERS(), OmpObjectList> t;
};
// 2.7.1 sched-modifier -> MONOTONIC | NONMONOTONIC | SIMD
diff --git a/flang/include/flang/Semantics/openmp-modifiers.h b/flang/include/flang/Semantics/openmp-modifiers.h
index 6be582761ed687..28fec7314cd8b5 100644
--- a/flang/include/flang/Semantics/openmp-modifiers.h
+++ b/flang/include/flang/Semantics/openmp-modifiers.h
@@ -70,7 +70,11 @@ const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpLinearModifier>();
template <>
const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpReductionIdentifier>();
template <>
+const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpReductionModifier>();
+template <>
const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpTaskDependenceType>();
+template <>
+const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpVariableCategory>();
// Explanation of terminology:
//
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index 5e514b4ba9f0da..3e0a11bc785e33 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -12,6 +12,7 @@
#include "flang/Evaluate/expression.h"
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/expression.h"
+#include "flang/Semantics/openmp-modifiers.h"
#include "flang/Semantics/symbol.h"
#include "llvm/Frontend/OpenMP/OMPConstants.h"
@@ -571,7 +572,8 @@ Defaultmap make(const parser::OmpClause::Defaultmap &inp,
);
CLAUSET_ENUM_CONVERT( //
- convert2, wrapped::VariableCategory, Defaultmap::VariableCategory,
+ convert2, parser::OmpVariableCategory::Value,
+ Defaultmap::VariableCategory,
// clang-format off
MS(Aggregate, Aggregate)
MS(All, All)
@@ -581,10 +583,11 @@ Defaultmap make(const parser::OmpClause::Defaultmap &inp,
// clang-format on
);
+ auto &mods{semantics::OmpGetModifiers(inp.v)};
auto &t0 = std::get<wrapped::ImplicitBehavior>(inp.v.t);
- auto &t1 = std::get<std::optional<wrapped::VariableCategory>>(inp.v.t);
+ auto *t1 = semantics::OmpGetUniqueModifier<parser::OmpVariableCategory>(mods);
- auto category = t1 ? convert2(*t1) : Defaultmap::VariableCategory::All;
+ auto category = t1 ? convert2(t1->v) : Defaultmap::VariableCategory::All;
return Defaultmap{{/*ImplicitBehavior=*/convert1(t0),
/*VariableCategory=*/category}};
}
@@ -1173,10 +1176,9 @@ ProcBind make(const parser::OmpClause::ProcBind &inp,
Reduction make(const parser::OmpClause::Reduction &inp,
semantics::SemanticsContext &semaCtx) {
// inp.v -> parser::OmpReductionClause
- using wrapped = parser::OmpReductionClause;
-
CLAUSET_ENUM_CONVERT( //
- convert, wrapped::ReductionModifier, Reduction::ReductionModifier,
+ convert, parser::OmpReductionModifier::Value,
+ Reduction::ReductionModifier,
// clang-format off
MS(Inscan, Inscan)
MS(Task, Task)
@@ -1184,16 +1186,17 @@ Reduction make(const parser::OmpClause::Reduction &inp,
// clang-format on
);
- auto &t0 =
- std::get<std::optional<parser::OmpReductionClause::ReductionModifier>>(
- inp.v.t);
- auto &t1 = std::get<parser::OmpReductionIdentifier>(inp.v.t);
+ auto &mods = semantics::OmpGetModifiers(inp.v);
+ auto *t0 =
+ semantics::OmpGetUniqueModifier<parser::OmpReductionModifier>(mods);
+ auto *t1 =
+ semantics::OmpGetUniqueModifier<parser::OmpReductionIdentifier>(mods);
auto &t2 = std::get<parser::OmpObjectList>(inp.v.t);
return Reduction{
{/*ReductionModifier=*/t0
- ? std::make_optional<Reduction::ReductionModifier>(convert(*t0))
+ ? std::make_optional<Reduction::ReductionModifier>(convert(t0->v))
: std::nullopt,
- /*ReductionIdentifiers=*/{makeReductionOperator(t1, semaCtx)},
+ /*ReductionIdentifiers=*/{makeReductionOperator(*t1, semaCtx)},
/*List=*/makeObjects(t2, semaCtx)}};
}
@@ -1314,10 +1317,12 @@ Permutation make(const parser::OmpClause::Permutation &inp,
TaskReduction make(const parser::OmpClause::TaskReduction &inp,
semantics::SemanticsContext &semaCtx) {
// inp.v -> parser::OmpReductionClause
- auto &t0 = std::get<parser::OmpReductionIdentifier>(inp.v.t);
+ auto &mods = semantics::OmpGetModifiers(inp.v);
+ auto *t0 =
+ semantics::OmpGetUniqueModifier<parser::OmpReductionIdentifier>(mods);
auto &t1 = std::get<parser::OmpObjectList>(inp.v.t);
return TaskReduction{
- {/*ReductionIdentifiers=*/{makeReductionOperator(t0, semaCtx)},
+ {/*ReductionIdentifiers=*/{makeReductionOperator(*t0, semaCtx)},
/*List=*/makeObjects(t1, semaCtx)}};
}
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index b4d45873abb3ec..063201fc86ca41 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -229,6 +229,11 @@ TYPE_PARSER(construct<OmpLinearModifier>( //
TYPE_PARSER(construct<OmpReductionIdentifier>(Parser<DefinedOperator>{}) ||
construct<OmpReductionIdentifier>(Parser<ProcedureDesignator>{}))
+TYPE_PARSER(construct<OmpReductionModifier>(
+ "INSCAN" >> pure(OmpReductionModifier::Value::Inscan) ||
+ "TASK" >> pure(OmpReductionModifier::Value::Task) ||
+ "DEFAULT" >> pure(OmpReductionModifier::Value::Default)))
+
TYPE_PARSER(construct<OmpTaskDependenceType>(
"DEPOBJ" >> pure(OmpTaskDependenceType::Value::Depobj) ||
"IN"_id >> pure(OmpTaskDependenceType::Value::In) ||
@@ -237,6 +242,22 @@ TYPE_PARSER(construct<OmpTaskDependenceType>(
"MUTEXINOUTSET" >> pure(OmpTaskDependenceType::Value::Mutexinoutset) ||
"OUT" >> pure(OmpTaskDependenceType::Value::Out)))
+// This could be auto-generated.
+TYPE_PARSER(sourced(construct<OmpReductionClause::Modifier>(sourced(
+ construct<OmpReductionClause::Modifier>(Parser<OmpReductionModifier>{}) ||
+ construct<OmpReductionClause::Modifier>(
+ Parser<OmpReductionIdentifier>{})))))
+
+TYPE_PARSER(construct<OmpVariableCategory>(
+ "AGGREGATE" >> pure(OmpVariableCategory::Value::Aggregate) ||
+ "ALL"_id >> pure(OmpVariableCategory::Value::All) ||
+ "ALLOCATABLE" >> pure(OmpVariableCategory::Value::Allocatable) ||
+ "POINTER" >> pure(OmpVariableCategory::Value::Pointer) ||
+ "SCALAR" >> pure(OmpVariableCategory::Value::Scalar)))
+
+TYPE_PARSER(sourced(construct<OmpDefaultmapClause::Modifier>(
+ Parser<OmpVariableCategory>{})))
+
// --- Parsers for clauses --------------------------------------------
// [5.0] 2.10.1 affinity([aff-modifier:] locator-list)
@@ -309,16 +330,7 @@ TYPE_PARSER(construct<OmpDefaultmapClause>(
pure(OmpDefaultmapClause::ImplicitBehavior::Firstprivate) ||
"NONE" >> pure(OmpDefaultmapClause::ImplicitBehavior::None) ||
"DEFAULT" >> pure(OmpDefaultmapClause::ImplicitBehavior::Default)),
- maybe(":" >>
- construct<OmpDefaultmapClause::VariableCategory>(
- "ALL"_id >> pure(OmpDefaultmapClause::VariableCategory::All) ||
- "SCALAR" >> pure(OmpDefaultmapClause::VariableCategory::Scalar) ||
- "AGGREGATE" >>
- pure(OmpDefaultmapClause::VariableCategory::Aggregate) ||
- "ALLOCATABLE" >>
- pure(OmpDefaultmapClause::VariableCategory::Allocatable) ||
- "POINTER" >>
- pure(OmpDefaultmapClause::VariableCategory::Pointer)))))
+ maybe(":" >> nonemptyList(Parser<OmpDefaultmapClause::Modifier>{}))))
// 2.7.1 SCHEDULE ([modifier1 [, modifier2]:]kind[, chunk_size])
// Modifier -> MONITONIC | NONMONOTONIC | SIMD
@@ -375,12 +387,8 @@ TYPE_PARSER(construct<OmpIfClause>(
scalarLogicalExpr))
TYPE_PARSER(construct<OmpReductionClause>(
- maybe(
- ("INSCAN" >> pure(OmpReductionClause::ReductionModifier::Inscan) ||
- "TASK" >> pure(OmpReductionClause::ReductionModifier::Task) ||
- "DEFAULT" >> pure(OmpReductionClause::ReductionModifier::Default)) /
- ","),
- Parser<OmpReductionIdentifier>{} / ":", Parser<OmpObjectList>{}))
+ maybe(nonemptyList(Parser<OmpReductionClause::Modifier>{}) / ":"),
+ Parser<OmpObjectList>{}))
// OMP 5.0 2.19.5.6 IN_REDUCTION (reduction-identifier: variable-name-list)
TYPE_PARSER(construct<OmpInReductionClause>(
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index b2a0256cb58d0c..e06f4dea80f488 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2179,10 +2179,8 @@ class UnparseVisitor {
Walk(":", x.step);
}
void Unparse(const OmpReductionClause &x) {
- Walk(std::get<std::optional<OmpReductionClause::ReductionModifier>>(x.t),
- ",");
- Walk(std::get<OmpReductionIdentifier>(x.t));
- Put(":");
+ using Modifier = OmpReductionClause::Modifier;
+ Walk(std::get<std::optional<std::list<Modifier>>>(x.t), ":");
Walk(std::get<OmpObjectList>(x.t));
}
void Unparse(const OmpDetachClause &x) { Walk(x.v); }
@@ -2246,9 +2244,9 @@ class UnparseVisitor {
Walk(std::get<OmpObjectList>(x.t));
}
void Unparse(const OmpDefaultmapClause &x) {
+ using Modifier = OmpDefaultmapClause::Modifier;
Walk(std::get<OmpDefaultmapClause::ImplicitBehavior>(x.t));
- Walk(":",
- std::get<std::optional<OmpDefaultmapClause::VariableCategory>>(x.t));
+ Walk(":", std::get<std::optional<std::list<Modifier>>>(x.t));
}
void Unparse(const OmpToClause &x) {
auto &expect{
@@ -2896,7 +2894,7 @@ class UnparseVisitor {
WALK_NESTED_ENUM(OmpProcBindClause, Type) // OMP PROC_BIND
WALK_NESTED_ENUM(OmpDefaultClause, Type) // OMP DEFAULT
WALK_NESTED_ENUM(OmpDefaultmapClause, ImplicitBehavior) // OMP DEFAULTMAP
- WALK_NESTED_ENUM(OmpDefaultmapClause, VariableCategory) // OMP DEFAULTMAP
+ WALK_NESTED_ENUM(OmpVariableCategory, Value) // OMP variable-category
WALK_NESTED_ENUM(
OmpLastprivateClause, LastprivateModifier) // OMP lastprivate-modifier
WALK_NESTED_ENUM(OmpScheduleModifierType, ModType) // OMP schedule-modifier
@@ -2905,8 +2903,7 @@ class UnparseVisitor {
WALK_NESTED_ENUM(OmpScheduleClause, ScheduleType) // OMP schedule-type
WALK_NESTED_ENUM(OmpDeviceClause, DeviceModifier) // OMP device modifier
WALK_NESTED_ENUM(OmpDeviceTypeClause, Type) // OMP DEVICE_TYPE
- WALK_NESTED_ENUM(
- OmpReductionClause, ReductionModifier) // OMP reduction-modifier
+ WALK_NESTED_ENUM(OmpReductionModifier, Value) // OMP reduction-modifier
WALK_NESTED_ENUM(OmpFromClause, Expectation) // OMP motion-expectation
WALK_NESTED_ENUM(OmpIfClause, DirectiveNameModifier) // OMP directive-modifier
WALK_NESTED_ENUM(OmpCancelType, Type) // OMP cancel-type
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 9cac652216fcf2..079d0fd17bfac1 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -11,6 +11,7 @@
#include "flang/Evaluate/check-expression.h"
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/expression.h"
+#include "flang/Semantics/openmp-modifiers.h"
#include "flang/Semantics/tools.h"
#include <variant>
@@ -195,7 +196,7 @@ bool OmpStructureChecker::CheckAllowedClause(llvmOmpClause clause) {
if (!llvm::omp::isAllowedClauseForDirective(dir, clause, version)) {
unsigned allowedInVersion{[&] {
- for (unsigned v : {45, 50, 51, 52, 60}) {
+ for (unsigned v : llvm::omp::getOpenMPVersions()) {
if (v <= version) {
continue;
}
@@ -979,12 +980,14 @@ void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &x) {
// constructs inside LOOP may add the relevant information. Scan reduction is
// supported only in loop constructs, so same checks are not applicable to
// other directives.
+ using ReductionModifier = parser::OmpReductionModifier;
for (const auto &clause : clauseList.v) {
if (const auto *reductionClause{
std::get_if<parser::OmpClause::Reduction>(&clause.u)}) {
- const auto &maybeModifier{
- std::get<std::optional<ReductionModifier>>(reductionClause->v.t)};
- if (maybeModifier && *maybeModifier == ReductionModifier::Inscan) {
+ auto &modifiers{OmpGetModifiers(reductionClause->v)};
+ auto *maybeModifier{OmpGetUniqueModifier<ReductionModifier>(modifiers)};
+ if (maybeModifier &&
+ maybeModifier->v == ReductionModifier::Value::Inscan) {
const auto &objectList{
std::get<parser::OmpObjectList>(reductionClause->v.t)};
auto checkReductionSymbolInScan = [&](const parser::Name *name) {
@@ -2850,20 +2853,27 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Destroy &x) {
void OmpStructureChecker::Enter(const parser::OmpClause::Reduction &x) {
CheckAllowedClause(llvm::omp::Clause::OMPC_reduction);
- if (CheckReductionOperators(x)) {
- CheckReductionTypeList(x);
- }
- if (const auto &maybeModifier{
- std::get<std::optional<ReductionModifier>>(x.v.t)}) {
- const ReductionModifier modifier{*maybeModifier};
- CheckReductionModifier(modifier);
+ if (OmpVerifyModifiers(x.v, GetContext().clauseSource, context_)) {
+ if (CheckReductionOperators(x)) {
+ CheckReductionTypeList(x);
+ }
+ auto &modifiers{OmpGetModifiers(x.v)};
+ using ReductionModifier = parser::OmpReductionModifier;
+ if (auto *maybeModifier{
+ OmpGetUniqueModifier<ReductionModifier>(modifiers)}) {
+ CheckReductionModifier(*maybeModifier);
+ }
}
}
bool OmpStructureChecker::CheckReductionOperators(
const parser::OmpClause::Reduction &x) {
-
- const auto &definedOp{std::get<parser::OmpReductionIdentifier>(x.v.t)};
+ auto &modifiers{OmpGetModifiers(x.v)};
+ const auto *definedOp{
+ OmpGetUniqueModifier<parser::OmpReductionIdentifier>(modifiers)};
+ if (!definedOp) {
+ return false;
+ }
bool ok = false;
common::visit(
common::visitors{
@@ -2896,7 +2906,7 @@ bool OmpStructureChecker::CheckReductionOperators(
}
},
},
- definedOp.u);
+ definedOp->u);
return ok;
}
@@ -2928,7 +2938,12 @@ bool OmpStructureChecker::CheckIntrinsicOperator(
static bool IsReductionAllowedForType(
const parser::OmpClause::Reduction &x, const DeclTypeSpec &type) {
- const auto &definedOp{std::get<parser::OmpReductionIdentifier>(x.v.t)};
+ auto &modifiers{OmpGetModifiers(x.v)};
+ const auto *definedOp{
+ OmpGetUniqueModifier<parser::OmpReductionIdentifier>(modifiers)};
+ if (!definedOp) {
+ return false;
+ }
// TODO: user defined reduction operators. Just allow everything for now.
bool ok{true};
@@ -3002,7 +3017,7 @@ static bool IsReductionAllowedForType(
}
},
},
- definedOp.u);
+ definedOp->u);
return ok;
}
@@ -3035,8 +3050,9 @@ void OmpStructureChecker::CheckReductionTypeList(
}
void OmpStructureChecker::CheckReductionModifier(
- const ReductionModifier &modifier) {
- if (modifier == ReductionModifier::Default) {
+ const parser::OmpReductionModifier &modifier) {
+ using ReductionModifier = parser::OmpReductionModifier;
+ if (modifier.v == ReductionModifier::Value::Default) {
// The default one is always ok.
return;
}
@@ -3049,7 +3065,7 @@ void OmpStructureChecker::CheckReductionModifier(
context_.Say(GetContext().clauseSource,
"REDUCTION modifier on LOOP directive must be DEFAULT"_err_en_US);
}
- if (modifier == ReductionModifier::Task) {
+ if (modifier.v == ReductionModifier::Value::Task) {
// "Task" is only allowed on...
[truncated]
|
@llvm/pr-subscribers-flang-fir-hlfir Author: Krzysztof Parzyszek (kparzysz) ChangesAlso, define helper macros in parse-tree.h. Apply the new modifier representation to the DEFAULTMAP and REDUCTION clauses, with testcases utilizing the new modifier validation. OpenMP modifier overhaul: #3/3 Patch is 37.74 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/116658.diff 15 Files Affected:
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index df5bf1d8d3200e..9c59ce520a31aa 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -509,9 +509,11 @@ class ParseTreeDumper {
NODE(parser, OmpDeclareMapperSpecifier)
NODE(parser, OmpDefaultClause)
NODE_ENUM(OmpDefaultClause, Type)
+ NODE(parser, OmpVariableCategory)
+ NODE_ENUM(OmpVariableCategory, Value)
NODE(parser, OmpDefaultmapClause)
NODE_ENUM(OmpDefaultmapClause, ImplicitBehavior)
- NODE_ENUM(OmpDefaultmapClause, VariableCategory)
+ NODE(OmpDefaultmapClause, Modifier)
NODE(parser, OmpDependenceType)
NODE_ENUM(OmpDependenceType, Value)
NODE(parser, OmpTaskDependenceType)
@@ -567,8 +569,10 @@ class ParseTreeDumper {
NODE_ENUM(OmpBindClause, Type)
NODE(parser, OmpProcBindClause)
NODE_ENUM(OmpProcBindClause, Type)
- NODE_ENUM(OmpReductionClause, ReductionModifier)
+ NODE(parser, OmpReductionModifier)
+ NODE_ENUM(OmpReductionModifier, Value)
NODE(parser, OmpReductionClause)
+ NODE(OmpReductionClause, Modifier)
NODE(parser, OmpInReductionClause)
NODE(parser, OmpReductionCombiner)
NODE(OmpReductionCombiner, FunctionCombiner)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index ef49a36578270e..5b28bcd4e21b80 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3440,6 +3440,16 @@ struct OmpObject {
WRAPPER_CLASS(OmpObjectList, std::list<OmpObject>);
+#define MODIFIER_BOILERPLATE(...) \
+ struct Modifier { \
+ using Variant = std::variant<__VA_ARGS__>; \
+ UNION_CLASS_BOILERPLATE(Modifier); \
+ CharBlock source; \
+ Variant u; \
+ }
+
+#define MODIFIERS() std::optional<std::list<Modifier>>
+
inline namespace modifier {
// For uniformity, in all keyword modifiers the name of the type defined
// by ENUM_CLASS is "Value", e.g.
@@ -3505,12 +3515,20 @@ struct OmpLinearModifier {
// - | // since 4.5, until 5.2
// + | * | .AND. | .OR. | .EQV. | .NEQV. | // since 4.5
// MIN | MAX | IAND | IOR | IEOR // since 4.5
-//
struct OmpReductionIdentifier {
UNION_CLASS_BOILERPLATE(OmpReductionIdentifier);
std::variant<DefinedOperator, ProcedureDesignator> u;
};
+// Ref: [5.0:300-302], [5.1:332-334], [5.2:134-137]
+//
+// reduction-modifier ->
+// DEFAULT | INSCAN | TASK // since 5.0
+struct OmpReductionModifier {
+ ENUM_CLASS(Value, Default, Inscan, Task);
+ WRAPPER_CLASS_BOILERPLATE(OmpReductionModifier, Value);
+};
+
// 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
@@ -3521,6 +3539,17 @@ struct OmpTaskDependenceType {
ENUM_CLASS(Value, In, Out, Inout, Inoutset, Mutexinoutset, Depobj)
WRAPPER_CLASS_BOILERPLATE(OmpTaskDependenceType, Value);
};
+
+// Ref: [4.5:229-230], [5.0:324-325], [5.1:357-358], [5.2:161-162]
+//
+// variable-category ->
+// SCALAR | // since 4.5
+// AGGREGATE | ALLOCATABLE | POINTER | // since 5.0
+// ALL // since 5.2
+struct OmpVariableCategory {
+ ENUM_CLASS(Value, Aggregate, All, Allocatable, Pointer, Scalar)
+ WRAPPER_CLASS_BOILERPLATE(OmpVariableCategory, Value);
+};
} // namespace modifier
// --- Clauses
@@ -3578,8 +3607,8 @@ struct OmpDefaultmapClause {
TUPLE_CLASS_BOILERPLATE(OmpDefaultmapClause);
ENUM_CLASS(
ImplicitBehavior, Alloc, To, From, Tofrom, Firstprivate, None, Default)
- ENUM_CLASS(VariableCategory, All, Scalar, Aggregate, Allocatable, Pointer)
- std::tuple<ImplicitBehavior, std::optional<VariableCategory>> t;
+ MODIFIER_BOILERPLATE(OmpVariableCategory);
+ std::tuple<ImplicitBehavior, MODIFIERS()> t;
};
// 2.13.9 iteration-offset -> +/- non-negative-constant
@@ -3771,14 +3800,16 @@ struct OmpProcBindClause {
WRAPPER_CLASS_BOILERPLATE(OmpProcBindClause, Type);
};
-// 2.15.3.6 reduction-clause -> REDUCTION (reduction-identifier:
-// variable-name-list)
+// Ref: [4.5:201-207], [5.0:300-302], [5.1:332-334], [5.2:134-137]
+//
+// reduction-clause ->
+// REDUCTION(reduction-identifier: list) | // since 4.5
+// REDUCTION([reduction-modifier,]
+// reduction-identifier: list) // since 5.0
struct OmpReductionClause {
TUPLE_CLASS_BOILERPLATE(OmpReductionClause);
- ENUM_CLASS(ReductionModifier, Inscan, Task, Default)
- std::tuple<std::optional<ReductionModifier>, OmpReductionIdentifier,
- OmpObjectList>
- t;
+ MODIFIER_BOILERPLATE(OmpReductionModifier, OmpReductionIdentifier);
+ std::tuple<MODIFIERS(), OmpObjectList> t;
};
// 2.7.1 sched-modifier -> MONOTONIC | NONMONOTONIC | SIMD
diff --git a/flang/include/flang/Semantics/openmp-modifiers.h b/flang/include/flang/Semantics/openmp-modifiers.h
index 6be582761ed687..28fec7314cd8b5 100644
--- a/flang/include/flang/Semantics/openmp-modifiers.h
+++ b/flang/include/flang/Semantics/openmp-modifiers.h
@@ -70,7 +70,11 @@ const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpLinearModifier>();
template <>
const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpReductionIdentifier>();
template <>
+const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpReductionModifier>();
+template <>
const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpTaskDependenceType>();
+template <>
+const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpVariableCategory>();
// Explanation of terminology:
//
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index 5e514b4ba9f0da..3e0a11bc785e33 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -12,6 +12,7 @@
#include "flang/Evaluate/expression.h"
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/expression.h"
+#include "flang/Semantics/openmp-modifiers.h"
#include "flang/Semantics/symbol.h"
#include "llvm/Frontend/OpenMP/OMPConstants.h"
@@ -571,7 +572,8 @@ Defaultmap make(const parser::OmpClause::Defaultmap &inp,
);
CLAUSET_ENUM_CONVERT( //
- convert2, wrapped::VariableCategory, Defaultmap::VariableCategory,
+ convert2, parser::OmpVariableCategory::Value,
+ Defaultmap::VariableCategory,
// clang-format off
MS(Aggregate, Aggregate)
MS(All, All)
@@ -581,10 +583,11 @@ Defaultmap make(const parser::OmpClause::Defaultmap &inp,
// clang-format on
);
+ auto &mods{semantics::OmpGetModifiers(inp.v)};
auto &t0 = std::get<wrapped::ImplicitBehavior>(inp.v.t);
- auto &t1 = std::get<std::optional<wrapped::VariableCategory>>(inp.v.t);
+ auto *t1 = semantics::OmpGetUniqueModifier<parser::OmpVariableCategory>(mods);
- auto category = t1 ? convert2(*t1) : Defaultmap::VariableCategory::All;
+ auto category = t1 ? convert2(t1->v) : Defaultmap::VariableCategory::All;
return Defaultmap{{/*ImplicitBehavior=*/convert1(t0),
/*VariableCategory=*/category}};
}
@@ -1173,10 +1176,9 @@ ProcBind make(const parser::OmpClause::ProcBind &inp,
Reduction make(const parser::OmpClause::Reduction &inp,
semantics::SemanticsContext &semaCtx) {
// inp.v -> parser::OmpReductionClause
- using wrapped = parser::OmpReductionClause;
-
CLAUSET_ENUM_CONVERT( //
- convert, wrapped::ReductionModifier, Reduction::ReductionModifier,
+ convert, parser::OmpReductionModifier::Value,
+ Reduction::ReductionModifier,
// clang-format off
MS(Inscan, Inscan)
MS(Task, Task)
@@ -1184,16 +1186,17 @@ Reduction make(const parser::OmpClause::Reduction &inp,
// clang-format on
);
- auto &t0 =
- std::get<std::optional<parser::OmpReductionClause::ReductionModifier>>(
- inp.v.t);
- auto &t1 = std::get<parser::OmpReductionIdentifier>(inp.v.t);
+ auto &mods = semantics::OmpGetModifiers(inp.v);
+ auto *t0 =
+ semantics::OmpGetUniqueModifier<parser::OmpReductionModifier>(mods);
+ auto *t1 =
+ semantics::OmpGetUniqueModifier<parser::OmpReductionIdentifier>(mods);
auto &t2 = std::get<parser::OmpObjectList>(inp.v.t);
return Reduction{
{/*ReductionModifier=*/t0
- ? std::make_optional<Reduction::ReductionModifier>(convert(*t0))
+ ? std::make_optional<Reduction::ReductionModifier>(convert(t0->v))
: std::nullopt,
- /*ReductionIdentifiers=*/{makeReductionOperator(t1, semaCtx)},
+ /*ReductionIdentifiers=*/{makeReductionOperator(*t1, semaCtx)},
/*List=*/makeObjects(t2, semaCtx)}};
}
@@ -1314,10 +1317,12 @@ Permutation make(const parser::OmpClause::Permutation &inp,
TaskReduction make(const parser::OmpClause::TaskReduction &inp,
semantics::SemanticsContext &semaCtx) {
// inp.v -> parser::OmpReductionClause
- auto &t0 = std::get<parser::OmpReductionIdentifier>(inp.v.t);
+ auto &mods = semantics::OmpGetModifiers(inp.v);
+ auto *t0 =
+ semantics::OmpGetUniqueModifier<parser::OmpReductionIdentifier>(mods);
auto &t1 = std::get<parser::OmpObjectList>(inp.v.t);
return TaskReduction{
- {/*ReductionIdentifiers=*/{makeReductionOperator(t0, semaCtx)},
+ {/*ReductionIdentifiers=*/{makeReductionOperator(*t0, semaCtx)},
/*List=*/makeObjects(t1, semaCtx)}};
}
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index b4d45873abb3ec..063201fc86ca41 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -229,6 +229,11 @@ TYPE_PARSER(construct<OmpLinearModifier>( //
TYPE_PARSER(construct<OmpReductionIdentifier>(Parser<DefinedOperator>{}) ||
construct<OmpReductionIdentifier>(Parser<ProcedureDesignator>{}))
+TYPE_PARSER(construct<OmpReductionModifier>(
+ "INSCAN" >> pure(OmpReductionModifier::Value::Inscan) ||
+ "TASK" >> pure(OmpReductionModifier::Value::Task) ||
+ "DEFAULT" >> pure(OmpReductionModifier::Value::Default)))
+
TYPE_PARSER(construct<OmpTaskDependenceType>(
"DEPOBJ" >> pure(OmpTaskDependenceType::Value::Depobj) ||
"IN"_id >> pure(OmpTaskDependenceType::Value::In) ||
@@ -237,6 +242,22 @@ TYPE_PARSER(construct<OmpTaskDependenceType>(
"MUTEXINOUTSET" >> pure(OmpTaskDependenceType::Value::Mutexinoutset) ||
"OUT" >> pure(OmpTaskDependenceType::Value::Out)))
+// This could be auto-generated.
+TYPE_PARSER(sourced(construct<OmpReductionClause::Modifier>(sourced(
+ construct<OmpReductionClause::Modifier>(Parser<OmpReductionModifier>{}) ||
+ construct<OmpReductionClause::Modifier>(
+ Parser<OmpReductionIdentifier>{})))))
+
+TYPE_PARSER(construct<OmpVariableCategory>(
+ "AGGREGATE" >> pure(OmpVariableCategory::Value::Aggregate) ||
+ "ALL"_id >> pure(OmpVariableCategory::Value::All) ||
+ "ALLOCATABLE" >> pure(OmpVariableCategory::Value::Allocatable) ||
+ "POINTER" >> pure(OmpVariableCategory::Value::Pointer) ||
+ "SCALAR" >> pure(OmpVariableCategory::Value::Scalar)))
+
+TYPE_PARSER(sourced(construct<OmpDefaultmapClause::Modifier>(
+ Parser<OmpVariableCategory>{})))
+
// --- Parsers for clauses --------------------------------------------
// [5.0] 2.10.1 affinity([aff-modifier:] locator-list)
@@ -309,16 +330,7 @@ TYPE_PARSER(construct<OmpDefaultmapClause>(
pure(OmpDefaultmapClause::ImplicitBehavior::Firstprivate) ||
"NONE" >> pure(OmpDefaultmapClause::ImplicitBehavior::None) ||
"DEFAULT" >> pure(OmpDefaultmapClause::ImplicitBehavior::Default)),
- maybe(":" >>
- construct<OmpDefaultmapClause::VariableCategory>(
- "ALL"_id >> pure(OmpDefaultmapClause::VariableCategory::All) ||
- "SCALAR" >> pure(OmpDefaultmapClause::VariableCategory::Scalar) ||
- "AGGREGATE" >>
- pure(OmpDefaultmapClause::VariableCategory::Aggregate) ||
- "ALLOCATABLE" >>
- pure(OmpDefaultmapClause::VariableCategory::Allocatable) ||
- "POINTER" >>
- pure(OmpDefaultmapClause::VariableCategory::Pointer)))))
+ maybe(":" >> nonemptyList(Parser<OmpDefaultmapClause::Modifier>{}))))
// 2.7.1 SCHEDULE ([modifier1 [, modifier2]:]kind[, chunk_size])
// Modifier -> MONITONIC | NONMONOTONIC | SIMD
@@ -375,12 +387,8 @@ TYPE_PARSER(construct<OmpIfClause>(
scalarLogicalExpr))
TYPE_PARSER(construct<OmpReductionClause>(
- maybe(
- ("INSCAN" >> pure(OmpReductionClause::ReductionModifier::Inscan) ||
- "TASK" >> pure(OmpReductionClause::ReductionModifier::Task) ||
- "DEFAULT" >> pure(OmpReductionClause::ReductionModifier::Default)) /
- ","),
- Parser<OmpReductionIdentifier>{} / ":", Parser<OmpObjectList>{}))
+ maybe(nonemptyList(Parser<OmpReductionClause::Modifier>{}) / ":"),
+ Parser<OmpObjectList>{}))
// OMP 5.0 2.19.5.6 IN_REDUCTION (reduction-identifier: variable-name-list)
TYPE_PARSER(construct<OmpInReductionClause>(
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index b2a0256cb58d0c..e06f4dea80f488 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2179,10 +2179,8 @@ class UnparseVisitor {
Walk(":", x.step);
}
void Unparse(const OmpReductionClause &x) {
- Walk(std::get<std::optional<OmpReductionClause::ReductionModifier>>(x.t),
- ",");
- Walk(std::get<OmpReductionIdentifier>(x.t));
- Put(":");
+ using Modifier = OmpReductionClause::Modifier;
+ Walk(std::get<std::optional<std::list<Modifier>>>(x.t), ":");
Walk(std::get<OmpObjectList>(x.t));
}
void Unparse(const OmpDetachClause &x) { Walk(x.v); }
@@ -2246,9 +2244,9 @@ class UnparseVisitor {
Walk(std::get<OmpObjectList>(x.t));
}
void Unparse(const OmpDefaultmapClause &x) {
+ using Modifier = OmpDefaultmapClause::Modifier;
Walk(std::get<OmpDefaultmapClause::ImplicitBehavior>(x.t));
- Walk(":",
- std::get<std::optional<OmpDefaultmapClause::VariableCategory>>(x.t));
+ Walk(":", std::get<std::optional<std::list<Modifier>>>(x.t));
}
void Unparse(const OmpToClause &x) {
auto &expect{
@@ -2896,7 +2894,7 @@ class UnparseVisitor {
WALK_NESTED_ENUM(OmpProcBindClause, Type) // OMP PROC_BIND
WALK_NESTED_ENUM(OmpDefaultClause, Type) // OMP DEFAULT
WALK_NESTED_ENUM(OmpDefaultmapClause, ImplicitBehavior) // OMP DEFAULTMAP
- WALK_NESTED_ENUM(OmpDefaultmapClause, VariableCategory) // OMP DEFAULTMAP
+ WALK_NESTED_ENUM(OmpVariableCategory, Value) // OMP variable-category
WALK_NESTED_ENUM(
OmpLastprivateClause, LastprivateModifier) // OMP lastprivate-modifier
WALK_NESTED_ENUM(OmpScheduleModifierType, ModType) // OMP schedule-modifier
@@ -2905,8 +2903,7 @@ class UnparseVisitor {
WALK_NESTED_ENUM(OmpScheduleClause, ScheduleType) // OMP schedule-type
WALK_NESTED_ENUM(OmpDeviceClause, DeviceModifier) // OMP device modifier
WALK_NESTED_ENUM(OmpDeviceTypeClause, Type) // OMP DEVICE_TYPE
- WALK_NESTED_ENUM(
- OmpReductionClause, ReductionModifier) // OMP reduction-modifier
+ WALK_NESTED_ENUM(OmpReductionModifier, Value) // OMP reduction-modifier
WALK_NESTED_ENUM(OmpFromClause, Expectation) // OMP motion-expectation
WALK_NESTED_ENUM(OmpIfClause, DirectiveNameModifier) // OMP directive-modifier
WALK_NESTED_ENUM(OmpCancelType, Type) // OMP cancel-type
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 9cac652216fcf2..079d0fd17bfac1 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -11,6 +11,7 @@
#include "flang/Evaluate/check-expression.h"
#include "flang/Parser/parse-tree.h"
#include "flang/Semantics/expression.h"
+#include "flang/Semantics/openmp-modifiers.h"
#include "flang/Semantics/tools.h"
#include <variant>
@@ -195,7 +196,7 @@ bool OmpStructureChecker::CheckAllowedClause(llvmOmpClause clause) {
if (!llvm::omp::isAllowedClauseForDirective(dir, clause, version)) {
unsigned allowedInVersion{[&] {
- for (unsigned v : {45, 50, 51, 52, 60}) {
+ for (unsigned v : llvm::omp::getOpenMPVersions()) {
if (v <= version) {
continue;
}
@@ -979,12 +980,14 @@ void OmpStructureChecker::Leave(const parser::OpenMPLoopConstruct &x) {
// constructs inside LOOP may add the relevant information. Scan reduction is
// supported only in loop constructs, so same checks are not applicable to
// other directives.
+ using ReductionModifier = parser::OmpReductionModifier;
for (const auto &clause : clauseList.v) {
if (const auto *reductionClause{
std::get_if<parser::OmpClause::Reduction>(&clause.u)}) {
- const auto &maybeModifier{
- std::get<std::optional<ReductionModifier>>(reductionClause->v.t)};
- if (maybeModifier && *maybeModifier == ReductionModifier::Inscan) {
+ auto &modifiers{OmpGetModifiers(reductionClause->v)};
+ auto *maybeModifier{OmpGetUniqueModifier<ReductionModifier>(modifiers)};
+ if (maybeModifier &&
+ maybeModifier->v == ReductionModifier::Value::Inscan) {
const auto &objectList{
std::get<parser::OmpObjectList>(reductionClause->v.t)};
auto checkReductionSymbolInScan = [&](const parser::Name *name) {
@@ -2850,20 +2853,27 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Destroy &x) {
void OmpStructureChecker::Enter(const parser::OmpClause::Reduction &x) {
CheckAllowedClause(llvm::omp::Clause::OMPC_reduction);
- if (CheckReductionOperators(x)) {
- CheckReductionTypeList(x);
- }
- if (const auto &maybeModifier{
- std::get<std::optional<ReductionModifier>>(x.v.t)}) {
- const ReductionModifier modifier{*maybeModifier};
- CheckReductionModifier(modifier);
+ if (OmpVerifyModifiers(x.v, GetContext().clauseSource, context_)) {
+ if (CheckReductionOperators(x)) {
+ CheckReductionTypeList(x);
+ }
+ auto &modifiers{OmpGetModifiers(x.v)};
+ using ReductionModifier = parser::OmpReductionModifier;
+ if (auto *maybeModifier{
+ OmpGetUniqueModifier<ReductionModifier>(modifiers)}) {
+ CheckReductionModifier(*maybeModifier);
+ }
}
}
bool OmpStructureChecker::CheckReductionOperators(
const parser::OmpClause::Reduction &x) {
-
- const auto &definedOp{std::get<parser::OmpReductionIdentifier>(x.v.t)};
+ auto &modifiers{OmpGetModifiers(x.v)};
+ const auto *definedOp{
+ OmpGetUniqueModifier<parser::OmpReductionIdentifier>(modifiers)};
+ if (!definedOp) {
+ return false;
+ }
bool ok = false;
common::visit(
common::visitors{
@@ -2896,7 +2906,7 @@ bool OmpStructureChecker::CheckReductionOperators(
}
},
},
- definedOp.u);
+ definedOp->u);
return ok;
}
@@ -2928,7 +2938,12 @@ bool OmpStructureChecker::CheckIntrinsicOperator(
static bool IsReductionAllowedForType(
const parser::OmpClause::Reduction &x, const DeclTypeSpec &type) {
- const auto &definedOp{std::get<parser::OmpReductionIdentifier>(x.v.t)};
+ auto &modifiers{OmpGetModifiers(x.v)};
+ const auto *definedOp{
+ OmpGetUniqueModifier<parser::OmpReductionIdentifier>(modifiers)};
+ if (!definedOp) {
+ return false;
+ }
// TODO: user defined reduction operators. Just allow everything for now.
bool ok{true};
@@ -3002,7 +3017,7 @@ static bool IsReductionAllowedForType(
}
},
},
- definedOp.u);
+ definedOp->u);
return ok;
}
@@ -3035,8 +3050,9 @@ void OmpStructureChecker::CheckReductionTypeList(
}
void OmpStructureChecker::CheckReductionModifier(
- const ReductionModifier &modifier) {
- if (modifier == ReductionModifier::Default) {
+ const parser::OmpReductionModifier &modifier) {
+ using ReductionModifier = parser::OmpReductionModifier;
+ if (modifier.v == ReductionModifier::Value::Default) {
// The default one is always ok.
return;
}
@@ -3049,7 +3065,7 @@ void OmpStructureChecker::CheckReductionModifier(
context_.Say(GetContext().clauseSource,
"REDUCTION modifier on LOOP directive must be DEFAULT"_err_en_US);
}
- if (modifier == ReductionModifier::Task) {
+ if (modifier.v == ReductionModifier::Value::Task) {
// "Task" is only allowed on...
[truncated]
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
…parzysz/spr/m02-openmp-descriptors
…parzysz/spr/m03-semantic-checks
…parzysz/spr/m02-openmp-descriptors
…parzysz/spr/m03-semantic-checks
@@ -33,7 +33,7 @@ program main | |||
enddo | |||
!$omp end target parallel | |||
|
|||
!ERROR: The DEFAULTMAP clause requires a variable-category SCALAR in OpenMP v1.1, try -fopenmp-version=50 | |||
!ERROR: A variable-category modifier is required |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm worried that the new error messages are less informative than the old ones
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They can be improved. Right now OmpVerifyModifiers
checks all properties, and returns a single value: "everything passed" or "something is wrong". Based on the descriptor information, it could generate the "try different version" if applicable, plus it could return a more detailed information about what failed to the caller.
On the positive side, with the new functions, the actual modifier is highlighted (except if it's required but missing).
If you're ok with that, I can improve this in a subsequent PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improving in a subsequent PR is okay with me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tentatively +1, but if you can please wait for Kiran to take a look as well.
@kiranchandramohan: I'll wait for your feedback. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
auto &modifiers{OmpGetModifiers(x.v)}; | ||
if (!modifiers) { | ||
return false; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Generally frontend code is written in a nested fashion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll update that in a separate PR.
const auto *definedOp{ | ||
OmpGetUniqueModifier<parser::OmpReductionIdentifier>(modifiers)}; | ||
if (!definedOp) { | ||
return false; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: Generally frontend code is nested. Below also.
const auto &objList{std::get<parser::OmpObjectList>(x.v.t)}; | ||
ResolveOmpObjectList(objList, Symbol::Flag::OmpReduction); | ||
|
||
auto &modifiers{OmpGetModifiers(x.v)}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It took me a bit to process that reduction identifiers are modifiers also. Was that established in OpenMP 5.2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In explicit terms, yes. Before 5.2 it was a de facto modifier by the virtue of being on the left side of a :
, e.g. reduction(+: x, y, z)
. In 5.2 all such things are modifiers (there are also a couple of post-modifiers, i.e. following a :
).
Frontend code is generally nested. Follow-up to llvm#116658.
Frontend code is generally nested. Follow-up to #116658.
Also, define helper macros in parse-tree.h.
Apply the new modifier representation to the DEFAULTMAP and REDUCTION clauses, with testcases utilizing the new modifier validation.
OpenMP modifier overhaul: #3/3