[flang][OpenMP] Apply modifier representation to semantic checks (#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
2026-01-20 01:58:44 +08:00 · 2024-11-21 13:18:01 -06:00
parent 29afbd5893
commit 4fc1141e76
18 changed files with 240 additions and 133 deletions
--- a/flang/examples/FeatureList/FeatureList.cpp
+++ b/flang/examples/FeatureList/FeatureList.cpp
@@ -468,7 +468,7 @@ public:
  READ_FEATURE(OmpDefaultClause::Type)
  READ_FEATURE(OmpDefaultmapClause)
  READ_FEATURE(OmpDefaultmapClause::ImplicitBehavior)
-  READ_FEATURE(OmpDefaultmapClause::VariableCategory)
+  READ_FEATURE(OmpVariableCategory::Value)
  READ_FEATURE(OmpDependClause)
  READ_FEATURE(OmpDependClause::TaskDep)
  READ_FEATURE(OmpDoacross::Sink)
--- a/flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp
+++ b/flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp
@@ -208,10 +208,9 @@ void OpenMPCounterVisitor::Post(
      "implicit_behavior=" + std::string{OmpDefaultmapClause::EnumToString(c)} +
      ";";
 }
-void OpenMPCounterVisitor::Post(
-    const OmpDefaultmapClause::VariableCategory &c) {
+void OpenMPCounterVisitor::Post(const OmpVariableCategory::Value &c) {
  clauseDetails +=
-      "variable_category=" + std::string{OmpDefaultmapClause::EnumToString(c)} +
+      "variable_category=" + std::string{OmpVariableCategory::EnumToString(c)} +
      ";";
 }
 void OpenMPCounterVisitor::Post(const OmpScheduleModifierType::ModType &c) {
--- a/flang/examples/FlangOmpReport/FlangOmpReportVisitor.h
+++ b/flang/examples/FlangOmpReport/FlangOmpReportVisitor.h
@@ -69,7 +69,7 @@ struct OpenMPCounterVisitor {
  void Post(const OmpProcBindClause::Type &c);
  void Post(const OmpDefaultClause::Type &c);
  void Post(const OmpDefaultmapClause::ImplicitBehavior &c);
-  void Post(const OmpDefaultmapClause::VariableCategory &c);
+  void Post(const OmpVariableCategory::Value &c);
  void Post(const OmpDeviceTypeClause::Type &c);
  void Post(const OmpScheduleModifierType::ModType &c);
  void Post(const OmpLinearModifier::Value &c);
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -509,9 +509,11 @@ public:
  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)
@@ -568,8 +570,10 @@ public:
  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)
--- 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
@@ -3775,14 +3804,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
--- 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:
 //
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -13,6 +13,7 @@
 #include "flang/Optimizer/Builder/Todo.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"
@@ -572,7 +573,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)
@@ -582,10 +584,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}};
 }
@@ -1178,10 +1181,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)
@@ -1189,16 +1191,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)}};
 }

@@ -1319,10 +1322,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)}};
 }

--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -228,6 +228,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) ||
@@ -236,6 +241,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)
@@ -313,16 +334,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
@@ -379,12 +391,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>(
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2189,10 +2189,8 @@ public:
    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); }
@@ -2256,9 +2254,9 @@ public:
    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{
@@ -2906,7 +2904,7 @@ public:
  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
@@ -2915,8 +2913,7 @@ public:
  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
--- 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 worksharing or "parallel" directive.
    static llvm::omp::Directive worksharing[]{
        llvm::omp::Directive::OMPD_do, llvm::omp::Directive::OMPD_scope,
@@ -3065,7 +3081,7 @@ void OmpStructureChecker::CheckReductionModifier(
          "Modifier 'TASK' on REDUCTION clause is only allowed with "
          "PARALLEL or worksharing directive"_err_en_US);
    }
-  } else if (modifier == ReductionModifier::Inscan) {
+  } else if (modifier.v == ReductionModifier::Value::Inscan) {
    // "Inscan" is only allowed on worksharing-loop, worksharing-loop simd,
    // or "simd" directive.
    // The worksharing-loop directives are OMPD_do and OMPD_for. Only the
@@ -3382,28 +3398,27 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Defaultmap &x) {
          ThisVersion(version), TryVersion(50));
    }
  }
-  using VariableCategory = parser::OmpDefaultmapClause::VariableCategory;
-  auto maybeCategory{std::get<std::optional<VariableCategory>>(x.v.t)};
-  if (!maybeCategory) {
-    if (version <= 45) {
-      context_.Say(GetContext().clauseSource,
-          "The DEFAULTMAP clause requires a variable-category SCALAR in %s, %s"_warn_en_US,
-          ThisVersion(version), TryVersion(50));
-    }
-  } else {
-    VariableCategory category{*maybeCategory};
+  if (!OmpVerifyModifiers(x.v, GetContext().clauseSource, context_)) {
+    // If modifier verification fails, return early.
+    return;
+  }
+  auto &modifiers{OmpGetModifiers(x.v)};
+  auto *maybeCategory{
+      OmpGetUniqueModifier<parser::OmpVariableCategory>(modifiers)};
+  if (maybeCategory) {
+    using VariableCategory = parser::OmpVariableCategory;
+    VariableCategory::Value category{maybeCategory->v};
    unsigned tryVersion{0};
-    if (version <= 45 && category != VariableCategory::Scalar) {
+    if (version <= 45 && category != VariableCategory::Value::Scalar) {
      tryVersion = 50;
    }
-    if (version < 52 && category == VariableCategory::All) {
+    if (version < 52 && category == VariableCategory::Value::All) {
      tryVersion = 52;
    }
    if (tryVersion) {
      context_.Say(GetContext().clauseSource,
          "%s is not allowed in %s, %s"_warn_en_US,
-          parser::ToUpperCaseLetters(
-              parser::OmpDefaultmapClause::EnumToString(category)),
+          parser::ToUpperCaseLetters(VariableCategory::EnumToString(category)),
          ThisVersion(version), TryVersion(tryVersion));
    }
  }
--- a/flang/lib/Semantics/check-omp-structure.h
+++ b/flang/lib/Semantics/check-omp-structure.h
@@ -70,7 +70,6 @@ public:
        ) {
  }
  using llvmOmpClause = const llvm::omp::Clause;
-  using ReductionModifier = parser::OmpReductionClause::ReductionModifier;

  void Enter(const parser::OpenMPConstruct &);
  void Leave(const parser::OpenMPConstruct &);
@@ -227,7 +226,7 @@ private:
  bool CheckIntrinsicOperator(
      const parser::DefinedOperator::IntrinsicOperator &);
  void CheckReductionTypeList(const parser::OmpClause::Reduction &);
-  void CheckReductionModifier(const ReductionModifier &);
+  void CheckReductionModifier(const parser::OmpReductionModifier &);
  void CheckMasterNesting(const parser::OpenMPBlockConstruct &x);
  void ChecksOnOrderedAsBlock();
  void CheckBarrierNesting(const parser::OpenMPSimpleStandaloneConstruct &x);
--- a/flang/lib/Semantics/openmp-modifiers.cpp
+++ b/flang/lib/Semantics/openmp-modifiers.cpp
@@ -128,6 +128,22 @@ OmpGetDescriptor<parser::OmpReductionIdentifier>() {
  return desc;
 }

+template <>
+const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpReductionModifier>() {
+  static const OmpModifierDescriptor desc{
+      /*name=*/"reduction-modifier",
+      /*props=*/
+      {
+          {45, {OmpProperty::Unique}},
+      },
+      /*clauses=*/
+      {
+          {45, {Clause::OMPC_reduction}},
+      },
+  };
+  return desc;
+}
+
 template <>
 const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpTaskDependenceType>() {
  static const OmpModifierDescriptor desc{
@@ -143,4 +159,21 @@ const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpTaskDependenceType>() {
  };
  return desc;
 }
+
+template <>
+const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpVariableCategory>() {
+  static const OmpModifierDescriptor desc{
+      /*name=*/"variable-category",
+      /*props=*/
+      {
+          {45, {OmpProperty::Required, OmpProperty::Unique}},
+          {50, {OmpProperty::Unique}},
+      },
+      /*clauses=*/
+      {
+          {45, {Clause::OMPC_defaultmap}},
+      },
+  };
+  return desc;
+}
 } // namespace Fortran::semantics
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -19,6 +19,7 @@
 #include "flang/Parser/parse-tree.h"
 #include "flang/Parser/tools.h"
 #include "flang/Semantics/expression.h"
+#include "flang/Semantics/openmp-modifiers.h"
 #include "flang/Semantics/symbol.h"
 #include "flang/Semantics/tools.h"
 #include <list>
@@ -518,8 +519,14 @@ public:
  }

  bool Pre(const parser::OmpClause::Reduction &x) {
-    const parser::OmpReductionIdentifier &opr{
-        std::get<parser::OmpReductionIdentifier>(x.v.t)};
+    const auto &objList{std::get<parser::OmpObjectList>(x.v.t)};
+    ResolveOmpObjectList(objList, Symbol::Flag::OmpReduction);
+
+    auto &modifiers{OmpGetModifiers(x.v)};
+    if (!modifiers) {
+      return false;
+    }
+
    auto createDummyProcSymbol = [&](const parser::Name *name) {
      // If name resolution failed, create a dummy symbol
      const auto namePair{
@@ -530,30 +537,35 @@ public:
      }
      name->symbol = &newSymbol;
    };
-    if (const auto *procD{parser::Unwrap<parser::ProcedureDesignator>(opr.u)}) {
-      if (const auto *name{parser::Unwrap<parser::Name>(procD->u)}) {
-        if (!name->symbol) {
-          if (!ResolveName(name)) {
-            createDummyProcSymbol(name);
+
+    for (auto &mod : *modifiers) {
+      if (!std::holds_alternative<parser::OmpReductionIdentifier>(mod.u)) {
+        continue;
+      }
+      auto &opr{std::get<parser::OmpReductionIdentifier>(mod.u)};
+      if (auto *procD{parser::Unwrap<parser::ProcedureDesignator>(opr.u)}) {
+        if (auto *name{parser::Unwrap<parser::Name>(procD->u)}) {
+          if (!name->symbol) {
+            if (!ResolveName(name)) {
+              createDummyProcSymbol(name);
+            }
          }
        }
-      }
-      if (const auto *procRef{
-              parser::Unwrap<parser::ProcComponentRef>(procD->u)}) {
-        if (!procRef->v.thing.component.symbol) {
-          if (!ResolveName(&procRef->v.thing.component)) {
-            createDummyProcSymbol(&procRef->v.thing.component);
+        if (auto *procRef{parser::Unwrap<parser::ProcComponentRef>(procD->u)}) {
+          if (!procRef->v.thing.component.symbol) {
+            if (!ResolveName(&procRef->v.thing.component)) {
+              createDummyProcSymbol(&procRef->v.thing.component);
+            }
          }
        }
      }
    }
-    const auto &objList{std::get<parser::OmpObjectList>(x.v.t)};
-    ResolveOmpObjectList(objList, Symbol::Flag::OmpReduction);
-    using ReductionModifier = parser::OmpReductionClause::ReductionModifier;
-    const auto &maybeModifier{
-        std::get<std::optional<ReductionModifier>>(x.v.t)};
-    if (maybeModifier && *maybeModifier == ReductionModifier::Inscan) {
-      ResolveOmpObjectList(objList, Symbol::Flag::OmpInScanReduction);
+    using ReductionModifier = parser::OmpReductionModifier;
+    if (auto *maybeModifier{
+            OmpGetUniqueModifier<ReductionModifier>(modifiers)}) {
+      if (maybeModifier->v == ReductionModifier::Value::Inscan) {
+        ResolveOmpObjectList(objList, Symbol::Flag::OmpInScanReduction);
+      }
    }
    return false;
  }
--- a/flang/test/Parser/OpenMP/defaultmap-clause.f90
+++ b/flang/test/Parser/OpenMP/defaultmap-clause.f90
@@ -31,7 +31,7 @@ end
 !PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE: | OmpClauseList -> OmpClause -> Defaultmap -> OmpDefaultmapClause
 !PARSE-TREE: | | ImplicitBehavior = Firstprivate
-!PARSE-TREE: | | VariableCategory = Aggregate
+!PARSE-TREE: | | Modifier -> OmpVariableCategory -> Value = Aggregate

 subroutine f02
  !$omp target defaultmap(alloc: all)
@@ -47,7 +47,7 @@ end
 !PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE: | OmpClauseList -> OmpClause -> Defaultmap -> OmpDefaultmapClause
 !PARSE-TREE: | | ImplicitBehavior = Alloc
-!PARSE-TREE: | | VariableCategory = All
+!PARSE-TREE: | | Modifier -> OmpVariableCategory -> Value = All

 ! Both "all" and "allocatable" are valid, and "all" is a prefix of
 ! "allocatable". Make sure we parse this correctly.
@@ -65,7 +65,7 @@ end
 !PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE: | OmpClauseList -> OmpClause -> Defaultmap -> OmpDefaultmapClause
 !PARSE-TREE: | | ImplicitBehavior = Alloc
-!PARSE-TREE: | | VariableCategory = Allocatable
+!PARSE-TREE: | | Modifier -> OmpVariableCategory -> Value = Allocatable

 subroutine f04
  !$omp target defaultmap(tofrom: scalar)
@@ -81,4 +81,4 @@ end
 !PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE: | OmpClauseList -> OmpClause -> Defaultmap -> OmpDefaultmapClause
 !PARSE-TREE: | | ImplicitBehavior = Tofrom
-!PARSE-TREE: | | VariableCategory = Scalar
+!PARSE-TREE: | | Modifier -> OmpVariableCategory -> Value  = Scalar
--- a/flang/test/Parser/OpenMP/defaultmap-unparse.f90
+++ b/flang/test/Parser/OpenMP/defaultmap-unparse.f90
@@ -38,7 +38,7 @@ program main
 !PARSE-TREE:        OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE:        OmpClauseList -> OmpClause -> Defaultmap -> OmpDefaultmapClause
 !PARSE-TREE:          ImplicitBehavior = Tofrom
-!PARSE-TREE:          VariableCategory = Scalar
+!PARSE-TREE:          Modifier -> OmpVariableCategory -> Value = Scalar

 !CHECK: !$omp target defaultmap(alloc:scalar)
  !$omp target defaultmap(alloc:scalar)
@@ -50,7 +50,7 @@ program main
 !PARSE-TREE:        OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE:        OmpClauseList -> OmpClause -> Defaultmap -> OmpDefaultmapClause
 !PARSE-TREE:          ImplicitBehavior = Alloc
-!PARSE-TREE:          VariableCategory = Scalar
+!PARSE-TREE:          Modifier -> OmpVariableCategory -> Value = Scalar

 !CHECK: !$omp target defaultmap(none)
  !$omp target defaultmap(none)
@@ -73,7 +73,7 @@ program main
 !PARSE-TREE:        OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE:        OmpClauseList -> OmpClause -> Defaultmap -> OmpDefaultmapClause
 !PARSE-TREE:          ImplicitBehavior = None
-!PARSE-TREE:          VariableCategory = Scalar
+!PARSE-TREE:          Modifier -> OmpVariableCategory -> Value = Scalar

 !CHECK: !$omp target defaultmap(to:scalar)
  !$omp target defaultmap(to:scalar)
@@ -85,7 +85,7 @@ program main
 !PARSE-TREE:        OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE:        OmpClauseList -> OmpClause -> Defaultmap -> OmpDefaultmapClause
 !PARSE-TREE:          ImplicitBehavior = To
-!PARSE-TREE:          VariableCategory = Scalar
+!PARSE-TREE:          Modifier -> OmpVariableCategory -> Value = Scalar

 !CHECK: !$omp target defaultmap(firstprivate:scalar)
  !$omp target defaultmap(firstprivate:scalar)
@@ -97,7 +97,7 @@ program main
 !PARSE-TREE:        OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE:        OmpClauseList -> OmpClause -> Defaultmap -> OmpDefaultmapClause
 !PARSE-TREE:          ImplicitBehavior = Firstprivate
-!PARSE-TREE:          VariableCategory = Scalar
+!PARSE-TREE:          Modifier -> OmpVariableCategory -> Value = Scalar
 
 !CHECK: !$omp target defaultmap(tofrom:aggregate)
  !$omp target defaultmap(tofrom:aggregate)
@@ -112,7 +112,7 @@ program main
 !PARSE-TREE:        OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE:        OmpClauseList -> OmpClause -> Defaultmap -> OmpDefaultmapClause
 !PARSE-TREE:          ImplicitBehavior = Tofrom
-!PARSE-TREE:          VariableCategory = Aggregate
+!PARSE-TREE:          Modifier -> OmpVariableCategory -> Value = Aggregate
 
 !CHECK: !$omp target defaultmap(tofrom:allocatable)  
  !$omp target defaultmap(tofrom:allocatable)
@@ -124,7 +124,7 @@ program main
 !PARSE-TREE:        OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE:        OmpClauseList -> OmpClause -> Defaultmap -> OmpDefaultmapClause
 !PARSE-TREE:          ImplicitBehavior = Tofrom
-!PARSE-TREE:          VariableCategory = Allocatable
+!PARSE-TREE:          Modifier -> OmpVariableCategory -> Value = Allocatable
 
 !CHECK: !$omp target defaultmap(default:pointer)
  !$omp target defaultmap(default:pointer)
@@ -138,7 +138,7 @@ program main
 !PARSE-TREE:        OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE:        OmpClauseList -> OmpClause -> Defaultmap -> OmpDefaultmapClause
 !PARSE-TREE:          ImplicitBehavior = Default
-!PARSE-TREE:          VariableCategory = Pointer
+!PARSE-TREE:          Modifier -> OmpVariableCategory -> Value = Pointer

 end program main
 !CHECK-LABEL: end program main
--- a/flang/test/Parser/OpenMP/reduction-modifier.f90
+++ b/flang/test/Parser/OpenMP/reduction-modifier.f90
@@ -4,13 +4,13 @@
 subroutine foo()
  integer :: i, j
  j = 0
-! CHECK: !$OMP DO  REDUCTION(TASK,*:j)
+! CHECK: !$OMP DO  REDUCTION(TASK, *:j)
 ! PARSE-TREE: | | ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPLoopConstruct
 ! PARSE-TREE: | | | OmpBeginLoopDirective
 ! PARSE-TREE: | | | | OmpLoopDirective -> llvm::omp::Directive = do
 ! PARSE-TREE: | | | | OmpClauseList -> OmpClause -> Reduction -> OmpReductionClause
-! PARSE-TREE: | | | | | ReductionModifier = Task
-! PARSE-TREE: | | | | | OmpReductionIdentifier -> DefinedOperator -> IntrinsicOperator = Multiply
+! PARSE-TREE: | | | | | Modifier -> OmpReductionModifier -> Value = Task
+! PARSE-TREE: | | | | | Modifier -> OmpReductionIdentifier -> DefinedOperator -> IntrinsicOperator = Multiply
 ! PARSE-TREE: | | | | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'j
  !$omp do reduction (task, *: j)
  do i = 1, 10
--- a/flang/test/Semantics/OpenMP/combined-constructs.f90
+++ b/flang/test/Semantics/OpenMP/combined-constructs.f90
@@ -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
  !$omp target parallel defaultmap(tofrom)
  do i = 1, N
     a(i) = 3.14
@@ -80,7 +80,7 @@ program main
  enddo
  !$omp end target parallel do

-  !ERROR: The DEFAULTMAP clause requires a variable-category SCALAR in OpenMP v1.1, try -fopenmp-version=50
+  !ERROR: A variable-category modifier is required
  !$omp target parallel do defaultmap(tofrom)
  do i = 1, N
     a(i) = 3.14
@@ -140,7 +140,7 @@ program main
  enddo
  !$omp end target teams

-  !ERROR: The DEFAULTMAP clause requires a variable-category SCALAR in OpenMP v1.1, try -fopenmp-version=50
+  !ERROR: A variable-category modifier is required
  !$omp target teams defaultmap(tofrom)
  do i = 1, N
     a(i) = 3.14
@@ -240,7 +240,7 @@ program main
  enddo
  !$omp end target teams distribute

-  !ERROR: The DEFAULTMAP clause requires a variable-category SCALAR in OpenMP v1.1, try -fopenmp-version=50
+  !ERROR: A variable-category modifier is required
  !$omp target teams distribute defaultmap(tofrom)
  do i = 1, N
     a(i) = 3.14
@@ -333,7 +333,7 @@ program main
  enddo
  !$omp end target teams distribute parallel do

-  !ERROR: The DEFAULTMAP clause requires a variable-category SCALAR in OpenMP v1.1, try -fopenmp-version=50
+  !ERROR: A variable-category modifier is required
  !$omp target teams distribute parallel do defaultmap(tofrom)
  do i = 1, N
     a(i) = 3.14
@@ -433,7 +433,7 @@ program main
  enddo
  !$omp end target teams distribute parallel do simd

-  !ERROR: The DEFAULTMAP clause requires a variable-category SCALAR in OpenMP v1.1, try -fopenmp-version=50
+  !ERROR: A variable-category modifier is required
  !$omp target teams distribute parallel do simd defaultmap(tofrom)
  do i = 1, N
     a(i) = 3.14
--- a/flang/test/Semantics/OpenMP/defaultmap-clause-v45.f90
+++ b/flang/test/Semantics/OpenMP/defaultmap-clause-v45.f90
@@ -1,7 +1,7 @@
 !RUN: %python %S/../test_errors.py %s %flang -fopenmp -fopenmp-version=45 -Werror

 subroutine f00
-!WARNING: The DEFAULTMAP clause requires a variable-category SCALAR in OpenMP v4.5, try -fopenmp-version=50
+!WARNING: A variable-category modifier is required
  !$omp target defaultmap(tofrom)
  !$omp end target
 end