[flang-commits] [flang] [llvm] [flang][OpenMP] Use new modifiers in IF/LASTPRIVATE (PR #118128)

Krzysztof Parzyszek via flang-commits flang-commits at lists.llvm.org
Mon Dec 2 13:47:18 PST 2024


https://github.com/kparzysz updated https://github.com/llvm/llvm-project/pull/118128

>From aa17e9dcf1a5d85cef029b4e1e67dc7d0d8895e7 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 25 Nov 2024 13:11:50 -0600
Subject: [PATCH 01/11] [flang][OpenMP] Use new modifiers in ALLOCATE clause

Again, this simplifies the semantic checks and lowering quite a bit.
Update the check for positive alignment to use a more informative
message, and to highlight the modifier itsef, not the whole clause.
Remove the checks for the allocator expression itself being positive:
there is nothing in the spec that says that it should be positive.

Remove the "simple" modifier from the AllocateT template, since both
simple and complex modifiers are the same thing, only differing in
syntax.
---
 flang/include/flang/Parser/dump-parse-tree.h  |  8 +--
 flang/include/flang/Parser/parse-tree.h       | 52 +++++++++++------
 .../flang/Semantics/openmp-modifiers.h        | 19 +++++++
 flang/lib/Lower/OpenMP/ClauseProcessor.cpp    |  6 +-
 flang/lib/Lower/OpenMP/Clauses.cpp            | 56 +++++++------------
 flang/lib/Lower/OpenMP/Clauses.h              |  5 +-
 flang/lib/Parser/openmp-parsers.cpp           | 45 +++++++--------
 flang/lib/Parser/unparse.cpp                  | 32 +++--------
 flang/lib/Semantics/check-omp-structure.cpp   | 48 +++++++---------
 flang/lib/Semantics/openmp-modifiers.cpp      | 50 +++++++++++++++++
 flang/lib/Semantics/resolve-directives.cpp    | 13 ++---
 .../test/Parser/OpenMP/allocators-unparse.f90 | 19 +++----
 .../Semantics/OpenMP/allocate-clause01.f90    | 10 +---
 flang/test/Semantics/OpenMP/allocators01.f90  |  2 +-
 flang/test/Semantics/OpenMP/allocators04.f90  |  2 +-
 flang/test/Semantics/OpenMP/allocators05.f90  |  2 +-
 flang/test/Semantics/OpenMP/allocators06.f90  |  2 +-
 flang/test/Semantics/OpenMP/resolve06.f90     |  2 +-
 llvm/include/llvm/Frontend/OpenMP/ClauseT.h   |  6 +-
 19 files changed, 205 insertions(+), 174 deletions(-)

diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 68f9406dc28309..d499b414827d50 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -586,10 +586,10 @@ class ParseTreeDumper {
   NODE(parser, OmpReductionInitializerClause)
   NODE(parser, OmpReductionIdentifier)
   NODE(parser, OmpAllocateClause)
-  NODE(OmpAllocateClause, AllocateModifier)
-  NODE(OmpAllocateClause::AllocateModifier, Allocator)
-  NODE(OmpAllocateClause::AllocateModifier, ComplexModifier)
-  NODE(OmpAllocateClause::AllocateModifier, Align)
+  NODE(OmpAllocateClause, Modifier)
+  NODE(parser, OmpAlignModifier)
+  NODE(parser, OmpAllocatorComplexModifier)
+  NODE(parser, OmpAllocatorSimpleModifier)
   NODE(parser, OmpScheduleClause)
   NODE(OmpScheduleClause, Modifier)
   NODE_ENUM(OmpScheduleClause, Kind)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 8d7119a56b7f8e..e9a02a87812452 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3457,6 +3457,30 @@ inline namespace modifier {
 //   ENUM_CLASS(Value, Keyword1, Keyword2);
 // };
 
+// Ref: [5.1:184-185], [5.2:178-179]
+//
+// align-modifier ->
+//    ALIGN(alignment)                              // since 5.1
+struct OmpAlignModifier {
+  WRAPPER_CLASS_BOILERPLATE(OmpAlignModifier, ScalarIntExpr);
+};
+
+// Ref: [5.0:158-159], [5.1:184-185], [5.2:178-179]
+//
+// allocator-simple-modifier ->
+//    allocator                                     // since 5.0
+struct OmpAllocatorSimpleModifier {
+  WRAPPER_CLASS_BOILERPLATE(OmpAllocatorSimpleModifier, ScalarIntExpr);
+};
+
+// Ref: [5.1:184-185], [5.2:178-179]
+//
+// allocator-complex-modifier ->
+//    ALLOCATOR(allocator)                          // since 5.1
+struct OmpAllocatorComplexModifier {
+  WRAPPER_CLASS_BOILERPLATE(OmpAllocatorComplexModifier, ScalarIntExpr);
+};
+
 // Ref: [5.2:252-254]
 //
 // chunk-modifier ->
@@ -3646,24 +3670,20 @@ struct OmpAlignedClause {
   std::tuple<OmpObjectList, std::optional<ScalarIntConstantExpr>> t;
 };
 
-// OMP 5.0 2.11.4 allocate-clause -> ALLOCATE ([allocator:] variable-name-list)
-// OMP 5.2 2.13.4 allocate-clause -> ALLOCATE ([allocate-modifier [,
-//  	                               allocate-modifier] :]
-//                                   variable-name-list)
-//                allocate-modifier -> allocator | align
+// Ref: [5.0:158-159], [5.1:184-185], [5.2:178-179]
+//
+// allocate-clause ->
+//    ALLOCATE(
+//        [allocator-simple-modifier:] list) |      // since 5.0
+//    ALLOCATE([modifier...:] list)                 // since 5.1
+// modifier ->
+//    allocator-simple-modifier |
+//    allocator-complex-modifier | align-modifier   // since 5.1
 struct OmpAllocateClause {
-  struct AllocateModifier {
-    WRAPPER_CLASS(Allocator, ScalarIntExpr);
-    WRAPPER_CLASS(Align, ScalarIntExpr);
-    struct ComplexModifier {
-      TUPLE_CLASS_BOILERPLATE(ComplexModifier);
-      std::tuple<Allocator, Align> t;
-    };
-    UNION_CLASS_BOILERPLATE(AllocateModifier);
-    std::variant<Allocator, ComplexModifier, Align> u;
-  };
+  MODIFIER_BOILERPLATE(OmpAlignModifier, OmpAllocatorSimpleModifier,
+      OmpAllocatorComplexModifier);
   TUPLE_CLASS_BOILERPLATE(OmpAllocateClause);
-  std::tuple<std::optional<AllocateModifier>, OmpObjectList> t;
+  std::tuple<MODIFIERS(), OmpObjectList> t;
 };
 
 // OMP 5.0 2.4 atomic-default-mem-order-clause ->
diff --git a/flang/include/flang/Semantics/openmp-modifiers.h b/flang/include/flang/Semantics/openmp-modifiers.h
index 60f116e6f00339..a6316cf7aba564 100644
--- a/flang/include/flang/Semantics/openmp-modifiers.h
+++ b/flang/include/flang/Semantics/openmp-modifiers.h
@@ -67,6 +67,9 @@ template <typename SpecificTy> const OmpModifierDescriptor &OmpGetDescriptor();
 #define DECLARE_DESCRIPTOR(name) \
   template <> const OmpModifierDescriptor &OmpGetDescriptor<name>()
 
+DECLARE_DESCRIPTOR(parser::OmpAlignModifier);
+DECLARE_DESCRIPTOR(parser::OmpAllocatorComplexModifier);
+DECLARE_DESCRIPTOR(parser::OmpAllocatorSimpleModifier);
 DECLARE_DESCRIPTOR(parser::OmpChunkModifier);
 DECLARE_DESCRIPTOR(parser::OmpDependenceType);
 DECLARE_DESCRIPTOR(parser::OmpExpectation);
@@ -216,10 +219,26 @@ OmpGetRepeatableModifier(const std::optional<std::list<UnionTy>> &modifiers) {
       OmpSpecificModifierIterator(items, items->end()));
 }
 
+// Attempt to prevent creating a range based on an expiring modifier list.
 template <typename SpecificTy, typename UnionTy>
 llvm::iterator_range<OmpSpecificModifierIterator<SpecificTy>>
 OmpGetRepeatableModifier(std::optional<std::list<UnionTy>> &&) = delete;
 
+template <typename SpecificTy, typename UnionTy>
+Fortran::parser::CharBlock OmpGetModifierSource(
+    const std::optional<std::list<UnionTy>> &modifiers,
+    const SpecificTy *specific) {
+  if (!modifiers || !specific) {
+    return Fortran::parser::CharBlock{};
+  }
+  for (auto &m : *modifiers) {
+    if (std::get_if<SpecificTy>(&m.u) == specific) {
+      return m.source;
+    }
+  }
+  llvm_unreachable("`specific` must be a member of `modifiers`");
+}
+
 namespace detail {
 template <typename T> constexpr const T *make_nullptr() {
   return static_cast<const T *>(nullptr);
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 6baa22a44eafb1..8670a5470cdc67 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -81,12 +81,8 @@ genAllocateClause(lower::AbstractConverter &converter,
   // Check if allocate clause has allocator specified. If so, add it
   // to list of allocators, otherwise, add default allocator to
   // list of allocators.
-  using SimpleModifier = Allocate::AllocatorSimpleModifier;
   using ComplexModifier = Allocate::AllocatorComplexModifier;
-  if (auto &mod = std::get<std::optional<SimpleModifier>>(clause.t)) {
-    mlir::Value operand = fir::getBase(converter.genExprValue(*mod, stmtCtx));
-    allocatorOperands.append(objects.size(), operand);
-  } else if (auto &mod = std::get<std::optional<ComplexModifier>>(clause.t)) {
+  if (auto &mod = std::get<std::optional<ComplexModifier>>(clause.t)) {
     mlir::Value operand = fir::getBase(converter.genExprValue(mod->v, stmtCtx));
     allocatorOperands.append(objects.size(), operand);
   } else {
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index bf20f42bdecaf1..ddc91ef2030bd7 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -415,47 +415,29 @@ Aligned make(const parser::OmpClause::Aligned &inp,
 Allocate make(const parser::OmpClause::Allocate &inp,
               semantics::SemanticsContext &semaCtx) {
   // inp.v -> parser::OmpAllocateClause
-  using wrapped = parser::OmpAllocateClause;
-  auto &t0 = std::get<std::optional<wrapped::AllocateModifier>>(inp.v.t);
+  auto &mods = semantics::OmpGetModifiers(inp.v);
+  auto *m0 = semantics::OmpGetUniqueModifier<parser::OmpAlignModifier>(mods);
+  auto *m1 =
+      semantics::OmpGetUniqueModifier<parser::OmpAllocatorComplexModifier>(
+          mods);
+  auto *m2 =
+      semantics::OmpGetUniqueModifier<parser::OmpAllocatorSimpleModifier>(mods);
   auto &t1 = std::get<parser::OmpObjectList>(inp.v.t);
 
-  if (!t0) {
-    return Allocate{{/*AllocatorSimpleModifier=*/std::nullopt,
-                     /*AllocatorComplexModifier=*/std::nullopt,
-                     /*AlignModifier=*/std::nullopt,
-                     /*List=*/makeObjects(t1, semaCtx)}};
-  }
+  auto makeAllocator = [&](auto *mod) -> std::optional<Allocator> {
+    if (mod)
+      return Allocator{makeExpr(mod->v, semaCtx)};
+    return std::nullopt;
+  };
 
-  using Tuple = decltype(Allocate::t);
+  auto makeAlign = [&](const parser::ScalarIntExpr &expr) {
+    return Align{makeExpr(expr, semaCtx)};
+  };
 
-  return Allocate{Fortran::common::visit(
-      common::visitors{
-          // simple-modifier
-          [&](const wrapped::AllocateModifier::Allocator &v) -> Tuple {
-            return {/*AllocatorSimpleModifier=*/makeExpr(v.v, semaCtx),
-                    /*AllocatorComplexModifier=*/std::nullopt,
-                    /*AlignModifier=*/std::nullopt,
-                    /*List=*/makeObjects(t1, semaCtx)};
-          },
-          // complex-modifier + align-modifier
-          [&](const wrapped::AllocateModifier::ComplexModifier &v) -> Tuple {
-            auto &s0 = std::get<wrapped::AllocateModifier::Allocator>(v.t);
-            auto &s1 = std::get<wrapped::AllocateModifier::Align>(v.t);
-            return {
-                /*AllocatorSimpleModifier=*/std::nullopt,
-                /*AllocatorComplexModifier=*/Allocator{makeExpr(s0.v, semaCtx)},
-                /*AlignModifier=*/Align{makeExpr(s1.v, semaCtx)},
-                /*List=*/makeObjects(t1, semaCtx)};
-          },
-          // align-modifier
-          [&](const wrapped::AllocateModifier::Align &v) -> Tuple {
-            return {/*AllocatorSimpleModifier=*/std::nullopt,
-                    /*AllocatorComplexModifier=*/std::nullopt,
-                    /*AlignModifier=*/Align{makeExpr(v.v, semaCtx)},
-                    /*List=*/makeObjects(t1, semaCtx)};
-          },
-      },
-      t0->u)};
+  auto maybeAllocator = m1 ? makeAllocator(m1) : makeAllocator(m2);
+  return Allocate{{/*AllocatorComplexModifier=*/std::move(maybeAllocator),
+                   /*AlignModifier=*/maybeApplyToV(makeAlign, m0),
+                   /*List=*/makeObjects(t1, semaCtx)}};
 }
 
 Allocator make(const parser::OmpClause::Allocator &inp,
diff --git a/flang/lib/Lower/OpenMP/Clauses.h b/flang/lib/Lower/OpenMP/Clauses.h
index 5fac5c2271c3bf..562685e43c13bf 100644
--- a/flang/lib/Lower/OpenMP/Clauses.h
+++ b/flang/lib/Lower/OpenMP/Clauses.h
@@ -153,10 +153,11 @@ std::optional<ResultTy> maybeApply(FuncTy &&func,
   return func(*arg);
 }
 
-template <
+template <           //
     typename FuncTy, //
     typename ArgTy,  //
-    typename ResultTy = std::invoke_result_t<FuncTy, typename ArgTy::Value>>
+    typename ResultTy =
+        std::invoke_result_t<FuncTy, decltype(std::declval<ArgTy>().v)>>
 std::optional<ResultTy> maybeApplyToV(FuncTy &&func, const ArgTy *arg) {
   if (!arg)
     return std::nullopt;
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 2040a3e7ed5aeb..f231290932c122 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -91,6 +91,14 @@ static TypeDeclarationStmt makeIterSpecDecl(std::list<ObjectName> &&names) {
 
 // --- Parsers for clause modifiers -----------------------------------
 
+TYPE_PARSER(construct<OmpAlignModifier>( //
+    "ALIGN" >> parenthesized(scalarIntExpr)))
+
+TYPE_PARSER(construct<OmpAllocatorComplexModifier>(
+    "ALLOCATOR" >> parenthesized(scalarIntExpr)))
+
+TYPE_PARSER(construct<OmpAllocatorSimpleModifier>(scalarIntExpr))
+
 TYPE_PARSER(construct<OmpChunkModifier>( //
     "SIMD" >> pure(OmpChunkModifier::Value::Simd)))
 
@@ -183,6 +191,16 @@ TYPE_PARSER(construct<OmpVariableCategory>(
     "SCALAR" >> pure(OmpVariableCategory::Value::Scalar)))
 
 // This could be auto-generated.
+TYPE_PARSER(sourced(construct<OmpAllocateClause::Modifier>(sourced(
+    construct<OmpAllocateClause::Modifier>(Parser<OmpAlignModifier>{}) ||
+    construct<OmpAllocateClause::Modifier>(
+        Parser<OmpAllocatorComplexModifier>{}) ||
+    construct<OmpAllocateClause::Modifier>(
+        Parser<OmpAllocatorSimpleModifier>{})))))
+
+TYPE_PARSER(sourced(
+    construct<OmpDefaultmapClause::Modifier>(Parser<OmpVariableCategory>{})))
+
 TYPE_PARSER(sourced(construct<OmpFromClause::Modifier>(
     sourced(construct<OmpFromClause::Modifier>(Parser<OmpExpectation>{}) ||
         construct<OmpFromClause::Modifier>(Parser<OmpMapper>{}) ||
@@ -211,9 +229,6 @@ TYPE_PARSER(sourced(construct<OmpToClause::Modifier>(
         construct<OmpToClause::Modifier>(Parser<OmpMapper>{}) ||
         construct<OmpToClause::Modifier>(Parser<OmpIterator>{})))))
 
-TYPE_PARSER(sourced(
-    construct<OmpDefaultmapClause::Modifier>(Parser<OmpVariableCategory>{})))
-
 // --- Parsers for clauses --------------------------------------------
 
 /// `MOBClause` is a clause that has a
@@ -334,29 +349,7 @@ TYPE_PARSER(construct<OmpInReductionClause>(
 //                                   variable-name-list)
 //                allocate-modifier -> allocator | align
 TYPE_PARSER(construct<OmpAllocateClause>(
-    maybe(
-        first(
-            construct<OmpAllocateClause::AllocateModifier>("ALLOCATOR" >>
-                construct<OmpAllocateClause::AllocateModifier::ComplexModifier>(
-                    parenthesized(construct<
-                        OmpAllocateClause::AllocateModifier::Allocator>(
-                        scalarIntExpr)) /
-                        ",",
-                    "ALIGN" >> parenthesized(construct<
-                                   OmpAllocateClause::AllocateModifier::Align>(
-                                   scalarIntExpr)))),
-            construct<OmpAllocateClause::AllocateModifier>("ALLOCATOR" >>
-                parenthesized(
-                    construct<OmpAllocateClause::AllocateModifier::Allocator>(
-                        scalarIntExpr))),
-            construct<OmpAllocateClause::AllocateModifier>("ALIGN" >>
-                parenthesized(
-                    construct<OmpAllocateClause::AllocateModifier::Align>(
-                        scalarIntExpr))),
-            construct<OmpAllocateClause::AllocateModifier>(
-                construct<OmpAllocateClause::AllocateModifier::Allocator>(
-                    scalarIntExpr))) /
-        ":"),
+    maybe(nonemptyList(Parser<OmpAllocateClause::Modifier>{}) / ":"),
     Parser<OmpObjectList>{}))
 
 // iteration-offset -> +/- non-negative-constant-expr
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index fe3f6ce7aa6291..192917512c17a2 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2148,35 +2148,21 @@ class UnparseVisitor {
     Walk(std::get<OmpObjectList>(x.t));
   }
   void Unparse(const OmpAllocateClause &x) {
-    Walk(
-        std::get<std::optional<OmpAllocateClause::AllocateModifier>>(x.t), ":");
+    using Modifier = OmpAllocateClause::Modifier;
+    Walk(std::get<std::optional<std::list<Modifier>>>(x.t), ": ");
     Walk(std::get<OmpObjectList>(x.t));
   }
-  void Unparse(const OmpAllocateClause::AllocateModifier &x) {
-    common::visit(
-        common::visitors{
-            [&](const OmpAllocateClause::AllocateModifier::Allocator &y) {
-              Walk(y);
-            },
-            [&](const OmpAllocateClause::AllocateModifier::ComplexModifier &y) {
-              Word("ALLOCATOR(");
-              Walk(std::get<OmpAllocateClause::AllocateModifier::Allocator>(
-                  y.t));
-              Put(")");
-              Put(",");
-              Walk(std::get<OmpAllocateClause::AllocateModifier::Align>(y.t));
-            },
-            [&](const OmpAllocateClause::AllocateModifier::Align &y) {
-              Walk(y);
-            },
-        },
-        x.u);
-  }
-  void Unparse(const OmpAllocateClause::AllocateModifier::Align &x) {
+  void Unparse(const OmpAlignModifier &x) {
     Word("ALIGN(");
     Walk(x.v);
     Put(")");
   }
+  void Unparse(const OmpAllocatorSimpleModifier &x) { Walk(x.v); }
+  void Unparse(const OmpAllocatorComplexModifier &x) {
+    Word("ALLOCATOR(");
+    Walk(x.v);
+    Put(")");
+  }
   void Unparse(const OmpOrderClause &x) {
     using Modifier = OmpOrderClause::Modifier;
     Walk(std::get<std::optional<std::list<Modifier>>>(x.t), ":");
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 3733ebfaf94925..b49258da506ce6 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -1481,34 +1481,26 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Allocator &x) {
 
 void OmpStructureChecker::Enter(const parser::OmpClause::Allocate &x) {
   CheckAllowedClause(llvm::omp::Clause::OMPC_allocate);
-  if (const auto &modifier{
-          std::get<std::optional<parser::OmpAllocateClause::AllocateModifier>>(
-              x.v.t)}) {
-    common::visit(
-        common::visitors{
-            [&](const parser::OmpAllocateClause::AllocateModifier::Allocator
-                    &y) {
-              RequiresPositiveParameter(llvm::omp::Clause::OMPC_allocate, y.v);
-              isPredefinedAllocator = GetIntValue(y.v).has_value();
-            },
-            [&](const parser::OmpAllocateClause::AllocateModifier::
-                    ComplexModifier &y) {
-              const auto &alloc = std::get<
-                  parser::OmpAllocateClause::AllocateModifier::Allocator>(y.t);
-              const auto &align =
-                  std::get<parser::OmpAllocateClause::AllocateModifier::Align>(
-                      y.t);
-              RequiresPositiveParameter(
-                  llvm::omp::Clause::OMPC_allocate, alloc.v);
-              RequiresPositiveParameter(
-                  llvm::omp::Clause::OMPC_allocate, align.v);
-              isPredefinedAllocator = GetIntValue(alloc.v).has_value();
-            },
-            [&](const parser::OmpAllocateClause::AllocateModifier::Align &y) {
-              RequiresPositiveParameter(llvm::omp::Clause::OMPC_allocate, y.v);
-            },
-        },
-        modifier->u);
+  if (OmpVerifyModifiers(
+          x.v, llvm::omp::OMPC_allocate, GetContext().clauseSource, context_)) {
+    auto &modifiers{OmpGetModifiers(x.v)};
+    if (auto *align{
+            OmpGetUniqueModifier<parser::OmpAlignModifier>(modifiers)}) {
+      if (const auto &v{GetIntValue(align->v)}; !v || *v <= 0) {
+        context_.Say(OmpGetModifierSource(modifiers, align),
+            "The alignment value should be a constant positive integer"_err_en_US);
+      }
+    }
+    // The simple and complex modifiers have the same structure. They only
+    // differ in their syntax.
+    if (auto *alloc{OmpGetUniqueModifier<parser::OmpAllocatorComplexModifier>(
+            modifiers)}) {
+      isPredefinedAllocator = GetIntValue(alloc->v).has_value();
+    }
+    if (auto *alloc{OmpGetUniqueModifier<parser::OmpAllocatorSimpleModifier>(
+            modifiers)}) {
+      isPredefinedAllocator = GetIntValue(alloc->v).has_value();
+    }
   }
 }
 
diff --git a/flang/lib/Semantics/openmp-modifiers.cpp b/flang/lib/Semantics/openmp-modifiers.cpp
index 1fd2358aa594ea..18863b7b904a4a 100644
--- a/flang/lib/Semantics/openmp-modifiers.cpp
+++ b/flang/lib/Semantics/openmp-modifiers.cpp
@@ -74,6 +74,56 @@ unsigned OmpModifierDescriptor::since(llvm::omp::Clause id) const {
 // Note: The intent for these functions is to have them be automatically-
 // generated in the future.
 
+template <>
+const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpAlignModifier>() {
+  static const OmpModifierDescriptor desc{
+      /*name=*/"align-modifier",
+      /*props=*/
+      {
+          {51, {OmpProperty::Unique}},
+      },
+      /*clauses=*/
+      {
+          {51, {Clause::OMPC_allocate}},
+      },
+  };
+  return desc;
+}
+
+template <>
+const OmpModifierDescriptor &
+OmpGetDescriptor<parser::OmpAllocatorComplexModifier>() {
+  static const OmpModifierDescriptor desc{
+      /*name=*/"allocator-complex-modifier",
+      /*props=*/
+      {
+          {51, {OmpProperty::Unique}},
+      },
+      /*clauses=*/
+      {
+          {51, {Clause::OMPC_allocate}},
+      },
+  };
+  return desc;
+}
+
+template <>
+const OmpModifierDescriptor &
+OmpGetDescriptor<parser::OmpAllocatorSimpleModifier>() {
+  static const OmpModifierDescriptor desc{
+      /*name=*/"allocator-simple-modifier",
+      /*props=*/
+      {
+          {50, {OmpProperty::Exclusive, OmpProperty::Unique}},
+      },
+      /*clauses=*/
+      {
+          {50, {Clause::OMPC_allocate}},
+      },
+  };
+  return desc;
+}
+
 template <>
 const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpChunkModifier>() {
   static const OmpModifierDescriptor desc{
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 0c3708b3fd29b4..4f56356d879a40 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2097,17 +2097,16 @@ void OmpAttributeVisitor::Post(const parser::OpenMPAllocatorsConstruct &x) {
           std::get<parser::OmpObjectList>(alloc->v.t),
           std::get<parser::Statement<parser::AllocateStmt>>(x.t).statement);
 
-      const auto &allocMod{
-          std::get<std::optional<parser::OmpAllocateClause::AllocateModifier>>(
-              alloc->v.t)};
+      auto &modifiers{OmpGetModifiers(alloc->v)};
+      bool hasAllocator{
+          OmpGetUniqueModifier<parser::OmpAllocatorSimpleModifier>(modifiers) ||
+          OmpGetUniqueModifier<parser::OmpAllocatorComplexModifier>(modifiers)};
+
       // TODO: As with allocate directive, exclude the case when a requires
       //       directive with the dynamic_allocators clause is present in
       //       the same compilation unit (OMP5.0 2.11.3).
       if (IsNestedInDirective(llvm::omp::Directive::OMPD_target) &&
-          (!allocMod.has_value() ||
-              std::holds_alternative<
-                  parser::OmpAllocateClause::AllocateModifier::Align>(
-                  allocMod->u))) {
+          !hasAllocator) {
         context_.Say(x.source,
             "ALLOCATORS directives that appear in a TARGET region "
             "must specify an allocator"_err_en_US);
diff --git a/flang/test/Parser/OpenMP/allocators-unparse.f90 b/flang/test/Parser/OpenMP/allocators-unparse.f90
index 062a48b02635f3..5cd0230471fc4a 100644
--- a/flang/test/Parser/OpenMP/allocators-unparse.f90
+++ b/flang/test/Parser/OpenMP/allocators-unparse.f90
@@ -18,18 +18,18 @@ subroutine allocate()
 end subroutine allocate
 
 !CHECK: INTEGER, ALLOCATABLE :: arr1(:), arr2(:,:)
-!CHECK-NEXT:!$OMP ALLOCATE ALLOCATE(omp_default_mem_alloc:arr1)
+!CHECK-NEXT:!$OMP ALLOCATE ALLOCATE(omp_default_mem_alloc: arr1)
 !CHECK-NEXT: ALLOCATE(arr1(5))
-!CHECK-NEXT:!$OMP ALLOCATE ALLOCATE(ALLOCATOR(omp_default_mem_alloc),ALIGN(32):arr1) ALLOC&
-!CHECK-NEXT:!$OMP&ATE(omp_default_mem_alloc:arr2)
+!CHECK-NEXT:!$OMP ALLOCATE ALLOCATE(ALLOCATOR(omp_default_mem_alloc), ALIGN(32): arr1) ALL&
+!CHECK-NEXT:!$OMP&OCATE(omp_default_mem_alloc: arr2)
 !CHECK-NEXT: ALLOCATE(arr1(10), arr2(3,2))
-!CHECK-NEXT:!$OMP ALLOCATE ALLOCATE(ALIGN(32):arr2)
+!CHECK-NEXT:!$OMP ALLOCATE ALLOCATE(ALIGN(32): arr2)
 !CHECK-NEXT: ALLOCATE(arr2(5,3))
 
 !PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPAllocatorsConstruct
 !PARSE-TREE-NEXT: Verbatim
 !PARSE-TREE-NEXT: OmpClauseList -> OmpClause -> Allocate -> OmpAllocateClause
-!PARSE-TREE-NEXT: AllocateModifier -> Allocator -> Scalar -> Integer -> Expr -> Designator -> DataRef -> Name =
+!PARSE-TREE-NEXT: Modifier -> OmpAllocatorSimpleModifier -> Scalar -> Integer -> Expr -> Designator -> DataRef -> Name = 'omp_default_mem_alloc'
 !PARSE-TREE-NEXT: OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'arr1'
 !PARSE-TREE-NEXT: AllocateStmt
 !PARSE-TREE-NEXT: Allocation
@@ -38,12 +38,11 @@ end subroutine allocate
 !PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPAllocatorsConstruct
 !PARSE-TREE-NEXT: Verbatim
 !PARSE-TREE-NEXT: OmpClauseList -> OmpClause -> Allocate -> OmpAllocateClause
-!PARSE-TREE-NEXT: AllocateModifier -> ComplexModifier
-!PARSE-TREE-NEXT: Allocator -> Scalar -> Integer -> Expr -> Designator -> DataRef -> Name =
-!PARSE-TREE-NEXT: Align -> Scalar -> Integer -> Expr -> LiteralConstant -> IntLiteralConstant = '32'
+!PARSE-TREE-NEXT: Modifier -> OmpAllocatorComplexModifier -> Scalar -> Integer -> Expr -> Designator -> DataRef -> Name = 'omp_default_mem_alloc'
+!PARSE-TREE-NEXT: Modifier -> OmpAlignModifier -> Scalar -> Integer -> Expr -> LiteralConstant -> IntLiteralConstant = '32'
 !PARSE-TREE-NEXT: OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'arr1'
 !PARSE-TREE-NEXT: OmpClause -> Allocate -> OmpAllocateClause
-!PARSE-TREE-NEXT: AllocateModifier -> Allocator -> Scalar -> Integer -> Expr -> Designator -> DataRef -> Name =
+!PARSE-TREE-NEXT: Modifier -> OmpAllocatorSimpleModifier -> Scalar -> Integer -> Expr -> Designator -> DataRef -> Name = 'omp_default_mem_alloc'
 !PARSE-TREE-NEXT: OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'arr2'
 !PARSE-TREE-NEXT: AllocateStmt
 !PARSE-TREE-NEXT: Allocation
@@ -56,7 +55,7 @@ end subroutine allocate
 !PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPAllocatorsConstruct
 !PARSE-TREE-NEXT: Verbatim
 !PARSE-TREE-NEXT: OmpClauseList -> OmpClause -> Allocate -> OmpAllocateClause
-!PARSE-TREE-NEXT: AllocateModifier -> Align -> Scalar -> Integer -> Expr -> LiteralConstant -> IntLiteralConstant = '32'
+!PARSE-TREE-NEXT: Modifier -> OmpAlignModifier -> Scalar -> Integer -> Expr -> LiteralConstant -> IntLiteralConstant = '32'
 !PARSE-TREE-NEXT: OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'arr2'
 !PARSE-TREE-NEXT: AllocateStmt
 !PARSE-TREE-NEXT: Allocation
diff --git a/flang/test/Semantics/OpenMP/allocate-clause01.f90 b/flang/test/Semantics/OpenMP/allocate-clause01.f90
index 2b9a72e928eba9..a014b548a87710 100644
--- a/flang/test/Semantics/OpenMP/allocate-clause01.f90
+++ b/flang/test/Semantics/OpenMP/allocate-clause01.f90
@@ -1,6 +1,6 @@
 ! REQUIRES: openmp_runtime
 
-! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags
+! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags -fopenmp-version=51
 ! OpenMP Version 5.2
 ! The allocate clause's allocator modifier must be of type allocator_handle
 ! and the align modifier must be constant, positive integer expression
@@ -10,15 +10,11 @@ subroutine allocate()
 
     integer, allocatable :: a, b, c
 
-    !ERROR: The parameter of the ALLOCATE clause must be a positive integer expression
-    !$omp allocators allocate(-1: a)
-        allocate(a)
-
-    !ERROR: The parameter of the ALLOCATE clause must be a positive integer expression
+    !ERROR: The alignment value should be a constant positive integer
     !$omp allocators allocate(allocator(-2), align(-3): b)
         allocate(b)
 
-    !ERROR: The parameter of the ALLOCATE clause must be a positive integer expression
+    !ERROR: The alignment value should be a constant positive integer
     !$omp allocators allocate(align(-4): c)
         allocate(c)
 end subroutine
diff --git a/flang/test/Semantics/OpenMP/allocators01.f90 b/flang/test/Semantics/OpenMP/allocators01.f90
index c75c522ecae10a..ff92fa3b234638 100644
--- a/flang/test/Semantics/OpenMP/allocators01.f90
+++ b/flang/test/Semantics/OpenMP/allocators01.f90
@@ -1,6 +1,6 @@
 ! REQUIRES: openmp_runtime
 
-! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags
+! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags -fopenmp-version=50
 ! OpenMP Version 5.2
 ! 6.7 allocators construct
 ! A list item that appears in an allocate clause must appear as
diff --git a/flang/test/Semantics/OpenMP/allocators04.f90 b/flang/test/Semantics/OpenMP/allocators04.f90
index 1d2e96443a9da3..212e48fbd1b263 100644
--- a/flang/test/Semantics/OpenMP/allocators04.f90
+++ b/flang/test/Semantics/OpenMP/allocators04.f90
@@ -1,6 +1,6 @@
 ! REQUIRES: openmp_runtime
 
-! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags
+! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags -fopenmp-version=50
 ! OpenMP Version 5.2
 ! Inherited from 2.11.3 allocate Directive
 ! If list items within the ALLOCATE directive have the SAVE attribute, are a common block name, or are declared in the scope of a
diff --git a/flang/test/Semantics/OpenMP/allocators05.f90 b/flang/test/Semantics/OpenMP/allocators05.f90
index d0e11ca5874d70..0e8366a2461e67 100644
--- a/flang/test/Semantics/OpenMP/allocators05.f90
+++ b/flang/test/Semantics/OpenMP/allocators05.f90
@@ -1,6 +1,6 @@
 ! REQUIRES: openmp_runtime
 
-! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags
+! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags -fopenmp-version=50
 ! OpenMP Version 5.2
 ! Inherited from 2.11.3 allocate directive
 ! allocate directives that appear in a target region must specify an
diff --git a/flang/test/Semantics/OpenMP/allocators06.f90 b/flang/test/Semantics/OpenMP/allocators06.f90
index a975204c11339e..8e63512369e38c 100644
--- a/flang/test/Semantics/OpenMP/allocators06.f90
+++ b/flang/test/Semantics/OpenMP/allocators06.f90
@@ -1,6 +1,6 @@
 ! REQUIRES: openmp_runtime
 
-! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags
+! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags -fopenmp-version=50
 ! OpenMP Version 5.2
 ! Inherited from 2.11.3 allocate directive
 ! The allocate directive must appear in the same scope as the declarations of
diff --git a/flang/test/Semantics/OpenMP/resolve06.f90 b/flang/test/Semantics/OpenMP/resolve06.f90
index 358b1b1cc2826b..cf544dfb73cad3 100644
--- a/flang/test/Semantics/OpenMP/resolve06.f90
+++ b/flang/test/Semantics/OpenMP/resolve06.f90
@@ -1,6 +1,6 @@
 ! REQUIRES: openmp_runtime
 
-! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags
+! RUN: %python %S/../test_errors.py %s %flang_fc1 %openmp_flags -fopenmp-version=50
 use omp_lib
 !2.11.4 Allocate Clause
 !For any list item that is specified in the allocate
diff --git a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h
index ce09bb5f730a72..07efd6fd4e9da4 100644
--- a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h
+++ b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h
@@ -384,15 +384,13 @@ struct AllocatorT;
 // V5.2: [6.6] `allocate` clause
 template <typename T, typename I, typename E> //
 struct AllocateT {
-  using AllocatorSimpleModifier = E;
+  // AllocatorSimpleModifier is same as AllocatorComplexModifier.
   using AllocatorComplexModifier = AllocatorT<T, I, E>;
   using AlignModifier = AlignT<T, I, E>;
   using List = ObjectListT<I, E>;
 
   using TupleTrait = std::true_type;
-  std::tuple<OPT(AllocatorSimpleModifier), OPT(AllocatorComplexModifier),
-             OPT(AlignModifier), List>
-      t;
+  std::tuple<OPT(AllocatorComplexModifier), OPT(AlignModifier), List> t;
 };
 
 // V5.2: [6.4] `allocator` clause

>From af029c812c178c2854f0fd394ccc5654acae233c Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 25 Nov 2024 15:09:18 -0600
Subject: [PATCH 02/11] fix examples

---
 flang/examples/FeatureList/FeatureList.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/flang/examples/FeatureList/FeatureList.cpp b/flang/examples/FeatureList/FeatureList.cpp
index 6ae92acf20608a..f7086b9e82b8d0 100644
--- a/flang/examples/FeatureList/FeatureList.cpp
+++ b/flang/examples/FeatureList/FeatureList.cpp
@@ -516,10 +516,10 @@ struct NodeVisitor {
   READ_FEATURE(OmpReductionInitializerClause)
   READ_FEATURE(OmpReductionIdentifier)
   READ_FEATURE(OmpAllocateClause)
-  READ_FEATURE(OmpAllocateClause::AllocateModifier)
-  READ_FEATURE(OmpAllocateClause::AllocateModifier::Allocator)
-  READ_FEATURE(OmpAllocateClause::AllocateModifier::ComplexModifier)
-  READ_FEATURE(OmpAllocateClause::AllocateModifier::Align)
+  READ_FEATURE(OmpAllocateClause::Modifier)
+  READ_FEATURE(OmpAllocatorSimpleModifier)
+  READ_FEATURE(OmpAllocatorComplexModifier)
+  READ_FEATURE(OmpAlignModifier)
   READ_FEATURE(OmpScheduleClause)
   READ_FEATURE(OmpScheduleClause::Kind)
   READ_FEATURE(OmpScheduleClause::Modifier)

>From 88ac6d0b50c26b69d0872e2b321be80e2584cb27 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 25 Nov 2024 17:15:09 -0600
Subject: [PATCH 03/11] fix llvm unittests

---
 .../Frontend/OpenMPDecompositionTest.cpp      | 50 ++++++++-----------
 1 file changed, 22 insertions(+), 28 deletions(-)

diff --git a/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp b/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp
index f9541131e4b23d..8f195c4f603268 100644
--- a/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp
+++ b/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp
@@ -686,12 +686,12 @@ TEST_F(OpenMPDecompositionTest, Order1) {
   std::string Dir3 = stringify(Dec.output[3]);
   std::string Dir4 = stringify(Dec.output[4]);
   std::string Dir5 = stringify(Dec.output[5]);
-  ASSERT_EQ(Dir0, "target"); // (31)
-  ASSERT_EQ(Dir1, "teams");  // (31)
+  ASSERT_EQ(Dir0, "target");                 // (31)
+  ASSERT_EQ(Dir1, "teams");                  // (31)
   ASSERT_EQ(Dir2, "distribute order(1, 0)"); // (31)
-  ASSERT_EQ(Dir3, "parallel");         // (31)
-  ASSERT_EQ(Dir4, "for order(1, 0)");  // (31)
-  ASSERT_EQ(Dir5, "simd order(1, 0)"); // (31)
+  ASSERT_EQ(Dir3, "parallel");               // (31)
+  ASSERT_EQ(Dir4, "for order(1, 0)");        // (31)
+  ASSERT_EQ(Dir5, "simd order(1, 0)");       // (31)
 }
 
 // ALLOCATE
@@ -708,8 +708,7 @@ TEST_F(OpenMPDecompositionTest, Allocate1) {
 
   // Allocate + firstprivate
   omp::List<omp::Clause> Clauses{
-      {OMPC_allocate,
-       omp::clause::Allocate{{std::nullopt, std::nullopt, std::nullopt, {x}}}},
+      {OMPC_allocate, omp::clause::Allocate{{std::nullopt, std::nullopt, {x}}}},
       {OMPC_firstprivate, omp::clause::Firstprivate{{x}}},
   };
 
@@ -719,8 +718,8 @@ TEST_F(OpenMPDecompositionTest, Allocate1) {
 
   std::string Dir0 = stringify(Dec.output[0]);
   std::string Dir1 = stringify(Dec.output[1]);
-  ASSERT_EQ(Dir0, "parallel shared(x)");                           // (33)
-  ASSERT_EQ(Dir1, "sections firstprivate(x) allocate(, , , (x))"); // (33)
+  ASSERT_EQ(Dir0, "parallel shared(x)");                         // (33)
+  ASSERT_EQ(Dir1, "sections firstprivate(x) allocate(, , (x))"); // (33)
 }
 
 TEST_F(OpenMPDecompositionTest, Allocate2) {
@@ -729,8 +728,7 @@ TEST_F(OpenMPDecompositionTest, Allocate2) {
 
   // Allocate + in_reduction
   omp::List<omp::Clause> Clauses{
-      {OMPC_allocate,
-       omp::clause::Allocate{{std::nullopt, std::nullopt, std::nullopt, {x}}}},
+      {OMPC_allocate, omp::clause::Allocate{{std::nullopt, std::nullopt, {x}}}},
       {OMPC_in_reduction, omp::clause::InReduction{{{Add}, {x}}}},
   };
 
@@ -740,8 +738,8 @@ TEST_F(OpenMPDecompositionTest, Allocate2) {
 
   std::string Dir0 = stringify(Dec.output[0]);
   std::string Dir1 = stringify(Dec.output[1]);
-  ASSERT_EQ(Dir0, "target in_reduction((3), (x)) allocate(, , , (x))"); // (33)
-  ASSERT_EQ(Dir1, "parallel");                                          // (33)
+  ASSERT_EQ(Dir0, "target in_reduction((3), (x)) allocate(, , (x))"); // (33)
+  ASSERT_EQ(Dir1, "parallel");                                        // (33)
 }
 
 TEST_F(OpenMPDecompositionTest, Allocate3) {
@@ -749,8 +747,7 @@ TEST_F(OpenMPDecompositionTest, Allocate3) {
 
   // Allocate + linear
   omp::List<omp::Clause> Clauses{
-      {OMPC_allocate,
-       omp::clause::Allocate{{std::nullopt, std::nullopt, std::nullopt, {x}}}},
+      {OMPC_allocate, omp::clause::Allocate{{std::nullopt, std::nullopt, {x}}}},
       {OMPC_linear,
        omp::clause::Linear{{std::nullopt, std::nullopt, std::nullopt, {x}}}},
   };
@@ -765,7 +762,7 @@ TEST_F(OpenMPDecompositionTest, Allocate3) {
   // should be fixed eventually.
   ASSERT_EQ(Dir0, "parallel shared(x) shared(x)"); // (33)
   ASSERT_EQ(Dir1, "for linear(, , , (x)) firstprivate(x) lastprivate(, (x)) "
-                  "allocate(, , , (x))"); // (33)
+                  "allocate(, , (x))"); // (33)
 }
 
 TEST_F(OpenMPDecompositionTest, Allocate4) {
@@ -773,8 +770,7 @@ TEST_F(OpenMPDecompositionTest, Allocate4) {
 
   // Allocate + lastprivate
   omp::List<omp::Clause> Clauses{
-      {OMPC_allocate,
-       omp::clause::Allocate{{std::nullopt, std::nullopt, std::nullopt, {x}}}},
+      {OMPC_allocate, omp::clause::Allocate{{std::nullopt, std::nullopt, {x}}}},
       {OMPC_lastprivate, omp::clause::Lastprivate{{std::nullopt, {x}}}},
   };
 
@@ -784,8 +780,8 @@ TEST_F(OpenMPDecompositionTest, Allocate4) {
 
   std::string Dir0 = stringify(Dec.output[0]);
   std::string Dir1 = stringify(Dec.output[1]);
-  ASSERT_EQ(Dir0, "parallel shared(x)");                              // (33)
-  ASSERT_EQ(Dir1, "sections lastprivate(, (x)) allocate(, , , (x))"); // (33)
+  ASSERT_EQ(Dir0, "parallel shared(x)");                            // (33)
+  ASSERT_EQ(Dir1, "sections lastprivate(, (x)) allocate(, , (x))"); // (33)
 }
 
 TEST_F(OpenMPDecompositionTest, Allocate5) {
@@ -793,8 +789,7 @@ TEST_F(OpenMPDecompositionTest, Allocate5) {
 
   // Allocate + private
   omp::List<omp::Clause> Clauses{
-      {OMPC_allocate,
-       omp::clause::Allocate{{std::nullopt, std::nullopt, std::nullopt, {x}}}},
+      {OMPC_allocate, omp::clause::Allocate{{std::nullopt, std::nullopt, {x}}}},
       {OMPC_private, omp::clause::Private{{x}}},
   };
 
@@ -804,8 +799,8 @@ TEST_F(OpenMPDecompositionTest, Allocate5) {
 
   std::string Dir0 = stringify(Dec.output[0]);
   std::string Dir1 = stringify(Dec.output[1]);
-  ASSERT_EQ(Dir0, "parallel");                                // (33)
-  ASSERT_EQ(Dir1, "sections private(x) allocate(, , , (x))"); // (33)
+  ASSERT_EQ(Dir0, "parallel");                              // (33)
+  ASSERT_EQ(Dir1, "sections private(x) allocate(, , (x))"); // (33)
 }
 
 TEST_F(OpenMPDecompositionTest, Allocate6) {
@@ -814,8 +809,7 @@ TEST_F(OpenMPDecompositionTest, Allocate6) {
 
   // Allocate + reduction
   omp::List<omp::Clause> Clauses{
-      {OMPC_allocate,
-       omp::clause::Allocate{{std::nullopt, std::nullopt, std::nullopt, {x}}}},
+      {OMPC_allocate, omp::clause::Allocate{{std::nullopt, std::nullopt, {x}}}},
       {OMPC_reduction, omp::clause::Reduction{{std::nullopt, {Add}, {x}}}},
   };
 
@@ -825,8 +819,8 @@ TEST_F(OpenMPDecompositionTest, Allocate6) {
 
   std::string Dir0 = stringify(Dec.output[0]);
   std::string Dir1 = stringify(Dec.output[1]);
-  ASSERT_EQ(Dir0, "parallel shared(x)");                                 // (33)
-  ASSERT_EQ(Dir1, "sections reduction(, (3), (x)) allocate(, , , (x))"); // (33)
+  ASSERT_EQ(Dir0, "parallel shared(x)");                               // (33)
+  ASSERT_EQ(Dir1, "sections reduction(, (3), (x)) allocate(, , (x))"); // (33)
 }
 
 // REDUCTION

>From ee00ecdc5a8b342c80ef34a8e1a1c8bb91855ab8 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 26 Nov 2024 09:01:49 -0600
Subject: [PATCH 04/11] [flang][OpenMP] Rename some `Type` members in OpenMP
 clauses

The intent is to keep names in sync with the terminology from the OpenMP
spec:
  OmpBindClause::Type       -> Binding
  OmpDefaultClause::Type    -> DataSharingAttribute
  OmpDeviceTypeClause::Type -> DeviceTypeDescription
  OmpProcBindClause::Type   -> AffinityPolicy

Add more comments with references to the OpenMP specs.
---
 flang/include/flang/Parser/dump-parse-tree.h  |   8 +-
 flang/include/flang/Parser/parse-tree.h       | 130 +++++++++++++-----
 flang/lib/Lower/OpenMP/Clauses.cpp            |   9 +-
 flang/lib/Parser/openmp-parsers.cpp           |  29 ++--
 flang/lib/Parser/unparse.cpp                  |   9 +-
 flang/lib/Semantics/check-omp-structure.cpp   |   4 +-
 flang/lib/Semantics/resolve-directives.cpp    |   8 +-
 .../OpenMP/declare_target-device_type.f90     |   6 +-
 flang/test/Parser/OpenMP/proc-bind.f90        |   2 +-
 .../Parser/OpenMP/target-loop-unparse.f90     |   2 +-
 10 files changed, 133 insertions(+), 74 deletions(-)

diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index d499b414827d50..e67b95ca61e8bf 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -513,7 +513,7 @@ class ParseTreeDumper {
   NODE(parser, OmpDeclareTargetWithList)
   NODE(parser, OmpDeclareMapperSpecifier)
   NODE(parser, OmpDefaultClause)
-  NODE_ENUM(OmpDefaultClause, Type)
+  NODE_ENUM(OmpDefaultClause, DataSharingAttribute)
   NODE(parser, OmpVariableCategory)
   NODE_ENUM(OmpVariableCategory, Value)
   NODE(parser, OmpDefaultmapClause)
@@ -573,9 +573,9 @@ class ParseTreeDumper {
   NODE(parser, OmpNumTasksClause)
   NODE_ENUM(OmpNumTasksClause, Prescriptiveness)
   NODE(parser, OmpBindClause)
-  NODE_ENUM(OmpBindClause, Type)
+  NODE_ENUM(OmpBindClause, Binding)
   NODE(parser, OmpProcBindClause)
-  NODE_ENUM(OmpProcBindClause, Type)
+  NODE_ENUM(OmpProcBindClause, AffinityPolicy)
   NODE(parser, OmpReductionModifier)
   NODE_ENUM(OmpReductionModifier, Value)
   NODE(parser, OmpReductionClause)
@@ -596,7 +596,7 @@ class ParseTreeDumper {
   NODE(parser, OmpDeviceClause)
   NODE_ENUM(OmpDeviceClause, DeviceModifier)
   NODE(parser, OmpDeviceTypeClause)
-  NODE_ENUM(OmpDeviceTypeClause, Type)
+  NODE_ENUM(OmpDeviceTypeClause, DeviceTypeDescription)
   NODE(parser, OmpUpdateClause)
   NODE(parser, OmpChunkModifier)
   NODE_ENUM(OmpChunkModifier, Value)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index e9a02a87812452..41b9ad1b7b0d61 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3686,21 +3686,51 @@ struct OmpAllocateClause {
   std::tuple<MODIFIERS(), OmpObjectList> t;
 };
 
-// OMP 5.0 2.4 atomic-default-mem-order-clause ->
-//                 ATOMIC_DEFAULT_MEM_ORDER (SEQ_CST | ACQ_REL |
-//                                           RELAXED)
+// Ref: [5.0:60-63], [5.1:83-86], [5.2:210-213]
+//
+// atomic-default-mem-order-clause ->
+//    ATOMIC_DEFAULT_MEM_ORDER(memory-order)        // since 5.0
+// memory-order ->
+//    SEQ_CST | ACQ_REL | RELAXED |                 // since 5.0
+//    ACQUIRE | RELEASE                             // since 5.2
 struct OmpAtomicDefaultMemOrderClause {
-  WRAPPER_CLASS_BOILERPLATE(
-      OmpAtomicDefaultMemOrderClause, common::OmpAtomicDefaultMemOrderType);
+  using MemoryOrder = common::OmpAtomicDefaultMemOrderType;
+  WRAPPER_CLASS_BOILERPLATE(OmpAtomicDefaultMemOrderClause, MemoryOrder);
+};
+
+// Ref: [5.0:128-131], [5.1:151-154], [5.2:258-259]
+//
+// bind-clause ->
+//    BIND(binding)                                 // since 5.0
+// binding ->
+//    TEAMS | PARALLEL | THREAD                     // since 5.0
+struct OmpBindClause {
+  ENUM_CLASS(Binding, Parallel, Teams, Thread)
+  WRAPPER_CLASS_BOILERPLATE(OmpBindClause, Binding);
 };
 
-// 2.15.3.1 default-clause -> DEFAULT (PRIVATE | FIRSTPRIVATE | SHARED | NONE)
+// Ref: [4.5:46-50], [5.0:74-78], [5.1:92-96], [5.2:109]
+//
+// default-clause ->
+//    DEFAULT(data-sharing-attribute)               // since 4.5
+// data-sharing-attribute ->
+//    SHARED | NONE |                               // since 4.5
+//    PRIVATE | FIRSTPRIVATE                        // since 5.0
 struct OmpDefaultClause {
-  ENUM_CLASS(Type, Private, Firstprivate, Shared, None)
-  WRAPPER_CLASS_BOILERPLATE(OmpDefaultClause, Type);
+  ENUM_CLASS(DataSharingAttribute, Private, Firstprivate, Shared, None)
+  WRAPPER_CLASS_BOILERPLATE(OmpDefaultClause, DataSharingAttribute);
 };
 
-// 2.15.5.2 defaultmap -> DEFAULTMAP (implicit-behavior[:variable-category])
+// Ref: [4.5:103-107], [5.0:324-325], [5.1:357-358], [5.2:161-162]
+//
+// defaultmap-clause ->
+//    DEFAULTMAP(implicit-behavior
+//        [: variable-category])                    // since 5.0
+// implicit-behavior ->
+//    TOFROM |                                      // since 4.5
+//    ALLOC | TO | FROM | FIRSTPRIVATE | NONE |
+//    DEFAULT |                                     // since 5.0
+//    PRESENT                                       // since 5.1
 struct OmpDefaultmapClause {
   TUPLE_CLASS_BOILERPLATE(OmpDefaultmapClause);
   ENUM_CLASS(
@@ -3709,23 +3739,35 @@ struct OmpDefaultmapClause {
   std::tuple<ImplicitBehavior, MODIFIERS()> t;
 };
 
-// 2.13.9 iteration-offset -> +/- non-negative-constant
+// Ref: [4.5:169-172], [5.0:255-259], [5.1:288-292], [5.2:91-93]
+//
+// iteration-offset ->
+//    +|- non-negative-constant                     // since 4.5
 struct OmpIterationOffset {
   TUPLE_CLASS_BOILERPLATE(OmpIterationOffset);
   std::tuple<DefinedOperator, ScalarIntConstantExpr> t;
 };
 
-// 2.13.9 iteration -> induction-variable [iteration-offset]
+// Ref: [4.5:169-172], [5.0:255-259], [5.1:288-292], [5.2:91-93]
+//
+// iteration ->
+//    induction-variable [iteration-offset]         // since 4.5
 struct OmpIteration {
   TUPLE_CLASS_BOILERPLATE(OmpIteration);
   std::tuple<Name, std::optional<OmpIterationOffset>> t;
 };
 
+// Ref: [4.5:169-172], [5.0:255-259], [5.1:288-292], [5.2:91-93]
+//
+// iteration-vector ->
+//    [iteration...]                                // since 4.5
 WRAPPER_CLASS(OmpIterationVector, std::list<OmpIteration>);
 
 // Extract this into a separate structure (instead of having it directly in
 // OmpDoacrossClause), so that the context in TYPE_CONTEXT_PARSER can be set
 // separately for OmpDependClause and OmpDoacrossClause.
+//
+// See: depend-clause, doacross-clause
 struct OmpDoacross {
   OmpDependenceType::Value GetDepType() const;
 
@@ -3735,15 +3777,15 @@ struct OmpDoacross {
   std::variant<Sink, Source> u;
 };
 
-// Ref: [4.5:169-170], [5.0:255-256], [5.1:288-289], [5.2:323-324]
+// Ref: [4.5:169-172], [5.0:255-259], [5.1:288-292], [5.2:323-326]
 //
 // depend-clause ->
-//    DEPEND(SOURCE) |                               // since 4.5, until 5.1
-//    DEPEND(SINK: iteration-vector) |               // since 4.5, until 5.1
+//    DEPEND(SOURCE) |                              // since 4.5, until 5.1
+//    DEPEND(SINK: iteration-vector) |              // since 4.5, until 5.1
 //    DEPEND([depend-modifier,]
-//           task-dependence-type: locator-list)     // since 4.5
+//           task-dependence-type: locator-list)    // since 4.5
 //
-// depend-modifier -> iterator-modifier              // since 5.0
+// depend-modifier -> iterator-modifier             // since 5.0
 struct OmpDependClause {
   UNION_CLASS_BOILERPLATE(OmpDependClause);
   struct TaskDep {
@@ -3755,6 +3797,10 @@ struct OmpDependClause {
   std::variant<TaskDep, OmpDoacross> u;
 };
 
+// Ref: [5.2:326-328]
+//
+// doacross-clause ->
+//    DOACROSS(dependence-type: iteration-vector)   // since 5.2
 WRAPPER_CLASS(OmpDoacrossClause, OmpDoacross);
 
 // Ref: [5.0:254-255], [5.1:287-288], [5.2:73]
@@ -3764,25 +3810,41 @@ WRAPPER_CLASS(OmpDoacrossClause, OmpDoacross);
 //    DESTROY(variable)     // since 5.2
 WRAPPER_CLASS(OmpDestroyClause, OmpObject);
 
-// device([ device-modifier :] scalar-integer-expression)
+// Ref: [5.0:135-140], [5.1:161-166], [5.2:265-266]
+//
+// detach-clause ->
+//    DETACH(event-handle)                          // since 5.0
+struct OmpDetachClause {
+  WRAPPER_CLASS_BOILERPLATE(OmpDetachClause, OmpObject);
+};
+
+// Ref: [4.5:103-107], [5.0:170-176], [5.1:197-205], [5.2:276-277]
+//
+// device-clause ->
+//    DEVICE(scalar-integer-expression) |           // since 4.5
+//    DEVICE([device-modifier:]
+//        scalar-integer-expression)                // since 5.0
 struct OmpDeviceClause {
   TUPLE_CLASS_BOILERPLATE(OmpDeviceClause);
   ENUM_CLASS(DeviceModifier, Ancestor, Device_Num)
   std::tuple<std::optional<DeviceModifier>, ScalarIntExpr> t;
 };
 
-// device_type(any | host | nohost)
+// Ref: [5.0:180-185], [5.1:210-216], [5.2:275]
+//
+// device-type-clause ->
+//    DEVICE_TYPE(ANY | HOST | NOHOST)              // since 5.0
 struct OmpDeviceTypeClause {
-  ENUM_CLASS(Type, Any, Host, Nohost)
-  WRAPPER_CLASS_BOILERPLATE(OmpDeviceTypeClause, Type);
+  ENUM_CLASS(DeviceTypeDescription, Any, Host, Nohost)
+  WRAPPER_CLASS_BOILERPLATE(OmpDeviceTypeClause, DeviceTypeDescription);
 };
 
 // Ref: [4.5:107-109], [5.0:176-180], [5.1:205-210], [5.2:167-168]
 //
 // from-clause ->
 //    FROM(locator-list) |
-//    FROM(mapper-modifier: locator-list) |             // since 5.0
-//    FROM(motion-modifier[,] ...: locator-list)        // since 5.1
+//    FROM(mapper-modifier: locator-list) |         // since 5.0
+//    FROM(motion-modifier[,] ...: locator-list)    // since 5.1
 //  motion-modifier ->
 //    PRESENT | mapper-modifier | iterator-modifier
 struct OmpFromClause {
@@ -3806,11 +3868,6 @@ struct OmpIfClause {
   std::tuple<std::optional<DirectiveNameModifier>, ScalarLogicalExpr> t;
 };
 
-// OpenMPv5.2 12.5.2 detach-clause -> DETACH (event-handle)
-struct OmpDetachClause {
-  WRAPPER_CLASS_BOILERPLATE(OmpDetachClause, OmpObject);
-};
-
 // OMP 5.0 2.19.5.6 in_reduction-clause -> IN_REDUCTION (reduction-identifier:
 //                                         variable-name-list)
 struct OmpInReductionClause {
@@ -3878,10 +3935,16 @@ struct OmpOrderClause {
   std::tuple<MODIFIERS(), Ordering> t;
 };
 
-// 2.5 proc-bind-clause -> PROC_BIND (MASTER | CLOSE | SPREAD)
+// Ref: [4.5:46-50], [5.0:74-78], [5.1:92-96], [5.2:229-230]
+//
+// proc-bind-clause ->
+//    PROC_BIND(affinity-policy)                    // since 4.5
+// affinity-policy ->
+//    CLOSE | PRIMARY | SPREAD |                    // since 4.5
+//    MASTER                                        // since 4.5, until 5.2
 struct OmpProcBindClause {
-  ENUM_CLASS(Type, Close, Master, Spread, Primary)
-  WRAPPER_CLASS_BOILERPLATE(OmpProcBindClause, Type);
+  ENUM_CLASS(AffinityPolicy, Close, Master, Spread, Primary)
+  WRAPPER_CLASS_BOILERPLATE(OmpProcBindClause, AffinityPolicy);
 };
 
 // Ref: [4.5:201-207], [5.0:300-302], [5.1:332-334], [5.2:134-137]
@@ -3945,13 +4008,6 @@ struct OmpUpdateClause {
   std::variant<OmpDependenceType, OmpTaskDependenceType> u;
 };
 
-// OMP 5.2 11.7.1 bind-clause ->
-//                  BIND( PARALLEL | TEAMS | THREAD )
-struct OmpBindClause {
-  ENUM_CLASS(Type, Parallel, Teams, Thread)
-  WRAPPER_CLASS_BOILERPLATE(OmpBindClause, Type);
-};
-
 // OpenMP Clauses
 struct OmpClause {
   UNION_CLASS_BOILERPLATE(OmpClause);
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index ddc91ef2030bd7..6d09cab700fd6f 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -478,7 +478,7 @@ Bind make(const parser::OmpClause::Bind &inp,
   using wrapped = parser::OmpBindClause;
 
   CLAUSET_ENUM_CONVERT( //
-      convert, wrapped::Type, Bind::Binding,
+      convert, wrapped::Binding, Bind::Binding,
       // clang-format off
       MS(Teams, Teams)
       MS(Parallel, Parallel)
@@ -523,7 +523,7 @@ Default make(const parser::OmpClause::Default &inp,
   using wrapped = parser::OmpDefaultClause;
 
   CLAUSET_ENUM_CONVERT( //
-      convert, wrapped::Type, Default::DataSharingAttribute,
+      convert, wrapped::DataSharingAttribute, Default::DataSharingAttribute,
       // clang-format off
       MS(Firstprivate, Firstprivate)
       MS(None,         None)
@@ -680,7 +680,8 @@ DeviceType make(const parser::OmpClause::DeviceType &inp,
   using wrapped = parser::OmpDeviceTypeClause;
 
   CLAUSET_ENUM_CONVERT( //
-      convert, wrapped::Type, DeviceType::DeviceTypeDescription,
+      convert, wrapped::DeviceTypeDescription,
+      DeviceType::DeviceTypeDescription,
       // clang-format off
       MS(Any,    Any)
       MS(Host,   Host)
@@ -1142,7 +1143,7 @@ ProcBind make(const parser::OmpClause::ProcBind &inp,
   using wrapped = parser::OmpProcBindClause;
 
   CLAUSET_ENUM_CONVERT( //
-      convert, wrapped::Type, ProcBind::AffinityPolicy,
+      convert, wrapped::AffinityPolicy, ProcBind::AffinityPolicy,
       // clang-format off
       MS(Close,    Close)
       MS(Master,   Master)
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index f231290932c122..873f7e93cd15e4 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -255,17 +255,18 @@ TYPE_PARSER(construct<OmpAffinityClause>(
 
 // 2.15.3.1 DEFAULT (PRIVATE | FIRSTPRIVATE | SHARED | NONE)
 TYPE_PARSER(construct<OmpDefaultClause>(
-    "PRIVATE" >> pure(OmpDefaultClause::Type::Private) ||
-    "FIRSTPRIVATE" >> pure(OmpDefaultClause::Type::Firstprivate) ||
-    "SHARED" >> pure(OmpDefaultClause::Type::Shared) ||
-    "NONE" >> pure(OmpDefaultClause::Type::None)))
+    "PRIVATE" >> pure(OmpDefaultClause::DataSharingAttribute::Private) ||
+    "FIRSTPRIVATE" >>
+        pure(OmpDefaultClause::DataSharingAttribute::Firstprivate) ||
+    "SHARED" >> pure(OmpDefaultClause::DataSharingAttribute::Shared) ||
+    "NONE" >> pure(OmpDefaultClause::DataSharingAttribute::None)))
 
 // 2.5 PROC_BIND (MASTER | CLOSE | PRIMARY | SPREAD)
 TYPE_PARSER(construct<OmpProcBindClause>(
-    "CLOSE" >> pure(OmpProcBindClause::Type::Close) ||
-    "MASTER" >> pure(OmpProcBindClause::Type::Master) ||
-    "PRIMARY" >> pure(OmpProcBindClause::Type::Primary) ||
-    "SPREAD" >> pure(OmpProcBindClause::Type::Spread)))
+    "CLOSE" >> pure(OmpProcBindClause::AffinityPolicy::Close) ||
+    "MASTER" >> pure(OmpProcBindClause::AffinityPolicy::Master) ||
+    "PRIMARY" >> pure(OmpProcBindClause::AffinityPolicy::Primary) ||
+    "SPREAD" >> pure(OmpProcBindClause::AffinityPolicy::Spread)))
 
 TYPE_PARSER(construct<OmpMapClause>(
     applyFunction<OmpMapClause>(makeMobClause<true>,
@@ -311,9 +312,9 @@ TYPE_PARSER(construct<OmpDeviceClause>(
 
 // device_type(any | host | nohost)
 TYPE_PARSER(construct<OmpDeviceTypeClause>(
-    "ANY" >> pure(OmpDeviceTypeClause::Type::Any) ||
-    "HOST" >> pure(OmpDeviceTypeClause::Type::Host) ||
-    "NOHOST" >> pure(OmpDeviceTypeClause::Type::Nohost)))
+    "ANY" >> pure(OmpDeviceTypeClause::DeviceTypeDescription::Any) ||
+    "HOST" >> pure(OmpDeviceTypeClause::DeviceTypeDescription::Host) ||
+    "NOHOST" >> pure(OmpDeviceTypeClause::DeviceTypeDescription::Nohost)))
 
 // 2.12 IF (directive-name-modifier: scalar-logical-expr)
 TYPE_PARSER(construct<OmpIfClause>(
@@ -432,9 +433,9 @@ TYPE_PARSER(construct<OmpLastprivateClause>(
 
 // OMP 5.2 11.7.1 BIND ( PARALLEL | TEAMS | THREAD )
 TYPE_PARSER(construct<OmpBindClause>(
-    "PARALLEL" >> pure(OmpBindClause::Type::Parallel) ||
-    "TEAMS" >> pure(OmpBindClause::Type::Teams) ||
-    "THREAD" >> pure(OmpBindClause::Type::Thread)))
+    "PARALLEL" >> pure(OmpBindClause::Binding::Parallel) ||
+    "TEAMS" >> pure(OmpBindClause::Binding::Teams) ||
+    "THREAD" >> pure(OmpBindClause::Binding::Thread)))
 
 TYPE_PARSER(
     "ACQUIRE" >> construct<OmpClause>(construct<OmpClause::Acquire>()) ||
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 192917512c17a2..58aaeb64d7ebc1 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2820,9 +2820,9 @@ class UnparseVisitor {
   WALK_NESTED_ENUM(InquireSpec::LogVar, Kind)
   WALK_NESTED_ENUM(ProcedureStmt, Kind) // R1506
   WALK_NESTED_ENUM(UseStmt, ModuleNature) // R1410
-  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(OmpProcBindClause, AffinityPolicy) // OMP proc_bind
+  WALK_NESTED_ENUM(OmpDefaultClause, DataSharingAttribute) // OMP default
+  WALK_NESTED_ENUM(OmpDefaultmapClause, ImplicitBehavior) // OMP defaultmap
   WALK_NESTED_ENUM(OmpVariableCategory, Value) // OMP variable-category
   WALK_NESTED_ENUM(
       OmpLastprivateClause, LastprivateModifier) // OMP lastprivate-modifier
@@ -2832,7 +2832,8 @@ class UnparseVisitor {
   WALK_NESTED_ENUM(OmpTaskDependenceType, Value) // OMP task-dependence-type
   WALK_NESTED_ENUM(OmpScheduleClause, Kind) // OMP schedule-kind
   WALK_NESTED_ENUM(OmpDeviceClause, DeviceModifier) // OMP device modifier
-  WALK_NESTED_ENUM(OmpDeviceTypeClause, Type) // OMP DEVICE_TYPE
+  WALK_NESTED_ENUM(
+      OmpDeviceTypeClause, DeviceTypeDescription) // OMP device_type
   WALK_NESTED_ENUM(OmpReductionModifier, Value) // OMP reduction-modifier
   WALK_NESTED_ENUM(OmpExpectation, Value) // OMP motion-expectation
   WALK_NESTED_ENUM(OmpIfClause, DirectiveNameModifier) // OMP directive-modifier
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index b49258da506ce6..013dcbaf0b0daa 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -428,7 +428,7 @@ void OmpStructureChecker::HasInvalidLoopBinding(
     for (const auto &clause : clauseList.v) {
       if (const auto *bindClause{
               std::get_if<parser::OmpClause::Bind>(&clause.u)}) {
-        if (bindClause->v.v != parser::OmpBindClause::Type::Teams) {
+        if (bindClause->v.v != parser::OmpBindClause::Binding::Teams) {
           context_.Say(beginDir.source, msg);
         }
       }
@@ -1644,7 +1644,7 @@ void OmpStructureChecker::Leave(const parser::OpenMPDeclareTargetConstruct &x) {
               [&](const parser::OmpClause::DeviceType &deviceTypeClause) {
                 deviceTypeClauseFound = true;
                 if (deviceTypeClause.v.v !=
-                    parser::OmpDeviceTypeClause::Type::Host) {
+                    parser::OmpDeviceTypeClause::DeviceTypeDescription::Host) {
                   // Function / subroutine explicitly marked as runnable by the
                   // target device.
                   deviceConstructFound_ = true;
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 4f56356d879a40..573f216b848ad8 100644
--- a/flang/lib/Semantics/resolve-directives.cpp
+++ b/flang/lib/Semantics/resolve-directives.cpp
@@ -2023,16 +2023,16 @@ bool OmpAttributeVisitor::Pre(const parser::OpenMPAllocatorsConstruct &x) {
 void OmpAttributeVisitor::Post(const parser::OmpDefaultClause &x) {
   if (!dirContext_.empty()) {
     switch (x.v) {
-    case parser::OmpDefaultClause::Type::Private:
+    case parser::OmpDefaultClause::DataSharingAttribute::Private:
       SetContextDefaultDSA(Symbol::Flag::OmpPrivate);
       break;
-    case parser::OmpDefaultClause::Type::Firstprivate:
+    case parser::OmpDefaultClause::DataSharingAttribute::Firstprivate:
       SetContextDefaultDSA(Symbol::Flag::OmpFirstPrivate);
       break;
-    case parser::OmpDefaultClause::Type::Shared:
+    case parser::OmpDefaultClause::DataSharingAttribute::Shared:
       SetContextDefaultDSA(Symbol::Flag::OmpShared);
       break;
-    case parser::OmpDefaultClause::Type::None:
+    case parser::OmpDefaultClause::DataSharingAttribute::None:
       SetContextDefaultDSA(Symbol::Flag::OmpNone);
       break;
     }
diff --git a/flang/test/Parser/OpenMP/declare_target-device_type.f90 b/flang/test/Parser/OpenMP/declare_target-device_type.f90
index 40eb1c2fa4caee..b6903614a628ee 100644
--- a/flang/test/Parser/OpenMP/declare_target-device_type.f90
+++ b/flang/test/Parser/OpenMP/declare_target-device_type.f90
@@ -31,7 +31,7 @@ subroutine openmp_declare_target
     end do
 
 !PARSE-TREE: OpenMPDeclarativeConstruct -> OpenMPDeclareTargetConstruct
-!PARSE-TREE: OmpDeclareTargetSpecifier -> OmpDeclareTargetWithClause -> OmpClauseList -> OmpClause -> DeviceType -> OmpDeviceTypeClause -> Type = Host
-!PARSE-TREE: OmpDeclareTargetSpecifier -> OmpDeclareTargetWithClause -> OmpClauseList -> OmpClause -> DeviceType -> OmpDeviceTypeClause -> Type = Nohost
-!PARSE-TREE: OmpDeclareTargetSpecifier -> OmpDeclareTargetWithClause -> OmpClauseList -> OmpClause -> DeviceType -> OmpDeviceTypeClause -> Type = Any
+!PARSE-TREE: OmpDeclareTargetSpecifier -> OmpDeclareTargetWithClause -> OmpClauseList -> OmpClause -> DeviceType -> OmpDeviceTypeClause -> DeviceTypeDescription = Host
+!PARSE-TREE: OmpDeclareTargetSpecifier -> OmpDeclareTargetWithClause -> OmpClauseList -> OmpClause -> DeviceType -> OmpDeviceTypeClause -> DeviceTypeDescription = Nohost
+!PARSE-TREE: OmpDeclareTargetSpecifier -> OmpDeclareTargetWithClause -> OmpClauseList -> OmpClause -> DeviceType -> OmpDeviceTypeClause -> DeviceTypeDescription = Any
 END subroutine openmp_declare_target
diff --git a/flang/test/Parser/OpenMP/proc-bind.f90 b/flang/test/Parser/OpenMP/proc-bind.f90
index 08bcf69e5e765d..3115b3771f27d6 100644
--- a/flang/test/Parser/OpenMP/proc-bind.f90
+++ b/flang/test/Parser/OpenMP/proc-bind.f90
@@ -6,7 +6,7 @@
 ! PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPBlockConstruct
 ! PARSE-TREE:  OmpBeginBlockDirective
 ! PARSE-TREE:   OmpBlockDirective -> llvm::omp::Directive = parallel
-! PARSE-TREE:   OmpClauseList -> OmpClause -> ProcBind -> OmpProcBindClause -> Type = Primary
+! PARSE-TREE:   OmpClauseList -> OmpClause -> ProcBind -> OmpProcBindClause -> AffinityPolicy = Primary
 subroutine sb1
   !$omp parallel proc_bind(primary)
   print *, "Hello"
diff --git a/flang/test/Parser/OpenMP/target-loop-unparse.f90 b/flang/test/Parser/OpenMP/target-loop-unparse.f90
index b2047070496527..ee0013f6134030 100644
--- a/flang/test/Parser/OpenMP/target-loop-unparse.f90
+++ b/flang/test/Parser/OpenMP/target-loop-unparse.f90
@@ -19,7 +19,7 @@ subroutine test_loop
 
   !PARSE-TREE: OmpBeginLoopDirective
   !PARSE-TREE-NEXT: OmpLoopDirective -> llvm::omp::Directive = loop
-  !PARSE-TREE-NEXT: OmpClauseList -> OmpClause -> Bind -> OmpBindClause -> Type = Thread
+  !PARSE-TREE-NEXT: OmpClauseList -> OmpClause -> Bind -> OmpBindClause -> Binding = Thread
   !CHECK: !$omp loop
   !$omp loop bind(thread)
   do i=1,10

>From 09b1270faf2dc5125c36c7f4cc246b3a922d5b08 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Mon, 25 Nov 2024 14:55:37 -0600
Subject: [PATCH 05/11] [flang][OpenMP] Use new modifiers with
 AFFINITY/ALIGNED/DEVICE

This is a mostly mechanical change from specific modifiers embedded
directly in a clause to the Modifier variant.

Additional comments and references to the OpenMP specs were added.
---
 flang/include/flang/Parser/dump-parse-tree.h  |  7 ++-
 flang/include/flang/Parser/parse-tree.h       | 45 ++++++++++++++-----
 .../flang/Semantics/openmp-modifiers.h        |  2 +
 flang/lib/Lower/OpenMP/Clauses.cpp            | 20 +++++----
 flang/lib/Parser/openmp-parsers.cpp           | 27 ++++++++---
 flang/lib/Parser/unparse.cpp                  | 12 ++---
 flang/lib/Semantics/check-omp-structure.cpp   | 41 ++++++++++-------
 flang/lib/Semantics/openmp-modifiers.cpp      | 32 +++++++++++++
 flang/test/Parser/OpenMP/affinity-clause.f90  |  4 +-
 .../Parser/OpenMP/target_device_parse.f90     | 16 +++----
 .../Parser/OpenMP/target_device_unparse.f90   |  8 ++--
 .../Semantics/OpenMP/clause-validity01.f90    |  4 +-
 12 files changed, 155 insertions(+), 63 deletions(-)

diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index e67b95ca61e8bf..3699aa34f4f8ad 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -484,7 +484,10 @@ class ParseTreeDumper {
   NODE(parser, OmpIteratorSpecifier)
   NODE(parser, OmpIterator)
   NODE(parser, OmpAffinityClause)
+  NODE(OmpAffinityClause, Modifier)
+  NODE(parser, OmpAlignment)
   NODE(parser, OmpAlignedClause)
+  NODE(OmpAlignedClause, Modifier)
   NODE(parser, OmpAtomic)
   NODE(parser, OmpAtomicCapture)
   NODE(OmpAtomicCapture, Stmt1)
@@ -594,7 +597,9 @@ class ParseTreeDumper {
   NODE(OmpScheduleClause, Modifier)
   NODE_ENUM(OmpScheduleClause, Kind)
   NODE(parser, OmpDeviceClause)
-  NODE_ENUM(OmpDeviceClause, DeviceModifier)
+  NODE(OmpDeviceClause, Modifier)
+  NODE(parser, OmpDeviceModifier)
+  NODE_ENUM(OmpDeviceModifier, Value)
   NODE(parser, OmpDeviceTypeClause)
   NODE_ENUM(OmpDeviceTypeClause, DeviceTypeDescription)
   NODE(parser, OmpUpdateClause)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 41b9ad1b7b0d61..2143e280457535 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3457,6 +3457,14 @@ inline namespace modifier {
 //   ENUM_CLASS(Value, Keyword1, Keyword2);
 // };
 
+// Ref: [4.5:72-81], [5.0:110-119], [5.1:134-143], [5.2:169-170]
+//
+// alignment ->
+//    scalar-integer-expression                     // since 4.5
+struct OmpAlignment {
+  WRAPPER_CLASS_BOILERPLATE(OmpAlignment, ScalarIntExpr);
+};
+
 // Ref: [5.1:184-185], [5.2:178-179]
 //
 // align-modifier ->
@@ -3526,6 +3534,15 @@ struct OmpDependenceType {
   WRAPPER_CLASS_BOILERPLATE(OmpDependenceType, Value);
 };
 
+// Ref: [5.0:170-176], [5.1:197-205], [5.2:276-277]
+//
+// device-modifier ->
+//    ANCESTOR | DEVICE_NUM                         // since 5.0
+struct OmpDeviceModifier {
+  ENUM_CLASS(Value, Ancestor, Device_Num)
+  WRAPPER_CLASS_BOILERPLATE(OmpDeviceModifier, Value);
+};
+
 // Ref: [5.1:205-209], [5.2:166-168]
 //
 // motion-modifier ->
@@ -3656,18 +3673,26 @@ struct OmpVariableCategory {
 
 // --- Clauses
 
-// OMP 5.0 2.10.1 affinity([aff-modifier:] locator-list)
-//                aff-modifier: interator-modifier
+// Ref: [5.0:135-140], [5.1:161-166], [5.2:264-265]
+//
+// affinity-clause ->
+//    AFFINITY([aff-modifier:] locator-list)        // since 5.0
+// aff-modifier ->
+//    interator-modifier                            // since 5.0
 struct OmpAffinityClause {
   TUPLE_CLASS_BOILERPLATE(OmpAffinityClause);
-  std::tuple<std::optional<OmpIterator>, OmpObjectList> t;
+  MODIFIER_BOILERPLATE(OmpIterator);
+  std::tuple<MODIFIERS(), OmpObjectList> t;
 };
 
-// 2.8.1 aligned-clause -> ALIGNED (variable-name-list[ : scalar-constant])
+// Ref: [4.5:72-81], [5.0:110-119], [5.1:134-143], [5.2:169-170]
+//
+// aligned-clause ->
+//    ALIGNED(list [: alignment])                   // since 4.5
 struct OmpAlignedClause {
   TUPLE_CLASS_BOILERPLATE(OmpAlignedClause);
-  CharBlock source;
-  std::tuple<OmpObjectList, std::optional<ScalarIntConstantExpr>> t;
+  MODIFIER_BOILERPLATE(OmpAlignment);
+  std::tuple<OmpObjectList, MODIFIERS()> t;
 };
 
 // Ref: [5.0:158-159], [5.1:184-185], [5.2:178-179]
@@ -3806,8 +3831,8 @@ WRAPPER_CLASS(OmpDoacrossClause, OmpDoacross);
 // Ref: [5.0:254-255], [5.1:287-288], [5.2:73]
 //
 // destroy-clause ->
-//    DESTROY |             // since 5.0, until 5.2
-//    DESTROY(variable)     // since 5.2
+//    DESTROY |                                     // since 5.0, until 5.1
+//    DESTROY(variable)                             // since 5.2
 WRAPPER_CLASS(OmpDestroyClause, OmpObject);
 
 // Ref: [5.0:135-140], [5.1:161-166], [5.2:265-266]
@@ -3826,8 +3851,8 @@ struct OmpDetachClause {
 //        scalar-integer-expression)                // since 5.0
 struct OmpDeviceClause {
   TUPLE_CLASS_BOILERPLATE(OmpDeviceClause);
-  ENUM_CLASS(DeviceModifier, Ancestor, Device_Num)
-  std::tuple<std::optional<DeviceModifier>, ScalarIntExpr> t;
+  MODIFIER_BOILERPLATE(OmpDeviceModifier);
+  std::tuple<MODIFIERS(), ScalarIntExpr> t;
 };
 
 // Ref: [5.0:180-185], [5.1:210-216], [5.2:275]
diff --git a/flang/include/flang/Semantics/openmp-modifiers.h b/flang/include/flang/Semantics/openmp-modifiers.h
index a6316cf7aba564..fab4b38090b049 100644
--- a/flang/include/flang/Semantics/openmp-modifiers.h
+++ b/flang/include/flang/Semantics/openmp-modifiers.h
@@ -67,11 +67,13 @@ template <typename SpecificTy> const OmpModifierDescriptor &OmpGetDescriptor();
 #define DECLARE_DESCRIPTOR(name) \
   template <> const OmpModifierDescriptor &OmpGetDescriptor<name>()
 
+DECLARE_DESCRIPTOR(parser::OmpAlignment);
 DECLARE_DESCRIPTOR(parser::OmpAlignModifier);
 DECLARE_DESCRIPTOR(parser::OmpAllocatorComplexModifier);
 DECLARE_DESCRIPTOR(parser::OmpAllocatorSimpleModifier);
 DECLARE_DESCRIPTOR(parser::OmpChunkModifier);
 DECLARE_DESCRIPTOR(parser::OmpDependenceType);
+DECLARE_DESCRIPTOR(parser::OmpDeviceModifier);
 DECLARE_DESCRIPTOR(parser::OmpExpectation);
 DECLARE_DESCRIPTOR(parser::OmpIterator);
 DECLARE_DESCRIPTOR(parser::OmpLinearModifier);
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index 6d09cab700fd6f..2792232253879f 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -384,11 +384,12 @@ Absent make(const parser::OmpClause::Absent &inp,
 Affinity make(const parser::OmpClause::Affinity &inp,
               semantics::SemanticsContext &semaCtx) {
   // inp.v -> parser::OmpAffinityClause
-  auto &t0 = std::get<std::optional<parser::OmpIterator>>(inp.v.t);
+  auto &mods = semantics::OmpGetModifiers(inp.v);
+  auto *m0 = semantics::OmpGetUniqueModifier<parser::OmpIterator>(mods);
   auto &t1 = std::get<parser::OmpObjectList>(inp.v.t);
 
   auto &&maybeIter =
-      maybeApply([&](auto &&s) { return makeIterator(s, semaCtx); }, t0);
+      m0 ? makeIterator(*m0, semaCtx) : std::optional<Iterator>{};
 
   return Affinity{{/*Iterator=*/std::move(maybeIter),
                    /*LocatorList=*/makeObjects(t1, semaCtx)}};
@@ -403,11 +404,12 @@ Align make(const parser::OmpClause::Align &inp,
 Aligned make(const parser::OmpClause::Aligned &inp,
              semantics::SemanticsContext &semaCtx) {
   // inp.v -> parser::OmpAlignedClause
+  auto &mods = semantics::OmpGetModifiers(inp.v);
   auto &t0 = std::get<parser::OmpObjectList>(inp.v.t);
-  auto &t1 = std::get<std::optional<parser::ScalarIntConstantExpr>>(inp.v.t);
+  auto *m1 = semantics::OmpGetUniqueModifier<parser::OmpAlignment>(mods);
 
   return Aligned{{
-      /*Alignment=*/maybeApply(makeExprFn(semaCtx), t1),
+      /*Alignment=*/maybeApplyToV(makeExprFn(semaCtx), m1),
       /*List=*/makeObjects(t0, semaCtx),
   }};
 }
@@ -659,18 +661,18 @@ Detach make(const parser::OmpClause::Detach &inp,
 Device make(const parser::OmpClause::Device &inp,
             semantics::SemanticsContext &semaCtx) {
   // inp.v -> parser::OmpDeviceClause
-  using wrapped = parser::OmpDeviceClause;
-
   CLAUSET_ENUM_CONVERT( //
-      convert, parser::OmpDeviceClause::DeviceModifier, Device::DeviceModifier,
+      convert, parser::OmpDeviceModifier::Value, Device::DeviceModifier,
       // clang-format off
       MS(Ancestor,   Ancestor)
       MS(Device_Num, DeviceNum)
       // clang-format on
   );
-  auto &t0 = std::get<std::optional<wrapped::DeviceModifier>>(inp.v.t);
+
+  auto &mods = semantics::OmpGetModifiers(inp.v);
+  auto *m0 = semantics::OmpGetUniqueModifier<parser::OmpDeviceModifier>(mods);
   auto &t1 = std::get<parser::ScalarIntExpr>(inp.v.t);
-  return Device{{/*DeviceModifier=*/maybeApply(convert, t0),
+  return Device{{/*DeviceModifier=*/maybeApplyToV(convert, m0),
                  /*DeviceDescription=*/makeExpr(t1, semaCtx)}};
 }
 
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 873f7e93cd15e4..5b61d053ad1622 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -91,6 +91,8 @@ static TypeDeclarationStmt makeIterSpecDecl(std::list<ObjectName> &&names) {
 
 // --- Parsers for clause modifiers -----------------------------------
 
+TYPE_PARSER(construct<OmpAlignment>(scalarIntExpr))
+
 TYPE_PARSER(construct<OmpAlignModifier>( //
     "ALIGN" >> parenthesized(scalarIntExpr)))
 
@@ -106,6 +108,10 @@ TYPE_PARSER(construct<OmpDependenceType>(
     "SINK" >> pure(OmpDependenceType::Value::Sink) ||
     "SOURCE" >> pure(OmpDependenceType::Value::Source)))
 
+TYPE_PARSER(construct<OmpDeviceModifier>(
+    "ANCESTOR" >> pure(OmpDeviceModifier::Value::Ancestor) ||
+    "DEVICE_NUM" >> pure(OmpDeviceModifier::Value::Device_Num)))
+
 TYPE_PARSER(construct<OmpExpectation>( //
     "PRESENT" >> pure(OmpExpectation::Value::Present)))
 
@@ -191,6 +197,12 @@ TYPE_PARSER(construct<OmpVariableCategory>(
     "SCALAR" >> pure(OmpVariableCategory::Value::Scalar)))
 
 // This could be auto-generated.
+TYPE_PARSER(
+    sourced(construct<OmpAffinityClause::Modifier>(Parser<OmpIterator>{})))
+
+TYPE_PARSER(
+    sourced(construct<OmpAlignedClause::Modifier>(Parser<OmpAlignment>{})))
+
 TYPE_PARSER(sourced(construct<OmpAllocateClause::Modifier>(sourced(
     construct<OmpAllocateClause::Modifier>(Parser<OmpAlignModifier>{}) ||
     construct<OmpAllocateClause::Modifier>(
@@ -201,6 +213,9 @@ TYPE_PARSER(sourced(construct<OmpAllocateClause::Modifier>(sourced(
 TYPE_PARSER(sourced(
     construct<OmpDefaultmapClause::Modifier>(Parser<OmpVariableCategory>{})))
 
+TYPE_PARSER(
+    sourced(construct<OmpDeviceClause::Modifier>(Parser<OmpDeviceModifier>{})))
+
 TYPE_PARSER(sourced(construct<OmpFromClause::Modifier>(
     sourced(construct<OmpFromClause::Modifier>(Parser<OmpExpectation>{}) ||
         construct<OmpFromClause::Modifier>(Parser<OmpMapper>{}) ||
@@ -251,7 +266,8 @@ static inline MOBClause makeMobClause(
 // [5.0] 2.10.1 affinity([aff-modifier:] locator-list)
 //              aff-modifier: interator-modifier
 TYPE_PARSER(construct<OmpAffinityClause>(
-    maybe(Parser<OmpIterator>{} / ":"), Parser<OmpObjectList>{}))
+    maybe(nonemptyList(Parser<OmpAffinityClause::Modifier>{}) / ":"),
+    Parser<OmpObjectList>{}))
 
 // 2.15.3.1 DEFAULT (PRIVATE | FIRSTPRIVATE | SHARED | NONE)
 TYPE_PARSER(construct<OmpDefaultClause>(
@@ -304,10 +320,7 @@ TYPE_PARSER(construct<OmpScheduleClause>(
 
 // device([ device-modifier :] scalar-integer-expression)
 TYPE_PARSER(construct<OmpDeviceClause>(
-    maybe(
-        ("ANCESTOR" >> pure(OmpDeviceClause::DeviceModifier::Ancestor) ||
-            "DEVICE_NUM" >> pure(OmpDeviceClause::DeviceModifier::Device_Num)) /
-        ":"),
+    maybe(nonemptyList(Parser<OmpDeviceClause::Modifier>{}) / ":"),
     scalarIntExpr))
 
 // device_type(any | host | nohost)
@@ -401,8 +414,8 @@ TYPE_CONTEXT_PARSER("Omp LINEAR clause"_en_US,
 TYPE_PARSER(construct<OmpDetachClause>(Parser<OmpObject>{}))
 
 // 2.8.1 ALIGNED (list: alignment)
-TYPE_PARSER(construct<OmpAlignedClause>(
-    Parser<OmpObjectList>{}, maybe(":" >> scalarIntConstantExpr)))
+TYPE_PARSER(construct<OmpAlignedClause>(Parser<OmpObjectList>{},
+    maybe(":" >> nonemptyList(Parser<OmpAlignedClause::Modifier>{}))))
 
 TYPE_PARSER(construct<OmpUpdateClause>(
     construct<OmpUpdateClause>(Parser<OmpDependenceType>{}) ||
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 58aaeb64d7ebc1..80ebd692bd1192 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2107,17 +2107,19 @@ class UnparseVisitor {
     Walk(",", std::get<std::optional<ScalarIntExpr>>(x.t));
   }
   void Unparse(const OmpDeviceClause &x) {
-    Walk(std::get<std::optional<OmpDeviceClause::DeviceModifier>>(x.t), ":");
+    using Modifier = OmpDeviceClause::Modifier;
+    Walk(std::get<std::optional<std::list<Modifier>>>(x.t), ": ");
     Walk(std::get<ScalarIntExpr>(x.t));
   }
   void Unparse(const OmpAffinityClause &x) {
-    Walk(std::get<std::optional<OmpIterator>>(x.t), ":");
+    using Modifier = OmpAffinityClause::Modifier;
+    Walk(std::get<std::optional<std::list<Modifier>>>(x.t), ": ");
     Walk(std::get<OmpObjectList>(x.t));
   }
   void Unparse(const OmpAlignedClause &x) {
+    using Modifier = OmpAlignedClause::Modifier;
     Walk(std::get<OmpObjectList>(x.t));
-    Put(",");
-    Walk(std::get<std::optional<ScalarIntConstantExpr>>(x.t));
+    Walk(": ", std::get<std::optional<std::list<Modifier>>>(x.t));
   }
   void Unparse(const OmpFromClause &x) {
     using Modifier = OmpFromClause::Modifier;
@@ -2831,7 +2833,7 @@ class UnparseVisitor {
   WALK_NESTED_ENUM(OmpOrderingModifier, Value) // OMP ordering-modifier
   WALK_NESTED_ENUM(OmpTaskDependenceType, Value) // OMP task-dependence-type
   WALK_NESTED_ENUM(OmpScheduleClause, Kind) // OMP schedule-kind
-  WALK_NESTED_ENUM(OmpDeviceClause, DeviceModifier) // OMP device modifier
+  WALK_NESTED_ENUM(OmpDeviceModifier, Value) // OMP device modifier
   WALK_NESTED_ENUM(
       OmpDeviceTypeClause, DeviceTypeDescription) // OMP device_type
   WALK_NESTED_ENUM(OmpReductionModifier, Value) // OMP reduction-modifier
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 013dcbaf0b0daa..ebcafb6a0e354e 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -3372,10 +3372,15 @@ void OmpStructureChecker::Leave(const parser::OmpAtomic &) {
 // generalized restrictions.
 void OmpStructureChecker::Enter(const parser::OmpClause::Aligned &x) {
   CheckAllowedClause(llvm::omp::Clause::OMPC_aligned);
-
-  if (const auto &expr{
-          std::get<std::optional<parser::ScalarIntConstantExpr>>(x.v.t)}) {
-    RequiresConstantPositiveParameter(llvm::omp::Clause::OMPC_aligned, *expr);
+  if (OmpVerifyModifiers(
+          x.v, llvm::omp::OMPC_aligned, GetContext().clauseSource, context_)) {
+    auto &modifiers{OmpGetModifiers(x.v)};
+    if (auto *align{OmpGetUniqueModifier<parser::OmpAlignment>(modifiers)}) {
+      if (const auto &v{GetIntValue(align->v)}; !v || *v <= 0) {
+        context_.Say(OmpGetModifierSource(modifiers, align),
+            "The alignment value should be a constant positive integer"_err_en_US);
+      }
+    }
   }
   // 2.8.1 TODO: list-item attribute check
 }
@@ -3597,19 +3602,25 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Schedule &x) {
 
 void OmpStructureChecker::Enter(const parser::OmpClause::Device &x) {
   CheckAllowedClause(llvm::omp::Clause::OMPC_device);
-  const parser::OmpDeviceClause &deviceClause = x.v;
-  const auto &device{std::get<1>(deviceClause.t)};
+  const parser::OmpDeviceClause &deviceClause{x.v};
+  const auto &device{std::get<parser::ScalarIntExpr>(deviceClause.t)};
   RequiresPositiveParameter(
       llvm::omp::Clause::OMPC_device, device, "device expression");
-  std::optional<parser::OmpDeviceClause::DeviceModifier> modifier =
-      std::get<0>(deviceClause.t);
-  if (modifier &&
-      *modifier == parser::OmpDeviceClause::DeviceModifier::Ancestor) {
-    if (GetContext().directive != llvm::omp::OMPD_target) {
-      context_.Say(GetContext().clauseSource,
-          "The ANCESTOR device-modifier must not appear on the DEVICE clause on"
-          " any directive other than the TARGET construct. Found on %s construct."_err_en_US,
-          parser::ToUpperCaseLetters(getDirectiveName(GetContext().directive)));
+  llvm::omp::Directive dir{GetContext().directive};
+
+  if (OmpVerifyModifiers(deviceClause, llvm::omp::OMPC_device,
+          GetContext().clauseSource, context_)) {
+    auto &modifiers{OmpGetModifiers(deviceClause)};
+
+    if (auto *deviceMod{
+            OmpGetUniqueModifier<parser::OmpDeviceModifier>(modifiers)}) {
+      using Value = parser::OmpDeviceModifier::Value;
+      if (dir != llvm::omp::OMPD_target && deviceMod->v == Value::Ancestor) {
+        auto name{OmpGetDescriptor<parser::OmpDeviceModifier>().name};
+        context_.Say(OmpGetModifierSource(modifiers, deviceMod),
+            "The ANCESTOR %s must not appear on the DEVICE clause on any directive other than the TARGET construct. Found on %s construct."_err_en_US,
+            name.str(), parser::ToUpperCaseLetters(getDirectiveName(dir)));
+      }
     }
   }
 }
diff --git a/flang/lib/Semantics/openmp-modifiers.cpp b/flang/lib/Semantics/openmp-modifiers.cpp
index 18863b7b904a4a..97227bfef0ea54 100644
--- a/flang/lib/Semantics/openmp-modifiers.cpp
+++ b/flang/lib/Semantics/openmp-modifiers.cpp
@@ -74,6 +74,22 @@ unsigned OmpModifierDescriptor::since(llvm::omp::Clause id) const {
 // Note: The intent for these functions is to have them be automatically-
 // generated in the future.
 
+template <>
+const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpAlignment>() {
+  static const OmpModifierDescriptor desc{
+      /*name=*/"alignment",
+      /*props=*/
+      {
+          {45, {OmpProperty::Unique, OmpProperty::Ultimate, OmpProperty::Post}},
+      },
+      /*clauses=*/
+      {
+          {45, {Clause::OMPC_aligned}},
+      },
+  };
+  return desc;
+}
+
 template <>
 const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpAlignModifier>() {
   static const OmpModifierDescriptor desc{
@@ -158,6 +174,22 @@ const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpDependenceType>() {
   return desc;
 }
 
+template <>
+const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpDeviceModifier>() {
+  static const OmpModifierDescriptor desc{
+      /*name=*/"device-modifier",
+      /*props=*/
+      {
+          {45, {OmpProperty::Unique}},
+      },
+      /*clauses=*/
+      {
+          {45, {Clause::OMPC_device}},
+      },
+  };
+  return desc;
+}
+
 template <>
 const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpExpectation>() {
   static const OmpModifierDescriptor desc{
diff --git a/flang/test/Parser/OpenMP/affinity-clause.f90 b/flang/test/Parser/OpenMP/affinity-clause.f90
index 5e9e0a2194babd..642af6aeb7e491 100644
--- a/flang/test/Parser/OpenMP/affinity-clause.f90
+++ b/flang/test/Parser/OpenMP/affinity-clause.f90
@@ -55,7 +55,7 @@ subroutine f02(x)
 
 !UNPARSE: SUBROUTINE f02 (x)
 !UNPARSE:  INTEGER x(10_4)
-!UNPARSE: !$OMP TASK  AFFINITY(ITERATOR(INTEGER i = 1_4:3_4):x(i))
+!UNPARSE: !$OMP TASK  AFFINITY(ITERATOR(INTEGER i = 1_4:3_4): x(i))
 !UNPARSE:   x=x+1_4
 !UNPARSE: !$OMP END TASK
 !UNPARSE: END SUBROUTINE
@@ -63,7 +63,7 @@ subroutine f02(x)
 !PARSE-TREE: OmpBeginBlockDirective
 !PARSE-TREE: | OmpBlockDirective -> llvm::omp::Directive = task
 !PARSE-TREE: | OmpClauseList -> OmpClause -> Affinity -> OmpAffinityClause
-!PARSE-TREE: | | OmpIterator -> OmpIteratorSpecifier
+!PARSE-TREE: | | Modifier -> OmpIterator -> OmpIteratorSpecifier
 !PARSE-TREE: | | | TypeDeclarationStmt
 !PARSE-TREE: | | | | DeclarationTypeSpec -> IntrinsicTypeSpec -> IntegerTypeSpec ->
 !PARSE-TREE: | | | | EntityDecl
diff --git a/flang/test/Parser/OpenMP/target_device_parse.f90 b/flang/test/Parser/OpenMP/target_device_parse.f90
index 30464c5b08ab8b..7f5bee3793b2e2 100644
--- a/flang/test/Parser/OpenMP/target_device_parse.f90
+++ b/flang/test/Parser/OpenMP/target_device_parse.f90
@@ -96,7 +96,7 @@ PROGRAM main
 !------------------------------------------------------
 ! Check Device Ancestor clause with a constant argument
 !------------------------------------------------------
-!CHECK: !$OMP TARGET DEVICE(ANCESTOR:1)
+!CHECK: !$OMP TARGET DEVICE(ANCESTOR: 1)
 !$OMP TARGET DEVICE(ANCESTOR: 1)
   M = M + 1
 !CHECK: !$OMP END TARGET
@@ -105,7 +105,7 @@ PROGRAM main
 !PARSE-TREE: OmpBeginBlockDirective
 !PARSE-TREE: OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE: OmpClauseList -> OmpClause -> Device -> OmpDeviceClause
-!PARSE-TREE: DeviceModifier = Ancestor
+!PARSE-TREE: OmpDeviceModifier -> Value = Ancestor
 !PARSE-TREE: Scalar -> Integer -> Expr = '1_4'
 !PARSE-TREE: LiteralConstant -> IntLiteralConstant = '1'
 !PARSE-TREE: OmpEndBlockDirective
@@ -116,7 +116,7 @@ PROGRAM main
 !--------------------------------------------------------
 ! Check Device Devive-Num clause with a constant argument
 !--------------------------------------------------------
-!CHECK: !$OMP TARGET DEVICE(DEVICE_NUM:2)
+!CHECK: !$OMP TARGET DEVICE(DEVICE_NUM: 2)
 !$OMP TARGET DEVICE(DEVICE_NUM: 2)
   M = M + 1
 !CHECK: !$OMP END TARGET
@@ -125,7 +125,7 @@ PROGRAM main
 !PARSE-TREE: OmpBeginBlockDirective
 !PARSE-TREE: OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE: OmpClauseList -> OmpClause -> Device -> OmpDeviceClause
-!PARSE-TREE: DeviceModifier = Device_Num
+!PARSE-TREE: OmpDeviceModifier -> Value = Device_Num
 !PARSE-TREE: Scalar -> Integer -> Expr = '2_4'
 !PARSE-TREE: LiteralConstant -> IntLiteralConstant = '2'
 !PARSE-TREE: OmpEndBlockDirective
@@ -136,7 +136,7 @@ PROGRAM main
 !-------------------------------------------------------------------
 ! Check Device Ancestor clause with a variable expression argument
 !-------------------------------------------------------------------
-!CHECK: !$OMP TARGET DEVICE(ANCESTOR:X+Y)
+!CHECK: !$OMP TARGET DEVICE(ANCESTOR: X+Y)
 !$OMP TARGET DEVICE(ANCESTOR: X + Y)
   M = M + 1
 !CHECK: !$OMP END TARGET
@@ -145,7 +145,7 @@ PROGRAM main
 !PARSE-TREE: OmpBeginBlockDirective
 !PARSE-TREE: OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE: OmpClauseList -> OmpClause -> Device -> OmpDeviceClause
-!PARSE-TREE: DeviceModifier = Ancestor
+!PARSE-TREE: OmpDeviceModifier -> Value = Ancestor
 !PARSE-TREE: Scalar -> Integer -> Expr = 'x+y'
 !PARSE-TREE: Add
 !PARSE-TREE: Expr = 'x'
@@ -160,7 +160,7 @@ PROGRAM main
 !-------------------------------------------------------------------
 ! Check Device Devive-Num clause with a variable expression argument
 !-------------------------------------------------------------------
-!CHECK: !$OMP TARGET DEVICE(DEVICE_NUM:X-Y)
+!CHECK: !$OMP TARGET DEVICE(DEVICE_NUM: X-Y)
 !$OMP TARGET DEVICE(DEVICE_NUM: X - Y)
   M = M + 1
 !CHECK: !$OMP END TARGET
@@ -169,7 +169,7 @@ PROGRAM main
 !PARSE-TREE: OmpBeginBlockDirective
 !PARSE-TREE: OmpBlockDirective -> llvm::omp::Directive = target
 !PARSE-TREE: OmpClauseList -> OmpClause -> Device -> OmpDeviceClause
-!PARSE-TREE: DeviceModifier = Device_Num
+!PARSE-TREE: OmpDeviceModifier -> Value = Device_Num
 !PARSE-TREE: Scalar -> Integer -> Expr = 'x-y'
 !PARSE-TREE: Subtract
 !PARSE-TREE: Expr = 'x'
diff --git a/flang/test/Parser/OpenMP/target_device_unparse.f90 b/flang/test/Parser/OpenMP/target_device_unparse.f90
index 605c2baf1b06bd..403b64887e747c 100644
--- a/flang/test/Parser/OpenMP/target_device_unparse.f90
+++ b/flang/test/Parser/OpenMP/target_device_unparse.f90
@@ -45,7 +45,7 @@ PROGRAM main
 !--------------------------------------------
 ! Ancestor followed by constant argument
 !--------------------------------------------
-!CHECK: !$OMP TARGET DEVICE(ANCESTOR:0)
+!CHECK: !$OMP TARGET DEVICE(ANCESTOR: 0)
 !CHECK: !$OMP END TARGET
 !$OMP TARGET DEVICE(ANCESTOR: 0)
   M = M + 1
@@ -54,7 +54,7 @@ PROGRAM main
 !--------------------------------------------
 ! Device_Num followed by constant argument
 !--------------------------------------------
-!CHECK: !$OMP TARGET DEVICE(DEVICE_NUM:1)
+!CHECK: !$OMP TARGET DEVICE(DEVICE_NUM: 1)
 !CHECK: !$OMP END TARGET
 !$OMP TARGET DEVICE(DEVICE_NUM: 1)
   M = M + 1
@@ -63,7 +63,7 @@ PROGRAM main
 !--------------------------------------------
 ! Ancestor followed by variable expression argument
 !--------------------------------------------
-!CHECK: !$OMP TARGET DEVICE(ANCESTOR:X+Y)
+!CHECK: !$OMP TARGET DEVICE(ANCESTOR: X+Y)
 !CHECK: !$OMP END TARGET
 !$OMP TARGET DEVICE(ANCESTOR: X + Y)
   M = M + 1
@@ -72,7 +72,7 @@ PROGRAM main
 !--------------------------------------------
 ! Device_Num followed by variable expression argument
 !--------------------------------------------
-!CHECK: !$OMP TARGET DEVICE(DEVICE_NUM:X-Y)
+!CHECK: !$OMP TARGET DEVICE(DEVICE_NUM: X-Y)
 !CHECK: !$OMP END TARGET
 !$OMP TARGET DEVICE(DEVICE_NUM: X - Y)
   M = M + 1
diff --git a/flang/test/Semantics/OpenMP/clause-validity01.f90 b/flang/test/Semantics/OpenMP/clause-validity01.f90
index 8dd6d10200cd3d..bc9d2d37060fc2 100644
--- a/flang/test/Semantics/OpenMP/clause-validity01.f90
+++ b/flang/test/Semantics/OpenMP/clause-validity01.f90
@@ -376,7 +376,7 @@
      a = 3.14
   enddo
 
-  !ERROR: The parameter of the ALIGNED clause must be a constant positive integer expression
+  !ERROR: The alignment value should be a constant positive integer
   !$omp simd aligned(cpt:-2)
   do i = 1, N
      a = 3.14
@@ -572,7 +572,7 @@
 
   allocate(allc)
   !ERROR: The parameter of the SIMDLEN clause must be a constant positive integer expression
-  !ERROR: The parameter of the ALIGNED clause must be a constant positive integer expression
+  !ERROR: The alignment value should be a constant positive integer
   !$omp taskloop simd simdlen(-1) aligned(allc:-2)
   do i = 1, N
      allc = 3.14

>From d520f1e23c72190ce807306f05f899f106be3070 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 26 Nov 2024 16:03:56 -0600
Subject: [PATCH 06/11] fix examples

---
 flang/examples/FeatureList/FeatureList.cpp              | 6 +++---
 flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp | 8 +++++---
 flang/examples/FlangOmpReport/FlangOmpReportVisitor.h   | 6 +++---
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/flang/examples/FeatureList/FeatureList.cpp b/flang/examples/FeatureList/FeatureList.cpp
index f7086b9e82b8d0..ab309b15e45416 100644
--- a/flang/examples/FeatureList/FeatureList.cpp
+++ b/flang/examples/FeatureList/FeatureList.cpp
@@ -465,7 +465,7 @@ struct NodeVisitor {
   READ_FEATURE(OmpDeclareTargetWithClause)
   READ_FEATURE(OmpDeclareTargetWithList)
   READ_FEATURE(OmpDefaultClause)
-  READ_FEATURE(OmpDefaultClause::Type)
+  READ_FEATURE(OmpDefaultClause::DataSharingAttribute)
   READ_FEATURE(OmpDefaultmapClause)
   READ_FEATURE(OmpDefaultmapClause::ImplicitBehavior)
   READ_FEATURE(OmpVariableCategory::Value)
@@ -508,7 +508,7 @@ struct NodeVisitor {
   READ_FEATURE(OmpOrderModifier)
   READ_FEATURE(OmpOrderModifier::Value)
   READ_FEATURE(OmpProcBindClause)
-  READ_FEATURE(OmpProcBindClause::Type)
+  READ_FEATURE(OmpProcBindClause::AffinityPolicy)
   READ_FEATURE(OmpReductionClause)
   READ_FEATURE(OmpInReductionClause)
   READ_FEATURE(OmpReductionCombiner)
@@ -526,7 +526,7 @@ struct NodeVisitor {
   READ_FEATURE(OmpDeviceClause)
   READ_FEATURE(OmpDeviceClause::DeviceModifier)
   READ_FEATURE(OmpDeviceTypeClause)
-  READ_FEATURE(OmpDeviceTypeClause::Type)
+  READ_FEATURE(OmpDeviceTypeClause::DeviceTypeDescription)
   READ_FEATURE(OmpChunkModifier)
   READ_FEATURE(OmpChunkModifier::Value)
   READ_FEATURE(OmpOrderingModifier)
diff --git a/flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp b/flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp
index 5bd8c761992781..afabd564ad5bdc 100644
--- a/flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp
+++ b/flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp
@@ -190,15 +190,17 @@ void OpenMPCounterVisitor::PostConstructsCommon() {
   delete curConstruct;
 }
 
-void OpenMPCounterVisitor::Post(const OmpProcBindClause::Type &c) {
+void OpenMPCounterVisitor::Post(const OmpProcBindClause::AffinityPolicy &c) {
   clauseDetails +=
       "type=" + std::string{OmpProcBindClause::EnumToString(c)} + ";";
 }
-void OpenMPCounterVisitor::Post(const OmpDefaultClause::Type &c) {
+void OpenMPCounterVisitor::Post(
+    const OmpDefaultClause::DataSharingAttribute &c) {
   clauseDetails +=
       "type=" + std::string{OmpDefaultClause::EnumToString(c)} + ";";
 }
-void OpenMPCounterVisitor::Post(const OmpDeviceTypeClause::Type &c) {
+void OpenMPCounterVisitor::Post(
+    const OmpDeviceTypeClause::DeviceTypeDescription &c) {
   clauseDetails +=
       "type=" + std::string{OmpDeviceTypeClause::EnumToString(c)} + ";";
 }
diff --git a/flang/examples/FlangOmpReport/FlangOmpReportVisitor.h b/flang/examples/FlangOmpReport/FlangOmpReportVisitor.h
index 7e9ae94bef2973..ed202e8ed2a4c7 100644
--- a/flang/examples/FlangOmpReport/FlangOmpReportVisitor.h
+++ b/flang/examples/FlangOmpReport/FlangOmpReportVisitor.h
@@ -66,11 +66,11 @@ struct OpenMPCounterVisitor {
   void Post(const OpenMPConstruct &);
   void PostConstructsCommon();
 
-  void Post(const OmpProcBindClause::Type &c);
-  void Post(const OmpDefaultClause::Type &c);
+  void Post(const OmpProcBindClause::AffinityPolicy &c);
+  void Post(const OmpDefaultClause::DataSharingAttribute &c);
   void Post(const OmpDefaultmapClause::ImplicitBehavior &c);
   void Post(const OmpVariableCategory::Value &c);
-  void Post(const OmpDeviceTypeClause::Type &c);
+  void Post(const OmpDeviceTypeClause::DeviceTypeDescription &c);
   void Post(const OmpChunkModifier::Value &c);
   void Post(const OmpLinearModifier::Value &c);
   void Post(const OmpOrderingModifier::Value &c);

>From 923638f04160c6c4a5fdf7afa309dcea3ec9fb7e Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Tue, 26 Nov 2024 16:18:27 -0600
Subject: [PATCH 07/11] fix examples

---
 flang/examples/FeatureList/FeatureList.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/flang/examples/FeatureList/FeatureList.cpp b/flang/examples/FeatureList/FeatureList.cpp
index ab309b15e45416..2e90f19dc2e62c 100644
--- a/flang/examples/FeatureList/FeatureList.cpp
+++ b/flang/examples/FeatureList/FeatureList.cpp
@@ -523,8 +523,9 @@ struct NodeVisitor {
   READ_FEATURE(OmpScheduleClause)
   READ_FEATURE(OmpScheduleClause::Kind)
   READ_FEATURE(OmpScheduleClause::Modifier)
+  READ_FEATURE(OmpDeviceModifier)
   READ_FEATURE(OmpDeviceClause)
-  READ_FEATURE(OmpDeviceClause::DeviceModifier)
+  READ_FEATURE(OmpDeviceClause::Modifier)
   READ_FEATURE(OmpDeviceTypeClause)
   READ_FEATURE(OmpDeviceTypeClause::DeviceTypeDescription)
   READ_FEATURE(OmpChunkModifier)

>From 43f008a7f8b7a6377f6cb7f3ea4cc20394c2d79d Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Wed, 27 Nov 2024 08:34:33 -0600
Subject: [PATCH 08/11] [flang][OpenMP] Use new modifiers in
 DEPEND/GRAINSIZE/NUM_TASKS

The usual changes, added more references to OpenMP specs.
---
 flang/examples/FeatureList/FeatureList.cpp    |  6 +-
 flang/include/flang/Parser/dump-parse-tree.h  |  7 ++-
 flang/include/flang/Parser/parse-tree.h       | 42 ++++++++++----
 .../flang/Semantics/openmp-modifiers.h        |  1 +
 flang/lib/Lower/OpenMP/Clauses.cpp            | 55 +++++++++----------
 flang/lib/Lower/OpenMP/Clauses.h              |  1 +
 flang/lib/Parser/openmp-parsers.cpp           | 31 +++++++++--
 flang/lib/Parser/parse-tree.cpp               | 13 ++++-
 flang/lib/Parser/unparse.cpp                  | 16 +++---
 flang/lib/Semantics/check-omp-structure.cpp   | 15 ++---
 flang/lib/Semantics/openmp-modifiers.cpp      | 21 ++++++-
 flang/test/Parser/OpenMP/depobj-construct.f90 |  4 +-
 flang/test/Parser/OpenMP/taskloop.f90         |  8 +--
 flang/test/Semantics/OpenMP/depend05.f90      |  2 +-
 llvm/include/llvm/Frontend/OpenMP/ClauseT.h   |  5 +-
 15 files changed, 146 insertions(+), 81 deletions(-)

diff --git a/flang/examples/FeatureList/FeatureList.cpp b/flang/examples/FeatureList/FeatureList.cpp
index 2e90f19dc2e62c..c5cb8c8fdf40bb 100644
--- a/flang/examples/FeatureList/FeatureList.cpp
+++ b/flang/examples/FeatureList/FeatureList.cpp
@@ -488,7 +488,9 @@ struct NodeVisitor {
   READ_FEATURE(OmpEndLoopDirective)
   READ_FEATURE(OmpEndSectionsDirective)
   READ_FEATURE(OmpGrainsizeClause)
-  READ_FEATURE(OmpGrainsizeClause::Prescriptiveness)
+  READ_FEATURE(OmpGrainsizeClause::Modifier)
+  READ_FEATURE(OmpPrescriptiveness)
+  READ_FEATURE(OmpPrescriptiveness::Value)
   READ_FEATURE(OmpIfClause)
   READ_FEATURE(OmpIfClause::DirectiveNameModifier)
   READ_FEATURE(OmpLinearClause)
@@ -500,7 +502,7 @@ struct NodeVisitor {
   READ_FEATURE(OmpMapClause)
   READ_FEATURE(OmpMapClause::Modifier)
   READ_FEATURE(OmpNumTasksClause)
-  READ_FEATURE(OmpNumTasksClause::Prescriptiveness)
+  READ_FEATURE(OmpNumTasksClause::Modifier)
   READ_FEATURE(OmpObject)
   READ_FEATURE(OmpObjectList)
   READ_FEATURE(OmpOrderClause)
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 3699aa34f4f8ad..1ec38de29b85d6 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -534,6 +534,7 @@ class ParseTreeDumper {
   NODE(OmpDoacross, Source)
   NODE(parser, OmpDependClause)
   NODE(OmpDependClause, TaskDep)
+  NODE(OmpDependClause::TaskDep, Modifier)
   NODE(parser, OmpDetachClause)
   NODE(parser, OmpDoacrossClause)
   NODE(parser, OmpDestroyClause)
@@ -572,9 +573,11 @@ class ParseTreeDumper {
   NODE(parser, OmpOrderModifier)
   NODE_ENUM(OmpOrderModifier, Value)
   NODE(parser, OmpGrainsizeClause)
-  NODE_ENUM(OmpGrainsizeClause, Prescriptiveness)
+  NODE(OmpGrainsizeClause, Modifier)
+  NODE(parser, OmpPrescriptiveness)
+  NODE_ENUM(OmpPrescriptiveness, Value)
   NODE(parser, OmpNumTasksClause)
-  NODE_ENUM(OmpNumTasksClause, Prescriptiveness)
+  NODE(OmpNumTasksClause, Modifier)
   NODE(parser, OmpBindClause)
   NODE_ENUM(OmpBindClause, Binding)
   NODE(parser, OmpProcBindClause)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 2143e280457535..c00560b1f1726a 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3627,6 +3627,15 @@ struct OmpOrderModifier {
   WRAPPER_CLASS_BOILERPLATE(OmpOrderModifier, Value);
 };
 
+// Ref: [5.1:166-171], [5.2:269-270]
+//
+// prescriptiveness ->
+//    STRICT                                        // since 5.1
+struct OmpPrescriptiveness {
+  ENUM_CLASS(Value, Strict)
+  WRAPPER_CLASS_BOILERPLATE(OmpPrescriptiveness, Value);
+};
+
 // Ref: [4.5:201-207], [5.0:293-299], [5.1:325-331], [5.2:124]
 //
 // reduction-identifier ->
@@ -3816,8 +3825,8 @@ struct OmpDependClause {
   struct TaskDep {
     OmpTaskDependenceType::Value GetTaskDepType() const;
     TUPLE_CLASS_BOILERPLATE(TaskDep);
-    std::tuple<std::optional<OmpIterator>, OmpTaskDependenceType, OmpObjectList>
-        t;
+    MODIFIER_BOILERPLATE(OmpIterator, OmpTaskDependenceType);
+    std::tuple<MODIFIERS(), OmpObjectList> t;
   };
   std::variant<TaskDep, OmpDoacross> u;
 };
@@ -3878,11 +3887,15 @@ struct OmpFromClause {
   std::tuple<MODIFIERS(), OmpObjectList, bool> t;
 };
 
-// OMP 5.2 12.6.1 grainsize-clause -> grainsize ([prescriptiveness :] value)
+// Ref: [4.5:87-91], [5.0:140-146], [5.1:166-171], [5.2:269]
+//
+// grainsize-clause ->
+//    GRAINSIZE(grain-size) |                       // since 4.5
+//    GRAINSIZE([prescriptiveness:] grain-size)     // since 5.1
 struct OmpGrainsizeClause {
   TUPLE_CLASS_BOILERPLATE(OmpGrainsizeClause);
-  ENUM_CLASS(Prescriptiveness, Strict);
-  std::tuple<std::optional<Prescriptiveness>, ScalarIntExpr> t;
+  MODIFIER_BOILERPLATE(OmpPrescriptiveness);
+  std::tuple<MODIFIERS(), ScalarIntExpr> t;
 };
 
 // 2.12 if-clause -> IF ([ directive-name-modifier :] scalar-logical-expr)
@@ -3948,6 +3961,17 @@ struct OmpMapClause {
   std::tuple<MODIFIERS(), OmpObjectList, bool> t;
 };
 
+// Ref: [4.5:87-91], [5.0:140-146], [5.1:166-171], [5.2:270]
+//
+// num-tasks-clause ->
+//    NUM_TASKS(num-tasks) |                        // since 4.5
+//    NUM_TASKS([prescriptiveness:] num-tasks)      // since 5.1
+struct OmpNumTasksClause {
+  TUPLE_CLASS_BOILERPLATE(OmpNumTasksClause);
+  MODIFIER_BOILERPLATE(OmpPrescriptiveness);
+  std::tuple<MODIFIERS(), ScalarIntExpr> t;
+};
+
 // Ref: [5.0:101-109], [5.1:126-134], [5.2:233-234]
 //
 // order-clause ->
@@ -4015,13 +4039,6 @@ struct OmpToClause {
   std::tuple<MODIFIERS(), OmpObjectList, bool> t;
 };
 
-// OMP 5.2 12.6.2 num_tasks-clause -> num_tasks ([prescriptiveness :] value)
-struct OmpNumTasksClause {
-  TUPLE_CLASS_BOILERPLATE(OmpNumTasksClause);
-  ENUM_CLASS(Prescriptiveness, Strict);
-  std::tuple<std::optional<Prescriptiveness>, ScalarIntExpr> t;
-};
-
 // Ref: [5.0:254-255], [5.1:287-288], [5.2:321-322]
 //
 // update-clause ->
@@ -4030,6 +4047,7 @@ struct OmpNumTasksClause {
 //    UPDATE(task-dependence-type)                  // since 5.2
 struct OmpUpdateClause {
   UNION_CLASS_BOILERPLATE(OmpUpdateClause);
+  // The dependence type is an argument here, not a modifier.
   std::variant<OmpDependenceType, OmpTaskDependenceType> u;
 };
 
diff --git a/flang/include/flang/Semantics/openmp-modifiers.h b/flang/include/flang/Semantics/openmp-modifiers.h
index fab4b38090b049..dbc554198df21f 100644
--- a/flang/include/flang/Semantics/openmp-modifiers.h
+++ b/flang/include/flang/Semantics/openmp-modifiers.h
@@ -82,6 +82,7 @@ DECLARE_DESCRIPTOR(parser::OmpMapType);
 DECLARE_DESCRIPTOR(parser::OmpMapTypeModifier);
 DECLARE_DESCRIPTOR(parser::OmpOrderModifier);
 DECLARE_DESCRIPTOR(parser::OmpOrderingModifier);
+DECLARE_DESCRIPTOR(parser::OmpPrescriptiveness);
 DECLARE_DESCRIPTOR(parser::OmpReductionIdentifier);
 DECLARE_DESCRIPTOR(parser::OmpReductionModifier);
 DECLARE_DESCRIPTOR(parser::OmpTaskDependenceType);
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index 2792232253879f..ff2667983d4367 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -369,6 +369,15 @@ clause::DependenceType makeDepType(const parser::OmpTaskDependenceType &inp) {
   llvm_unreachable("Unexpected task dependence type");
 }
 
+clause::Prescriptiveness
+makePrescriptiveness(parser::OmpPrescriptiveness::Value v) {
+  switch (v) {
+  case parser::OmpPrescriptiveness::Value::Strict:
+    return clause::Prescriptiveness::Strict;
+  }
+  llvm_unreachable("Unexpected prescriptiveness");
+}
+
 // --------------------------------------------------------------------
 // Actual clauses. Each T (where tomp::T exists in ClauseT) has its "make".
 
@@ -615,15 +624,18 @@ Depend make(const parser::OmpClause::Depend &inp,
   using Variant = decltype(Depend::u);
 
   auto visitTaskDep = [&](const wrapped::TaskDep &s) -> Variant {
-    auto &t0 = std::get<std::optional<parser::OmpIterator>>(s.t);
-    auto &t1 = std::get<parser::OmpTaskDependenceType>(s.t);
-    auto &t2 = std::get<parser::OmpObjectList>(s.t);
+    auto &mods = semantics::OmpGetModifiers(s);
+    auto *m0 = semantics::OmpGetUniqueModifier<parser::OmpIterator>(mods);
+    auto *m1 =
+        semantics::OmpGetUniqueModifier<parser::OmpTaskDependenceType>(mods);
+    auto &t1 = std::get<parser::OmpObjectList>(s.t);
+    assert(m1 && "expecting task dependence type");
 
     auto &&maybeIter =
-        maybeApply([&](auto &&s) { return makeIterator(s, semaCtx); }, t0);
-    return Depend::TaskDep{{/*DependenceType=*/makeDepType(t1),
+        m0 ? makeIterator(*m0, semaCtx) : std::optional<Iterator>{};
+    return Depend::TaskDep{{/*DependenceType=*/makeDepType(*m1),
                             /*Iterator=*/std::move(maybeIter),
-                            /*LocatorList=*/makeObjects(t2, semaCtx)}};
+                            /*LocatorList=*/makeObjects(t1, semaCtx)}};
   };
 
   return Depend{common::visit( //
@@ -785,19 +797,12 @@ From make(const parser::OmpClause::From &inp,
 Grainsize make(const parser::OmpClause::Grainsize &inp,
                semantics::SemanticsContext &semaCtx) {
   // inp.v -> parser::OmpGrainsizeClause
-  using wrapped = parser::OmpGrainsizeClause;
-
-  CLAUSET_ENUM_CONVERT( //
-      convert, parser::OmpGrainsizeClause::Prescriptiveness,
-      Grainsize::Prescriptiveness,
-      // clang-format off
-      MS(Strict,   Strict)
-      // clang-format on
-  );
-  auto &t0 = std::get<std::optional<wrapped::Prescriptiveness>>(inp.v.t);
+  auto &mods = semantics::OmpGetModifiers(inp.v);
+  auto *m0 = semantics::OmpGetUniqueModifier<parser::OmpPrescriptiveness>(mods);
   auto &t1 = std::get<parser::ScalarIntExpr>(inp.v.t);
-  return Grainsize{{/*Prescriptiveness=*/maybeApply(convert, t0),
-                    /*Grainsize=*/makeExpr(t1, semaCtx)}};
+  return Grainsize{
+      {/*Prescriptiveness=*/maybeApplyToV(makePrescriptiveness, m0),
+       /*Grainsize=*/makeExpr(t1, semaCtx)}};
 }
 
 HasDeviceAddr make(const parser::OmpClause::HasDeviceAddr &inp,
@@ -1047,18 +1052,10 @@ Novariants make(const parser::OmpClause::Novariants &inp,
 NumTasks make(const parser::OmpClause::NumTasks &inp,
               semantics::SemanticsContext &semaCtx) {
   // inp.v -> parser::OmpNumTasksClause
-  using wrapped = parser::OmpNumTasksClause;
-
-  CLAUSET_ENUM_CONVERT( //
-      convert, parser::OmpNumTasksClause::Prescriptiveness,
-      NumTasks::Prescriptiveness,
-      // clang-format off
-      MS(Strict,   Strict)
-      // clang-format on
-  );
-  auto &t0 = std::get<std::optional<wrapped::Prescriptiveness>>(inp.v.t);
+  auto &mods = semantics::OmpGetModifiers(inp.v);
+  auto *m0 = semantics::OmpGetUniqueModifier<parser::OmpPrescriptiveness>(mods);
   auto &t1 = std::get<parser::ScalarIntExpr>(inp.v.t);
-  return NumTasks{{/*Prescriptiveness=*/maybeApply(convert, t0),
+  return NumTasks{{/*Prescriptiveness=*/maybeApplyToV(makePrescriptiveness, m0),
                    /*NumTasks=*/makeExpr(t1, semaCtx)}};
 }
 
diff --git a/flang/lib/Lower/OpenMP/Clauses.h b/flang/lib/Lower/OpenMP/Clauses.h
index 562685e43c13bf..65282d243d87af 100644
--- a/flang/lib/Lower/OpenMP/Clauses.h
+++ b/flang/lib/Lower/OpenMP/Clauses.h
@@ -176,6 +176,7 @@ using DefinedOperator = tomp::type::DefinedOperatorT<IdTy, ExprTy>;
 using ProcedureDesignator = tomp::type::ProcedureDesignatorT<IdTy, ExprTy>;
 using ReductionOperator = tomp::type::ReductionIdentifierT<IdTy, ExprTy>;
 using DependenceType = tomp::type::DependenceType;
+using Prescriptiveness = tomp::type::Prescriptiveness;
 
 // "Requires" clauses are handled early on, and the aggregated information
 // is stored in the Symbol details of modules, programs, and subprograms.
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 5b61d053ad1622..44465fd787e584 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -176,6 +176,9 @@ TYPE_PARSER(construct<OmpOrderingModifier>(
     "NONMONOTONIC" >> pure(OmpOrderingModifier::Value::Nonmonotonic) ||
     "SIMD" >> pure(OmpOrderingModifier::Value::Simd)))
 
+TYPE_PARSER(construct<OmpPrescriptiveness>(
+    "STRICT" >> pure(OmpPrescriptiveness::Value::Strict)))
+
 TYPE_PARSER(construct<OmpReductionModifier>(
     "INSCAN" >> pure(OmpReductionModifier::Value::Inscan) ||
     "TASK" >> pure(OmpReductionModifier::Value::Task) ||
@@ -213,6 +216,11 @@ TYPE_PARSER(sourced(construct<OmpAllocateClause::Modifier>(sourced(
 TYPE_PARSER(sourced(
     construct<OmpDefaultmapClause::Modifier>(Parser<OmpVariableCategory>{})))
 
+TYPE_PARSER(sourced(construct<OmpDependClause::TaskDep::Modifier>(sourced(
+    construct<OmpDependClause::TaskDep::Modifier>(Parser<OmpIterator>{}) ||
+    construct<OmpDependClause::TaskDep::Modifier>(
+        Parser<OmpTaskDependenceType>{})))))
+
 TYPE_PARSER(
     sourced(construct<OmpDeviceClause::Modifier>(Parser<OmpDeviceModifier>{})))
 
@@ -221,6 +229,9 @@ TYPE_PARSER(sourced(construct<OmpFromClause::Modifier>(
         construct<OmpFromClause::Modifier>(Parser<OmpMapper>{}) ||
         construct<OmpFromClause::Modifier>(Parser<OmpIterator>{})))))
 
+TYPE_PARSER(sourced(construct<OmpGrainsizeClause::Modifier>(
+    Parser<OmpPrescriptiveness>{})))
+
 TYPE_PARSER(sourced(construct<OmpMapClause::Modifier>(
     sourced(construct<OmpMapClause::Modifier>(Parser<OmpMapTypeModifier>{}) ||
         construct<OmpMapClause::Modifier>(Parser<OmpMapper>{}) ||
@@ -230,6 +241,9 @@ TYPE_PARSER(sourced(construct<OmpMapClause::Modifier>(
 TYPE_PARSER(
     sourced(construct<OmpOrderClause::Modifier>(Parser<OmpOrderModifier>{})))
 
+TYPE_PARSER(sourced(construct<OmpNumTasksClause::Modifier>(
+    Parser<OmpPrescriptiveness>{})))
+
 TYPE_PARSER(sourced(construct<OmpReductionClause::Modifier>(sourced(
     construct<OmpReductionClause::Modifier>(Parser<OmpReductionModifier>{}) ||
     construct<OmpReductionClause::Modifier>(
@@ -382,10 +396,17 @@ TYPE_PARSER(construct<OmpDoacross>(
 
 TYPE_CONTEXT_PARSER("Omp Depend clause"_en_US,
     construct<OmpDependClause>(
+        // Try to parse OmpDoacross first, because TaskDep will succeed on
+        // "sink: xxx", interpreting it to not have any modifiers, and "sink"
+        // being an OmpObject. Parsing of the TaskDep variant will stop right
+        // after the "sink", leaving the ": xxx" unvisited.
+        construct<OmpDependClause>(Parser<OmpDoacross>{}) ||
+        // Parse TaskDep after Doacross.
         construct<OmpDependClause>(construct<OmpDependClause::TaskDep>(
-            maybe(Parser<OmpIterator>{} / ","_tok),
-            Parser<OmpTaskDependenceType>{} / ":", Parser<OmpObjectList>{})) ||
-        construct<OmpDependClause>(Parser<OmpDoacross>{})))
+            maybe(nonemptyList(Parser<OmpDependClause::TaskDep::Modifier>{}) /
+                ": "),
+            Parser<OmpObjectList>{}))
+))
 
 TYPE_CONTEXT_PARSER("Omp Doacross clause"_en_US,
     construct<OmpDoacrossClause>(Parser<OmpDoacross>{}))
@@ -427,12 +448,12 @@ TYPE_PARSER(construct<OmpOrderClause>(
 
 // OMP 5.2 12.6.1 grainsize([ prescriptiveness :] scalar-integer-expression)
 TYPE_PARSER(construct<OmpGrainsizeClause>(
-    maybe("STRICT" >> pure(OmpGrainsizeClause::Prescriptiveness::Strict) / ":"),
+    maybe(nonemptyList(Parser<OmpGrainsizeClause::Modifier>{}) / ":"),
     scalarIntExpr))
 
 // OMP 5.2 12.6.2 num_tasks([ prescriptiveness :] scalar-integer-expression)
 TYPE_PARSER(construct<OmpNumTasksClause>(
-    maybe("STRICT" >> pure(OmpNumTasksClause::Prescriptiveness::Strict) / ":"),
+    maybe(nonemptyList(Parser<OmpNumTasksClause::Modifier>{}) / ":"),
     scalarIntExpr))
 
 TYPE_PARSER(
diff --git a/flang/lib/Parser/parse-tree.cpp b/flang/lib/Parser/parse-tree.cpp
index 24b2902f286f4b..a414f226058e3e 100644
--- a/flang/lib/Parser/parse-tree.cpp
+++ b/flang/lib/Parser/parse-tree.cpp
@@ -267,7 +267,18 @@ OmpDependenceType::Value OmpDoacross::GetDepType() const {
 }
 
 OmpTaskDependenceType::Value OmpDependClause::TaskDep::GetTaskDepType() const {
-  return std::get<parser::OmpTaskDependenceType>(t).v;
+  using Modifier = OmpDependClause::TaskDep::Modifier;
+  auto &modifiers{std::get<std::optional<std::list<Modifier>>>(t)};
+  if (modifiers) {
+    for (auto &m : *modifiers) {
+      if (auto *dep{std::get_if<OmpTaskDependenceType>(&m.u)}) {
+        return dep->v;
+      }
+    }
+    llvm_unreachable("expecting OmpTaskDependenceType in TaskDep");
+  } else {
+    llvm_unreachable("expecting modifiers on OmpDependClause::TaskDep");
+  }
 }
 
 } // namespace Fortran::parser
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index 80ebd692bd1192..cd025333a077d3 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2171,13 +2171,13 @@ class UnparseVisitor {
     Walk(std::get<OmpOrderClause::Ordering>(x.t));
   }
   void Unparse(const OmpGrainsizeClause &x) {
-    Walk(std::get<std::optional<OmpGrainsizeClause::Prescriptiveness>>(x.t),
-        ":");
+    using Modifier = OmpGrainsizeClause::Modifier;
+    Walk(std::get<std::optional<std::list<Modifier>>>(x.t), ": ");
     Walk(std::get<ScalarIntExpr>(x.t));
   }
   void Unparse(const OmpNumTasksClause &x) {
-    Walk(
-        std::get<std::optional<OmpNumTasksClause::Prescriptiveness>>(x.t), ":");
+    using Modifier = OmpNumTasksClause::Modifier;
+    Walk(std::get<std::optional<std::list<Modifier>>>(x.t), ": ");
     Walk(std::get<ScalarIntExpr>(x.t));
   }
   void Unparse(const OmpDoacross::Sink &x) {
@@ -2186,8 +2186,8 @@ class UnparseVisitor {
   }
   void Unparse(const OmpDoacross::Source &) { Word("SOURCE"); }
   void Unparse(const OmpDependClause::TaskDep &x) {
-    Walk(std::get<OmpTaskDependenceType>(x.t));
-    Put(":");
+    using Modifier = OmpDependClause::TaskDep::Modifier;
+    Walk(std::get<std::optional<std::list<Modifier>>>(x.t), ": ");
     Walk(std::get<OmpObjectList>(x.t));
   }
   void Unparse(const OmpDefaultmapClause &x) {
@@ -2842,9 +2842,7 @@ class UnparseVisitor {
   WALK_NESTED_ENUM(OmpCancelType, Type) // OMP cancel-type
   WALK_NESTED_ENUM(OmpOrderClause, Ordering) // OMP ordering
   WALK_NESTED_ENUM(OmpOrderModifier, Value) // OMP order-modifier
-  WALK_NESTED_ENUM(
-      OmpGrainsizeClause, Prescriptiveness) // OMP grainsize-modifier
-  WALK_NESTED_ENUM(OmpNumTasksClause, Prescriptiveness) // OMP numtasks-modifier
+  WALK_NESTED_ENUM(OmpPrescriptiveness, Value) // OMP prescriptiveness
   WALK_NESTED_ENUM(OmpMapType, Value) // OMP map-type
   WALK_NESTED_ENUM(OmpMapTypeModifier, Value) // OMP map-type-modifier
 #undef WALK_NESTED_ENUM
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index ebcafb6a0e354e..9e589067c8868d 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -3698,18 +3698,13 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Depend &x) {
         }
       }
     }
-    if (std::get<std::optional<parser::OmpIterator>>(taskDep->t)) {
-      unsigned allowedInVersion{50};
-      if (version < allowedInVersion) {
-        context_.Say(GetContext().clauseSource,
-            "Iterator modifiers are not supported in %s, %s"_warn_en_US,
-            ThisVersion(version), TryVersion(allowedInVersion));
-      } else {
+    if (OmpVerifyModifiers(*taskDep, llvm::omp::OMPC_depend,
+            GetContext().clauseSource, context_)) {
+      auto &modifiers{OmpGetModifiers(*taskDep)};
+      if (OmpGetUniqueModifier<parser::OmpIterator>(modifiers)) {
         if (dir == llvm::omp::OMPD_depobj) {
           context_.Say(GetContext().clauseSource,
-              "An iterator-modifier may specify multiple locators, "
-              "a DEPEND clause on a DEPOBJ construct must only specify "
-              "one locator"_warn_en_US);
+              "An iterator-modifier may specify multiple locators, a DEPEND clause on a DEPOBJ construct must only specify one locator"_warn_en_US);
         }
       }
     }
diff --git a/flang/lib/Semantics/openmp-modifiers.cpp b/flang/lib/Semantics/openmp-modifiers.cpp
index 97227bfef0ea54..e384b0270e6eae 100644
--- a/flang/lib/Semantics/openmp-modifiers.cpp
+++ b/flang/lib/Semantics/openmp-modifiers.cpp
@@ -321,6 +321,22 @@ const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpOrderingModifier>() {
   return desc;
 }
 
+template <>
+const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpPrescriptiveness>() {
+  static const OmpModifierDescriptor desc{
+      /*name=*/"prescriptiveness",
+      /*props=*/
+      {
+          {51, {OmpProperty::Unique}},
+      },
+      /*clauses=*/
+      {
+          {51, {Clause::OMPC_grainsize, Clause::OMPC_num_tasks}},
+      },
+  };
+  return desc;
+}
+
 template <>
 const OmpModifierDescriptor &
 OmpGetDescriptor<parser::OmpReductionIdentifier>() {
@@ -363,11 +379,12 @@ const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpTaskDependenceType>() {
       /*name=*/"task-dependence-type",
       /*props=*/
       {
-          {52, {OmpProperty::Required, OmpProperty::Ultimate}},
+          {45, {OmpProperty::Required, OmpProperty::Ultimate}},
       },
       /*clauses=*/
       {
-          {52, {Clause::OMPC_depend, Clause::OMPC_update}},
+          {45, {Clause::OMPC_depend}},
+          {51, {Clause::OMPC_depend, Clause::OMPC_update}},
       },
   };
   return desc;
diff --git a/flang/test/Parser/OpenMP/depobj-construct.f90 b/flang/test/Parser/OpenMP/depobj-construct.f90
index 51726a5adf99ec..f186c82a2ccc30 100644
--- a/flang/test/Parser/OpenMP/depobj-construct.f90
+++ b/flang/test/Parser/OpenMP/depobj-construct.f90
@@ -8,14 +8,14 @@ subroutine f00
 
 !UNPARSE: SUBROUTINE f00
 !UNPARSE:  INTEGER x, y
-!UNPARSE: !$OMP DEPOBJ(x) DEPEND(IN:y)
+!UNPARSE: !$OMP DEPOBJ(x) DEPEND(IN: y)
 !UNPARSE: END SUBROUTINE
 
 !PARSE-TREE: ExecutionPartConstruct -> ExecutableConstruct -> OpenMPConstruct -> OpenMPStandaloneConstruct -> OpenMPDepobjConstruct
 !PARSE-TREE: | Verbatim
 !PARSE-TREE: | OmpObject -> Designator -> DataRef -> Name = 'x'
 !PARSE-TREE: | OmpClause -> Depend -> OmpDependClause -> TaskDep
-!PARSE-TREE: | | OmpTaskDependenceType -> Value = In
+!PARSE-TREE: | | Modifier -> OmpTaskDependenceType -> Value = In
 !PARSE-TREE: | | OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'y'
 
 subroutine f01
diff --git a/flang/test/Parser/OpenMP/taskloop.f90 b/flang/test/Parser/OpenMP/taskloop.f90
index a9c361046bd5f5..f053aa7f0cff37 100644
--- a/flang/test/Parser/OpenMP/taskloop.f90
+++ b/flang/test/Parser/OpenMP/taskloop.f90
@@ -4,11 +4,11 @@
 subroutine parallel_work
   integer :: i
 
-!CHECK: !$OMP TASKLOOP  GRAINSIZE(STRICT:500_4)
+!CHECK: !$OMP TASKLOOP  GRAINSIZE(STRICT: 500_4)
 !PARSE-TREE: OmpBeginLoopDirective
 !PARSE-TREE-NEXT: OmpLoopDirective -> llvm::omp::Directive = taskloop
 !PARSE-TREE-NEXT: OmpClauseList -> OmpClause -> Grainsize -> OmpGrainsizeClause
-!PARSE-TREE-NEXT: Prescriptiveness = Strict
+!PARSE-TREE-NEXT: Modifier -> OmpPrescriptiveness -> Value = Strict
 !PARSE-TREE-NEXT: Scalar -> Integer -> Expr = '500_4'
   !$omp taskloop grainsize(strict: 500)
   do i=1,10000
@@ -27,11 +27,11 @@ subroutine parallel_work
   end do
   !$omp end taskloop
 
-!CHECK: !$OMP TASKLOOP  NUM_TASKS(STRICT:500_4)
+!CHECK: !$OMP TASKLOOP  NUM_TASKS(STRICT: 500_4)
 !PARSE-TREE: OmpBeginLoopDirective
 !PARSE-TREE-NEXT: OmpLoopDirective -> llvm::omp::Directive = taskloop
 !PARSE-TREE-NEXT: OmpClauseList -> OmpClause -> NumTasks -> OmpNumTasksClause
-!PARSE-TREE-NEXT: Prescriptiveness = Strict
+!PARSE-TREE-NEXT: Modifier -> OmpPrescriptiveness -> Value = Strict
 !PARSE-TREE-NEXT: Scalar -> Integer -> Expr = '500_4'
   !$omp taskloop num_tasks(strict: 500)
   do i=1,10000
diff --git a/flang/test/Semantics/OpenMP/depend05.f90 b/flang/test/Semantics/OpenMP/depend05.f90
index 53fd82bd08a9eb..3ca091ef3187d1 100644
--- a/flang/test/Semantics/OpenMP/depend05.f90
+++ b/flang/test/Semantics/OpenMP/depend05.f90
@@ -2,7 +2,7 @@
 
 subroutine f00(x)
   integer :: x(10)
-!WARNING: Iterator modifiers are not supported in OpenMP v4.5, try -fopenmp-version=50
+!WARNING: 'iterator' modifier is not supported in OpenMP v4.5, try -fopenmp-version=50
   !$omp task depend(iterator(i = 1:10), in: x(i))
   x = 0
   !$omp end task
diff --git a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h
index 07efd6fd4e9da4..67632fb79f8aac 100644
--- a/llvm/include/llvm/Frontend/OpenMP/ClauseT.h
+++ b/llvm/include/llvm/Frontend/OpenMP/ClauseT.h
@@ -242,6 +242,7 @@ ENUM(MotionExpectation, Present);
 // V5.2: [15.9.1] `task-dependence-type` modifier
 ENUM(DependenceType, Depobj, In, Inout, Inoutset, Mutexinoutset, Out, Sink,
      Source);
+ENUM(Prescriptiveness, Strict);
 
 template <typename I, typename E> //
 struct LoopIterationT {
@@ -643,7 +644,7 @@ struct FullT {
 // V5.2: [12.6.1] `grainsize` clause
 template <typename T, typename I, typename E> //
 struct GrainsizeT {
-  ENUM(Prescriptiveness, Strict);
+  using Prescriptiveness = type::Prescriptiveness;
   using GrainSize = E;
   using TupleTrait = std::true_type;
   std::tuple<OPT(Prescriptiveness), GrainSize> t;
@@ -876,8 +877,8 @@ struct NowaitT {
 // V5.2: [12.6.2] `num_tasks` clause
 template <typename T, typename I, typename E> //
 struct NumTasksT {
+  using Prescriptiveness = type::Prescriptiveness;
   using NumTasks = E;
-  ENUM(Prescriptiveness, Strict);
   using TupleTrait = std::true_type;
   std::tuple<OPT(Prescriptiveness), NumTasks> t;
 };

>From 24a9f62620e51bcbf1fad0fa21a127332597ca8c Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Wed, 27 Nov 2024 13:58:19 -0600
Subject: [PATCH 09/11] format

---
 flang/lib/Parser/openmp-parsers.cpp | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 44465fd787e584..1d3dd518601995 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -229,8 +229,8 @@ TYPE_PARSER(sourced(construct<OmpFromClause::Modifier>(
         construct<OmpFromClause::Modifier>(Parser<OmpMapper>{}) ||
         construct<OmpFromClause::Modifier>(Parser<OmpIterator>{})))))
 
-TYPE_PARSER(sourced(construct<OmpGrainsizeClause::Modifier>(
-    Parser<OmpPrescriptiveness>{})))
+TYPE_PARSER(sourced(
+    construct<OmpGrainsizeClause::Modifier>(Parser<OmpPrescriptiveness>{})))
 
 TYPE_PARSER(sourced(construct<OmpMapClause::Modifier>(
     sourced(construct<OmpMapClause::Modifier>(Parser<OmpMapTypeModifier>{}) ||
@@ -241,8 +241,8 @@ TYPE_PARSER(sourced(construct<OmpMapClause::Modifier>(
 TYPE_PARSER(
     sourced(construct<OmpOrderClause::Modifier>(Parser<OmpOrderModifier>{})))
 
-TYPE_PARSER(sourced(construct<OmpNumTasksClause::Modifier>(
-    Parser<OmpPrescriptiveness>{})))
+TYPE_PARSER(sourced(
+    construct<OmpNumTasksClause::Modifier>(Parser<OmpPrescriptiveness>{})))
 
 TYPE_PARSER(sourced(construct<OmpReductionClause::Modifier>(sourced(
     construct<OmpReductionClause::Modifier>(Parser<OmpReductionModifier>{}) ||
@@ -405,8 +405,7 @@ TYPE_CONTEXT_PARSER("Omp Depend clause"_en_US,
         construct<OmpDependClause>(construct<OmpDependClause::TaskDep>(
             maybe(nonemptyList(Parser<OmpDependClause::TaskDep::Modifier>{}) /
                 ": "),
-            Parser<OmpObjectList>{}))
-))
+            Parser<OmpObjectList>{}))))
 
 TYPE_CONTEXT_PARSER("Omp Doacross clause"_en_US,
     construct<OmpDoacrossClause>(Parser<OmpDoacross>{}))

>From cfcf8d1e7ffdcec92dc0dfffccb3c620a2df804f Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Wed, 27 Nov 2024 08:34:33 -0600
Subject: [PATCH 10/11] [flang][OpenMP] Use new modifiers in IF/LASTPRIVATE

The usual changes, added more references to OpenMP specs.
---
 flang/examples/FeatureList/FeatureList.cpp    |   3 +-
 .../FlangOmpReport/FlangOmpReportVisitor.cpp  |   5 +-
 .../FlangOmpReport/FlangOmpReportVisitor.h    |   2 +-
 flang/include/flang/Parser/dump-parse-tree.h  |   7 +-
 flang/include/flang/Parser/parse-tree.h       |  50 ++++++--
 .../flang/Semantics/openmp-modifiers.h        |   2 +
 flang/lib/Lower/OpenMP/Clauses.cpp            |  36 ++----
 flang/lib/Parser/CMakeLists.txt               |   1 +
 flang/lib/Parser/openmp-parsers.cpp           |  73 ++++++++---
 flang/lib/Parser/unparse.cpp                  |  15 ++-
 flang/lib/Semantics/check-omp-structure.cpp   | 121 +++++++++++-------
 flang/lib/Semantics/openmp-modifiers.cpp      |  34 +++++
 .../test/Parser/OpenMP/if-clause-unparse.f90  |  20 +--
 flang/test/Parser/OpenMP/if-clause.f90        |  22 ++--
 .../test/Parser/OpenMP/lastprivate-clause.f90 |   4 +-
 .../Semantics/OpenMP/clause-validity01.f90    |   4 +-
 flang/test/Semantics/OpenMP/if-clause.f90     |  64 ++++-----
 17 files changed, 297 insertions(+), 166 deletions(-)

diff --git a/flang/examples/FeatureList/FeatureList.cpp b/flang/examples/FeatureList/FeatureList.cpp
index c5cb8c8fdf40bb..41a6255207976d 100644
--- a/flang/examples/FeatureList/FeatureList.cpp
+++ b/flang/examples/FeatureList/FeatureList.cpp
@@ -492,7 +492,8 @@ struct NodeVisitor {
   READ_FEATURE(OmpPrescriptiveness)
   READ_FEATURE(OmpPrescriptiveness::Value)
   READ_FEATURE(OmpIfClause)
-  READ_FEATURE(OmpIfClause::DirectiveNameModifier)
+  READ_FEATURE(OmpIfClause::Modifier)
+  READ_FEATURE(OmpDirectiveNameModifier)
   READ_FEATURE(OmpLinearClause)
   READ_FEATURE(OmpLinearClause::WithModifier)
   READ_FEATURE(OmpLinearClause::WithoutModifier)
diff --git a/flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp b/flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp
index afabd564ad5bdc..2dc480f0c901b1 100644
--- a/flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp
+++ b/flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp
@@ -8,6 +8,7 @@
 
 #include "FlangOmpReportVisitor.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/Frontend/OpenMP/OMP.h"
 
 namespace Fortran {
 namespace parser {
@@ -238,9 +239,9 @@ void OpenMPCounterVisitor::Post(const OmpScheduleClause::Kind &c) {
   clauseDetails +=
       "type=" + std::string{OmpScheduleClause::EnumToString(c)} + ";";
 }
-void OpenMPCounterVisitor::Post(const OmpIfClause::DirectiveNameModifier &c) {
+void OpenMPCounterVisitor::Post(const OmpDirectiveNameModifier &c) {
   clauseDetails +=
-      "name_modifier=" + std::string{OmpIfClause::EnumToString(c)} + ";";
+      "name_modifier=" + llvm::omp::getOpenMPDirectiveName(c.v).str() + ";";
 }
 void OpenMPCounterVisitor::Post(const OmpCancelType::Type &c) {
   clauseDetails += "type=" + std::string{OmpCancelType::EnumToString(c)} + ";";
diff --git a/flang/examples/FlangOmpReport/FlangOmpReportVisitor.h b/flang/examples/FlangOmpReport/FlangOmpReportVisitor.h
index ed202e8ed2a4c7..59bdac8594cb7c 100644
--- a/flang/examples/FlangOmpReport/FlangOmpReportVisitor.h
+++ b/flang/examples/FlangOmpReport/FlangOmpReportVisitor.h
@@ -77,7 +77,7 @@ struct OpenMPCounterVisitor {
   void Post(const OmpTaskDependenceType::Value &c);
   void Post(const OmpMapType::Value &c);
   void Post(const OmpScheduleClause::Kind &c);
-  void Post(const OmpIfClause::DirectiveNameModifier &c);
+  void Post(const OmpDirectiveNameModifier &c);
   void Post(const OmpCancelType::Type &c);
   void Post(const OmpClause &c);
   void PostClauseCommon(const ClauseInfo &ci);
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 1ec38de29b85d6..b0b8d8d7ccc556 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -548,10 +548,13 @@ class ParseTreeDumper {
   NODE(OmpFromClause, Modifier)
   NODE(parser, OmpExpectation)
   NODE_ENUM(OmpExpectation, Value)
+  NODE(parser, OmpDirectiveNameModifier)
   NODE(parser, OmpIfClause)
-  NODE_ENUM(OmpIfClause, DirectiveNameModifier)
-  NODE_ENUM(OmpLastprivateClause, LastprivateModifier)
+  NODE(OmpIfClause, Modifier)
   NODE(parser, OmpLastprivateClause)
+  NODE(OmpLastprivateClause, Modifier)
+  NODE(parser, OmpLastprivateModifier)
+  NODE_ENUM(OmpLastprivateModifier, Value)
   NODE(parser, OmpLinearClause)
   NODE(OmpLinearClause, WithModifier)
   NODE(OmpLinearClause, WithoutModifier)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index c00560b1f1726a..06c32831d2c60c 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3543,6 +3543,23 @@ struct OmpDeviceModifier {
   WRAPPER_CLASS_BOILERPLATE(OmpDeviceModifier, Value);
 };
 
+// Ref: [5.2:72-73,230-323], in 4.5-5.1 it's scattered over individual
+// directives that allow the IF clause.
+//
+// directive-name-modifier ->
+//    PARALLEL | TARGET | TARGET DATA |
+//    TARGET ENTER DATA | TARGET EXIT DATA |
+//    TARGET UPDATE | TASK | TASKLOOP |             // since 4.5
+//    CANCEL[*] | SIMD |                            // since 5.0
+//    TEAMS                                         // since 5.2
+//
+// [*] The IF clause is allowed on CANCEL in OpenMP 4.5, but only without
+// the directive-name-modifier. For the sake of uniformity CANCEL can be
+// considered a valid value in 4.5 as well.
+struct OmpDirectiveNameModifier {
+  WRAPPER_CLASS_BOILERPLATE(OmpDirectiveNameModifier, llvm::omp::Directive);
+};
+
 // Ref: [5.1:205-209], [5.2:166-168]
 //
 // motion-modifier ->
@@ -3566,6 +3583,15 @@ struct OmpIterator {
   WRAPPER_CLASS_BOILERPLATE(OmpIterator, std::list<OmpIteratorSpecifier>);
 };
 
+// Ref: [5.0:288-290], [5.1:321-322], [5.2:115-117]
+//
+// lastprivate-modifier ->
+//    CONDITIONAL                                   // since 5.0
+struct OmpLastprivateModifier {
+  ENUM_CLASS(Value, Conditional)
+  WRAPPER_CLASS_BOILERPLATE(OmpLastprivateModifier, Value);
+};
+
 // Ref: [4.5:207-210], [5.0:290-293], [5.1:323-325], [5.2:117-120]
 //
 // linear-modifier ->
@@ -3898,12 +3924,16 @@ struct OmpGrainsizeClause {
   std::tuple<MODIFIERS(), ScalarIntExpr> t;
 };
 
-// 2.12 if-clause -> IF ([ directive-name-modifier :] scalar-logical-expr)
+// Ref: [5.2:72-73], in 4.5-5.1 it's scattered over individual directives
+// that allow the IF clause.
+//
+// if-clause ->
+//    IF([directive-name-modifier:]
+//        scalar-logical-expression)                // since 4.5
 struct OmpIfClause {
   TUPLE_CLASS_BOILERPLATE(OmpIfClause);
-  ENUM_CLASS(DirectiveNameModifier, Parallel, Simd, Target, TargetData,
-      TargetEnterData, TargetExitData, TargetUpdate, Task, Taskloop, Teams)
-  std::tuple<std::optional<DirectiveNameModifier>, ScalarLogicalExpr> t;
+  MODIFIER_BOILERPLATE(OmpDirectiveNameModifier);
+  std::tuple<MODIFIERS(), ScalarLogicalExpr> t;
 };
 
 // OMP 5.0 2.19.5.6 in_reduction-clause -> IN_REDUCTION (reduction-identifier:
@@ -3913,13 +3943,15 @@ struct OmpInReductionClause {
   std::tuple<OmpReductionIdentifier, OmpObjectList> t;
 };
 
-// OMP 5.0 2.19.4.5 lastprivate-clause ->
-//                    LASTPRIVATE ([lastprivate-modifier :] list)
-//                  lastprivate-modifier -> CONDITIONAL
+// Ref: [4.5:199-201], [5.0:288-290], [5.1:321-322], [5.2:115-117]
+//
+// lastprivate-clause ->
+//    LASTPRIVATE(list) |                           // since 4.5
+//    LASTPRIVATE([lastprivate-modifier:] list)     // since 5.0
 struct OmpLastprivateClause {
   TUPLE_CLASS_BOILERPLATE(OmpLastprivateClause);
-  ENUM_CLASS(LastprivateModifier, Conditional);
-  std::tuple<std::optional<LastprivateModifier>, OmpObjectList> t;
+  MODIFIER_BOILERPLATE(OmpLastprivateModifier);
+  std::tuple<MODIFIERS(), OmpObjectList> t;
 };
 
 // 2.15.3.7 linear-clause -> LINEAR (linear-list[ : linear-step])
diff --git a/flang/include/flang/Semantics/openmp-modifiers.h b/flang/include/flang/Semantics/openmp-modifiers.h
index dbc554198df21f..4025ce112d9cab 100644
--- a/flang/include/flang/Semantics/openmp-modifiers.h
+++ b/flang/include/flang/Semantics/openmp-modifiers.h
@@ -74,8 +74,10 @@ DECLARE_DESCRIPTOR(parser::OmpAllocatorSimpleModifier);
 DECLARE_DESCRIPTOR(parser::OmpChunkModifier);
 DECLARE_DESCRIPTOR(parser::OmpDependenceType);
 DECLARE_DESCRIPTOR(parser::OmpDeviceModifier);
+DECLARE_DESCRIPTOR(parser::OmpDirectiveNameModifier);
 DECLARE_DESCRIPTOR(parser::OmpExpectation);
 DECLARE_DESCRIPTOR(parser::OmpIterator);
+DECLARE_DESCRIPTOR(parser::OmpLastprivateModifier);
 DECLARE_DESCRIPTOR(parser::OmpLinearModifier);
 DECLARE_DESCRIPTOR(parser::OmpMapper);
 DECLARE_DESCRIPTOR(parser::OmpMapType);
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index ff2667983d4367..10c31963ec493a 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -825,27 +825,13 @@ Holds make(const parser::OmpClause::Holds &inp,
 If make(const parser::OmpClause::If &inp,
         semantics::SemanticsContext &semaCtx) {
   // inp.v -> parser::OmpIfClause
-  using wrapped = parser::OmpIfClause;
-
-  CLAUSET_ENUM_CONVERT( //
-      convert, wrapped::DirectiveNameModifier, llvm::omp::Directive,
-      // clang-format off
-      MS(Parallel,         OMPD_parallel)
-      MS(Simd,             OMPD_simd)
-      MS(Target,           OMPD_target)
-      MS(TargetData,       OMPD_target_data)
-      MS(TargetEnterData,  OMPD_target_enter_data)
-      MS(TargetExitData,   OMPD_target_exit_data)
-      MS(TargetUpdate,     OMPD_target_update)
-      MS(Task,             OMPD_task)
-      MS(Taskloop,         OMPD_taskloop)
-      MS(Teams,            OMPD_teams)
-      // clang-format on
-  );
-  auto &t0 = std::get<std::optional<wrapped::DirectiveNameModifier>>(inp.v.t);
+  auto &mods = semantics::OmpGetModifiers(inp.v);
+  auto *m0 =
+      semantics::OmpGetUniqueModifier<parser::OmpDirectiveNameModifier>(mods);
   auto &t1 = std::get<parser::ScalarLogicalExpr>(inp.v.t);
-  return If{{/*DirectiveNameModifier=*/maybeApply(convert, t0),
-             /*IfExpression=*/makeExpr(t1, semaCtx)}};
+  return If{
+      {/*DirectiveNameModifier=*/maybeApplyToV([](auto &&s) { return s; }, m0),
+       /*IfExpression=*/makeExpr(t1, semaCtx)}};
 }
 
 // Inbranch: empty
@@ -889,20 +875,20 @@ IsDevicePtr make(const parser::OmpClause::IsDevicePtr &inp,
 Lastprivate make(const parser::OmpClause::Lastprivate &inp,
                  semantics::SemanticsContext &semaCtx) {
   // inp.v -> parser::OmpLastprivateClause
-  using wrapped = parser::OmpLastprivateClause;
-
   CLAUSET_ENUM_CONVERT( //
-      convert, parser::OmpLastprivateClause::LastprivateModifier,
+      convert, parser::OmpLastprivateModifier::Value,
       Lastprivate::LastprivateModifier,
       // clang-format off
       MS(Conditional, Conditional)
       // clang-format on
   );
 
-  auto &t0 = std::get<std::optional<wrapped::LastprivateModifier>>(inp.v.t);
+  auto &mods = semantics::OmpGetModifiers(inp.v);
+  auto *m0 =
+      semantics::OmpGetUniqueModifier<parser::OmpLastprivateModifier>(mods);
   auto &t1 = std::get<parser::OmpObjectList>(inp.v.t);
 
-  return Lastprivate{{/*LastprivateModifier=*/maybeApply(convert, t0),
+  return Lastprivate{{/*LastprivateModifier=*/maybeApplyToV(convert, m0),
                       /*List=*/makeObjects(t1, semaCtx)}};
 }
 
diff --git a/flang/lib/Parser/CMakeLists.txt b/flang/lib/Parser/CMakeLists.txt
index 600a2f67df4431..d364671d7a3229 100644
--- a/flang/lib/Parser/CMakeLists.txt
+++ b/flang/lib/Parser/CMakeLists.txt
@@ -30,6 +30,7 @@ add_flang_library(FortranParser
   LINK_COMPONENTS
   Support
   FrontendOpenACC
+  FrontendOpenMP
 
   DEPENDS
   omp_gen
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 1d3dd518601995..920f97f7ca0f21 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -16,6 +16,10 @@
 #include "token-parsers.h"
 #include "type-parser-implementation.h"
 #include "flang/Parser/parse-tree.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Frontend/OpenMP/OMP.h"
 
 // OpenMP Directives and Clauses
 namespace Fortran::parser {
@@ -23,6 +27,47 @@ namespace Fortran::parser {
 constexpr auto startOmpLine = skipStuffBeforeStatement >> "!$OMP "_sptok;
 constexpr auto endOmpLine = space >> endOfLine;
 
+/// Parse OpenMP directive name (this includes compound directives).
+struct OmpDirectiveNameParser {
+  using resultType = llvm::omp::Directive;
+  using Token = TokenStringMatch<false, false>;
+
+  std::optional<resultType> Parse(ParseState &state) const {
+    for (const NameWithId &nid : directives()) {
+      if (attempt(Token(nid.first.data())).Parse(state)) {
+        return nid.second;
+      }
+    }
+    return std::nullopt;
+  }
+
+private:
+  using NameWithId = std::pair<std::string, llvm::omp::Directive>;
+
+  llvm::iterator_range<const NameWithId *> directives() const;
+  void initTokens(NameWithId *) const;
+};
+
+llvm::iterator_range<const OmpDirectiveNameParser::NameWithId *>
+OmpDirectiveNameParser::directives() const {
+  static NameWithId table[llvm::omp::Directive_enumSize];
+  [[maybe_unused]] static bool init = (initTokens(table), true);
+  return llvm::make_range(std::cbegin(table), std::cend(table));
+}
+
+void OmpDirectiveNameParser::initTokens(NameWithId *table) const {
+  for (size_t i{0}, e{llvm::omp::Directive_enumSize}; i != e; ++i) {
+    auto id{static_cast<llvm::omp::Directive>(i)};
+    llvm::StringRef name{llvm::omp::getOpenMPDirectiveName(id)};
+    table[i] = std::make_pair(name.str(), id);
+  }
+  // Sort the table with respect to the directive name length in a descending
+  // order. This is to make sure that longer names are tried first, before
+  // any potential prefix (e.g. "target update" before "target").
+  std::sort(table, table + llvm::omp::Directive_enumSize,
+      [](auto &a, auto &b) { return a.first.size() > b.first.size(); });
+}
+
 template <typename Clause, typename Separator> struct ModifierList {
   constexpr ModifierList(Separator sep) : sep_(sep) {}
   constexpr ModifierList(const ModifierList &) = default;
@@ -136,6 +181,9 @@ TYPE_PARSER(construct<OmpIterator>( //
     "ITERATOR" >>
     parenthesized(nonemptyList(sourced(Parser<OmpIteratorSpecifier>{})))))
 
+TYPE_PARSER(construct<OmpLastprivateModifier>(
+    "CONDITIONAL" >> pure(OmpLastprivateModifier::Value::Conditional)))
+
 // 2.15.3.7 LINEAR (linear-list: linear-step)
 //          linear-list -> list | modifier(list)
 //          linear-modifier -> REF | VAL | UVAL
@@ -232,6 +280,11 @@ TYPE_PARSER(sourced(construct<OmpFromClause::Modifier>(
 TYPE_PARSER(sourced(
     construct<OmpGrainsizeClause::Modifier>(Parser<OmpPrescriptiveness>{})))
 
+TYPE_PARSER(sourced(construct<OmpIfClause::Modifier>(OmpDirectiveNameParser{})))
+
+TYPE_PARSER(sourced(construct<OmpLastprivateClause::Modifier>(
+    Parser<OmpLastprivateModifier>{})))
+
 TYPE_PARSER(sourced(construct<OmpMapClause::Modifier>(
     sourced(construct<OmpMapClause::Modifier>(Parser<OmpMapTypeModifier>{}) ||
         construct<OmpMapClause::Modifier>(Parser<OmpMapper>{}) ||
@@ -345,22 +398,7 @@ TYPE_PARSER(construct<OmpDeviceTypeClause>(
 
 // 2.12 IF (directive-name-modifier: scalar-logical-expr)
 TYPE_PARSER(construct<OmpIfClause>(
-    maybe(
-        ("PARALLEL" >> pure(OmpIfClause::DirectiveNameModifier::Parallel) ||
-            "SIMD" >> pure(OmpIfClause::DirectiveNameModifier::Simd) ||
-            "TARGET ENTER DATA" >>
-                pure(OmpIfClause::DirectiveNameModifier::TargetEnterData) ||
-            "TARGET EXIT DATA" >>
-                pure(OmpIfClause::DirectiveNameModifier::TargetExitData) ||
-            "TARGET DATA" >>
-                pure(OmpIfClause::DirectiveNameModifier::TargetData) ||
-            "TARGET UPDATE" >>
-                pure(OmpIfClause::DirectiveNameModifier::TargetUpdate) ||
-            "TARGET" >> pure(OmpIfClause::DirectiveNameModifier::Target) ||
-            "TASK"_id >> pure(OmpIfClause::DirectiveNameModifier::Task) ||
-            "TASKLOOP" >> pure(OmpIfClause::DirectiveNameModifier::Taskloop) ||
-            "TEAMS" >> pure(OmpIfClause::DirectiveNameModifier::Teams)) /
-        ":"),
+    maybe(nonemptyList(Parser<OmpIfClause::Modifier>{}) / ":"),
     scalarLogicalExpr))
 
 TYPE_PARSER(construct<OmpReductionClause>(
@@ -460,8 +498,7 @@ TYPE_PARSER(
 
 // OMP 5.0 2.19.4.5 LASTPRIVATE ([lastprivate-modifier :] list)
 TYPE_PARSER(construct<OmpLastprivateClause>(
-    maybe("CONDITIONAL" >>
-        pure(OmpLastprivateClause::LastprivateModifier::Conditional) / ":"),
+    maybe(nonemptyList(Parser<OmpLastprivateClause::Modifier>{}) / ":"),
     Parser<OmpObjectList>{}))
 
 // OMP 5.2 11.7.1 BIND ( PARALLEL | TEAMS | THREAD )
diff --git a/flang/lib/Parser/unparse.cpp b/flang/lib/Parser/unparse.cpp
index cd025333a077d3..43c7ef540cb7af 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2074,6 +2074,9 @@ class UnparseVisitor {
                   },
         x.u);
   }
+  void Unparse(const OmpDirectiveNameModifier &x) {
+    Word(llvm::omp::getOpenMPDirectiveName(x.v));
+  }
   void Unparse(const OmpIteratorSpecifier &x) {
     Walk(std::get<TypeDeclarationStmt>(x.t));
     Put(" = ");
@@ -2090,9 +2093,8 @@ class UnparseVisitor {
     Put(")");
   }
   void Unparse(const OmpLastprivateClause &x) {
-    Walk(
-        std::get<std::optional<OmpLastprivateClause::LastprivateModifier>>(x.t),
-        ":");
+    using Modifier = OmpLastprivateClause::Modifier;
+    Walk(std::get<std::optional<std::list<Modifier>>>(x.t), ": ");
     Walk(std::get<OmpObjectList>(x.t));
   }
   void Unparse(const OmpMapClause &x) {
@@ -2127,7 +2129,8 @@ class UnparseVisitor {
     Walk(std::get<OmpObjectList>(x.t));
   }
   void Unparse(const OmpIfClause &x) {
-    Walk(std::get<std::optional<OmpIfClause::DirectiveNameModifier>>(x.t), ":");
+    using Modifier = OmpIfClause::Modifier;
+    Walk(std::get<std::optional<std::list<Modifier>>>(x.t), ": ");
     Walk(std::get<ScalarLogicalExpr>(x.t));
   }
   void Unparse(const OmpLinearClause::WithoutModifier &x) {
@@ -2826,8 +2829,7 @@ class UnparseVisitor {
   WALK_NESTED_ENUM(OmpDefaultClause, DataSharingAttribute) // OMP default
   WALK_NESTED_ENUM(OmpDefaultmapClause, ImplicitBehavior) // OMP defaultmap
   WALK_NESTED_ENUM(OmpVariableCategory, Value) // OMP variable-category
-  WALK_NESTED_ENUM(
-      OmpLastprivateClause, LastprivateModifier) // OMP lastprivate-modifier
+  WALK_NESTED_ENUM(OmpLastprivateModifier, Value) // OMP lastprivate-modifier
   WALK_NESTED_ENUM(OmpChunkModifier, Value) // OMP chunk-modifier
   WALK_NESTED_ENUM(OmpLinearModifier, Value) // OMP linear-modifier
   WALK_NESTED_ENUM(OmpOrderingModifier, Value) // OMP ordering-modifier
@@ -2838,7 +2840,6 @@ class UnparseVisitor {
       OmpDeviceTypeClause, DeviceTypeDescription) // OMP device_type
   WALK_NESTED_ENUM(OmpReductionModifier, Value) // OMP reduction-modifier
   WALK_NESTED_ENUM(OmpExpectation, Value) // OMP motion-expectation
-  WALK_NESTED_ENUM(OmpIfClause, DirectiveNameModifier) // OMP directive-modifier
   WALK_NESTED_ENUM(OmpCancelType, Type) // OMP cancel-type
   WALK_NESTED_ENUM(OmpOrderClause, Ordering) // OMP ordering
   WALK_NESTED_ENUM(OmpOrderModifier, Value) // OMP order-modifier
diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index 9e589067c8868d..a88aba5a54a023 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -3428,36 +3428,81 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Defaultmap &x) {
 
 void OmpStructureChecker::Enter(const parser::OmpClause::If &x) {
   CheckAllowedClause(llvm::omp::Clause::OMPC_if);
-  using dirNameModifier = parser::OmpIfClause::DirectiveNameModifier;
-  // TODO Check that, when multiple 'if' clauses are applied to a combined
-  // construct, at most one of them applies to each directive.
-  static std::unordered_map<dirNameModifier, OmpDirectiveSet>
-      dirNameModifierMap{{dirNameModifier::Parallel, llvm::omp::allParallelSet},
-          {dirNameModifier::Simd, llvm::omp::allSimdSet},
-          {dirNameModifier::Target, llvm::omp::allTargetSet},
-          {dirNameModifier::TargetData,
-              {llvm::omp::Directive::OMPD_target_data}},
-          {dirNameModifier::TargetEnterData,
-              {llvm::omp::Directive::OMPD_target_enter_data}},
-          {dirNameModifier::TargetExitData,
-              {llvm::omp::Directive::OMPD_target_exit_data}},
-          {dirNameModifier::TargetUpdate,
-              {llvm::omp::Directive::OMPD_target_update}},
-          {dirNameModifier::Task, {llvm::omp::Directive::OMPD_task}},
-          {dirNameModifier::Taskloop, llvm::omp::allTaskloopSet},
-          {dirNameModifier::Teams, llvm::omp::allTeamsSet}};
-  if (const auto &directiveName{
-          std::get<std::optional<dirNameModifier>>(x.v.t)}) {
-    auto search{dirNameModifierMap.find(*directiveName)};
-    if (search == dirNameModifierMap.end() ||
-        !search->second.test(GetContext().directive)) {
-      context_
-          .Say(GetContext().clauseSource,
-              "Unmatched directive name modifier %s on the IF clause"_err_en_US,
-              parser::ToUpperCaseLetters(
-                  parser::OmpIfClause::EnumToString(*directiveName)))
-          .Attach(
-              GetContext().directiveSource, "Cannot apply to directive"_en_US);
+  unsigned version{context_.langOptions().OpenMPVersion};
+  llvm::omp::Directive dir{GetContext().directive};
+
+  auto isConstituent{[](llvm::omp::Directive dir, llvm::omp::Directive part) {
+    using namespace llvm::omp;
+    llvm::ArrayRef<Directive> dirLeafs{getLeafConstructsOrSelf(dir)};
+    llvm::ArrayRef<Directive> partLeafs{getLeafConstructsOrSelf(part)};
+    // Maybe it's sufficient to check if every leaf of `part` is also a leaf
+    // of `dir`, but to be safe check if `partLeafs` is a sub-sequence of
+    // `dirLeafs`.
+    size_t dirSize{dirLeafs.size()}, partSize{partLeafs.size()};
+    // Find the first leaf from `part` in `dir`.
+    if (auto first = llvm::find(dirLeafs, partLeafs.front());
+        first != dirLeafs.end()) {
+      // A leaf can only appear once in a compound directive, so if `part`
+      // is a subsequence of `dir`, it must start here.
+      long firstPos{std::distance(dirLeafs.begin(), first)};
+      llvm::ArrayRef<Directive> subSeq{
+          first, std::min<size_t>(dirSize - firstPos, partSize)};
+      return subSeq == partLeafs;
+    }
+    return false;
+  }};
+
+  if (OmpVerifyModifiers(
+          x.v, llvm::omp::OMPC_if, GetContext().clauseSource, context_)) {
+    auto &modifiers{OmpGetModifiers(x.v)};
+    if (auto *dnm{OmpGetUniqueModifier<parser::OmpDirectiveNameModifier>(
+            modifiers)}) {
+      llvm::omp::Directive sub{dnm->v};
+      std::string subName{parser::ToUpperCaseLetters(
+          llvm::omp::getOpenMPDirectiveName(sub).str())};
+      std::string dirName{parser::ToUpperCaseLetters(
+          llvm::omp::getOpenMPDirectiveName(dir).str())};
+
+      parser::CharBlock modifierSource{OmpGetModifierSource(modifiers, dnm)};
+      auto desc{OmpGetDescriptor<parser::OmpDirectiveNameModifier>()};
+      std::string modName{desc.name.str()};
+
+      if (!isConstituent(dir, sub)) {
+        context_
+            .Say(modifierSource,
+                "%s is not a constituent of the %s directive"_err_en_US,
+                subName, dirName)
+            .Attach(GetContext().directiveSource,
+                "Cannot apply to directive"_en_US);
+      } else {
+        static llvm::omp::Directive valid45[]{
+            llvm::omp::OMPD_cancel, //
+            llvm::omp::OMPD_parallel, //
+            /* OMP 5.0+ also allows OMPD_simd */
+            llvm::omp::OMPD_target, //
+            llvm::omp::OMPD_target_data, //
+            llvm::omp::OMPD_target_enter_data, //
+            llvm::omp::OMPD_target_exit_data, //
+            llvm::omp::OMPD_target_update, //
+            llvm::omp::OMPD_task, //
+            llvm::omp::OMPD_taskloop, //
+            /* OMP 5.2+ also allows OMPD_teams */
+        };
+        if (version < 50 && sub == llvm::omp::OMPD_simd) {
+          context_.Say(modifierSource,
+              "%s is not allowed as '%s' in %s, %s"_warn_en_US, subName,
+              modName, ThisVersion(version), TryVersion(50));
+        } else if (version < 52 && sub == llvm::omp::OMPD_teams) {
+          context_.Say(modifierSource,
+              "%s is not allowed as '%s' in %s, %s"_warn_en_US, subName,
+              modName, ThisVersion(version), TryVersion(52));
+        } else if (!llvm::is_contained(valid45, sub) &&
+            sub != llvm::omp::OMPD_simd && sub != llvm::omp::OMPD_teams) {
+          context_.Say(modifierSource,
+              "%s is not allowed as '%s' in %s"_err_en_US, subName, modName,
+              ThisVersion(version));
+        }
+      }
     }
   }
 }
@@ -3857,20 +3902,8 @@ void OmpStructureChecker::Enter(const parser::OmpClause::Lastprivate &x) {
   CheckPrivateSymbolsInOuterCxt(
       currSymbols, dirClauseTriple, llvm::omp::Clause::OMPC_lastprivate);
 
-  using LastprivateModifier = parser::OmpLastprivateClause::LastprivateModifier;
-  const auto &maybeMod{std::get<std::optional<LastprivateModifier>>(x.v.t)};
-  if (maybeMod) {
-    unsigned version{context_.langOptions().OpenMPVersion};
-    unsigned allowedInVersion = 50;
-    if (version < allowedInVersion) {
-      std::string thisVersion{
-          std::to_string(version / 10) + "." + std::to_string(version % 10)};
-      context_.Say(GetContext().clauseSource,
-          "LASTPRIVATE clause with CONDITIONAL modifier is not "
-          "allowed in %s, %s"_err_en_US,
-          ThisVersion(version), TryVersion(allowedInVersion));
-    }
-  }
+  OmpVerifyModifiers(
+      x.v, llvm::omp::OMPC_lastprivate, GetContext().clauseSource, context_);
 }
 
 void OmpStructureChecker::Enter(const parser::OmpClause::Copyin &x) {
diff --git a/flang/lib/Semantics/openmp-modifiers.cpp b/flang/lib/Semantics/openmp-modifiers.cpp
index e384b0270e6eae..f8f81e6c6ffa15 100644
--- a/flang/lib/Semantics/openmp-modifiers.cpp
+++ b/flang/lib/Semantics/openmp-modifiers.cpp
@@ -190,6 +190,23 @@ const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpDeviceModifier>() {
   return desc;
 }
 
+template <>
+const OmpModifierDescriptor &
+OmpGetDescriptor<parser::OmpDirectiveNameModifier>() {
+  static const OmpModifierDescriptor desc{
+      /*name=*/"directive-name-modifier",
+      /*props=*/
+      {
+          {45, {OmpProperty::Unique}},
+      },
+      /*clauses=*/
+      {
+          {45, {Clause::OMPC_if}},
+      },
+  };
+  return desc;
+}
+
 template <>
 const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpExpectation>() {
   static const OmpModifierDescriptor desc{
@@ -225,6 +242,23 @@ const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpIterator>() {
   return desc;
 }
 
+template <>
+const OmpModifierDescriptor &
+OmpGetDescriptor<parser::OmpLastprivateModifier>() {
+  static const OmpModifierDescriptor desc{
+      /*name=*/"lastprivate-modifier",
+      /*props=*/
+      {
+          {50, {OmpProperty::Unique}},
+      },
+      /*clauses=*/
+      {
+          {50, {Clause::OMPC_lastprivate}},
+      },
+  };
+  return desc;
+}
+
 template <>
 const OmpModifierDescriptor &OmpGetDescriptor<parser::OmpLinearModifier>() {
   static const OmpModifierDescriptor desc{
diff --git a/flang/test/Parser/OpenMP/if-clause-unparse.f90 b/flang/test/Parser/OpenMP/if-clause-unparse.f90
index 20f7b36e743e1a..ce058caa54a938 100644
--- a/flang/test/Parser/OpenMP/if-clause-unparse.f90
+++ b/flang/test/Parser/OpenMP/if-clause-unparse.f90
@@ -10,50 +10,50 @@ program if_unparse
   !$omp target update if(cond)
 
   ! CHECK: !$OMP TARGET UPDATE
-  ! CHECK-SAME: IF(TARGETUPDATE:cond)
+  ! CHECK-SAME: IF(TARGET UPDATE: cond)
   !$omp target update if(target update: cond)
   
   ! CHECK: !$OMP TARGET UPDATE
-  ! CHECK-SAME: IF(TARGETUPDATE:cond)
+  ! CHECK-SAME: IF(TARGET UPDATE: cond)
   !$omp target update if(targetupdate: cond)
 
   ! CHECK: !$OMP TARGET ENTER DATA
-  ! CHECK-SAME: IF(TARGETENTERDATA:cond)
+  ! CHECK-SAME: IF(TARGET ENTER DATA: cond)
   !$omp target enter data map(to: i) if(target enter data: cond)
 
   ! CHECK: !$OMP TARGET EXIT DATA
-  ! CHECK-SAME: IF(TARGETEXITDATA:cond)
+  ! CHECK-SAME: IF(TARGET EXIT DATA: cond)
   !$omp target exit data map(from: i) if(target exit data: cond)
 
   ! CHECK: !$OMP TARGET DATA
-  ! CHECK-SAME: IF(TARGETDATA:cond)
+  ! CHECK-SAME: IF(TARGET DATA: cond)
   !$omp target data map(tofrom: i) if(target data: cond)
   !$omp end target data
 
   ! CHECK: !$OMP TARGET
-  ! CHECK-SAME: IF(TARGET:cond)
+  ! CHECK-SAME: IF(TARGET: cond)
   !$omp target if(target: cond)
   !$omp end target
 
   ! CHECK: !$OMP TEAMS
-  ! CHECK-SAME: IF(TEAMS:cond)
+  ! CHECK-SAME: IF(TEAMS: cond)
   !$omp teams if(teams: cond)
   !$omp end teams
 
   ! CHECK: !$OMP PARALLEL DO SIMD
-  ! CHECK-SAME: IF(PARALLEL:i<10) IF(SIMD:.FALSE.)
+  ! CHECK-SAME: IF(PARALLEL: i<10) IF(SIMD: .FALSE.)
   !$omp parallel do simd if(parallel: i < 10) if(simd: .false.)
   do i = 1, 10
   end do
   !$omp end parallel do simd
 
   ! CHECK: !$OMP TASK
-  ! CHECK-SAME: IF(TASK:cond)
+  ! CHECK-SAME: IF(TASK: cond)
   !$omp task if(task: cond)
   !$omp end task
 
   ! CHECK: !$OMP TASKLOOP
-  ! CHECK-SAME: IF(TASKLOOP:cond)
+  ! CHECK-SAME: IF(TASKLOOP: cond)
   !$omp taskloop if(taskloop: cond)
   do i = 1, 10
   end do
diff --git a/flang/test/Parser/OpenMP/if-clause.f90 b/flang/test/Parser/OpenMP/if-clause.f90
index 6d69e16e7cc731..b3e3913f8bd1cf 100644
--- a/flang/test/Parser/OpenMP/if-clause.f90
+++ b/flang/test/Parser/OpenMP/if-clause.f90
@@ -1,4 +1,4 @@
-! RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp %s | FileCheck %s
+! RUN: %flang_fc1 -fdebug-dump-parse-tree -fopenmp -fopenmp-version=52 %s | FileCheck %s
 
 program openmp_parse_if
   logical :: cond
@@ -11,34 +11,34 @@ program openmp_parse_if
 
   ! CHECK: OmpSimpleStandaloneDirective -> llvm::omp::Directive = target update
   ! CHECK-NEXT: OmpClause -> If -> OmpIfClause
-  ! CHECK-NEXT: DirectiveNameModifier = TargetUpdate
+  ! CHECK-NEXT: OmpDirectiveNameModifier -> llvm::omp::Directive = target update
   !$omp target update if(target update: cond) to(i)
 
   ! CHECK: OmpSimpleStandaloneDirective -> llvm::omp::Directive = target enter data
   ! CHECK: OmpClause -> If -> OmpIfClause
-  ! CHECK-NEXT: DirectiveNameModifier = TargetEnterData
+  ! CHECK-NEXT: OmpDirectiveNameModifier -> llvm::omp::Directive = target enter data
   !$omp target enter data map(to: i) if(target enter data: cond)
 
   ! CHECK: OmpSimpleStandaloneDirective -> llvm::omp::Directive = target exit data
   ! CHECK: OmpClause -> If -> OmpIfClause
-  ! CHECK-NEXT: DirectiveNameModifier = TargetExitData
+  ! CHECK-NEXT: OmpDirectiveNameModifier -> llvm::omp::Directive = target exit data
   !$omp target exit data map(from: i) if(target exit data: cond)
 
   ! CHECK: OmpBlockDirective -> llvm::omp::Directive = target data
   ! CHECK: OmpClause -> If -> OmpIfClause
-  ! CHECK-NEXT: DirectiveNameModifier = TargetData
+  ! CHECK-NEXT: OmpDirectiveNameModifier -> llvm::omp::Directive = target data
   !$omp target data map(tofrom: i) if(target data: cond)
   !$omp end target data
 
   ! CHECK: OmpLoopDirective -> llvm::omp::Directive = target teams distribute parallel do simd
   ! CHECK: OmpClause -> If -> OmpIfClause
-  ! CHECK-NEXT: DirectiveNameModifier = Target
+  ! CHECK-NEXT: OmpDirectiveNameModifier -> llvm::omp::Directive = target
   ! CHECK: OmpClause -> If -> OmpIfClause
-  ! CHECK-NEXT: DirectiveNameModifier = Teams
+  ! CHECK-NEXT: OmpDirectiveNameModifier -> llvm::omp::Directive = teams
   ! CHECK: OmpClause -> If -> OmpIfClause
-  ! CHECK-NEXT: DirectiveNameModifier = Parallel
+  ! CHECK-NEXT: OmpDirectiveNameModifier -> llvm::omp::Directive = parallel
   ! CHECK: OmpClause -> If -> OmpIfClause
-  ! CHECK-NEXT: DirectiveNameModifier = Simd
+  ! CHECK-NEXT: OmpDirectiveNameModifier -> llvm::omp::Directive = simd
   !$omp target teams distribute parallel do simd if(target: cond) &
   !$omp&    if(teams: cond) if(parallel: cond) if(simd: cond)
   do i = 1, 10
@@ -47,13 +47,13 @@ program openmp_parse_if
 
   ! CHECK: OmpBlockDirective -> llvm::omp::Directive = task
   ! CHECK-NEXT: OmpClause -> If -> OmpIfClause
-  ! CHECK-NEXT: DirectiveNameModifier = Task
+  ! CHECK-NEXT: OmpDirectiveNameModifier -> llvm::omp::Directive = task
   !$omp task if(task: cond)
   !$omp end task
 
   ! CHECK: OmpLoopDirective -> llvm::omp::Directive = taskloop
   ! CHECK-NEXT: OmpClause -> If -> OmpIfClause
-  ! CHECK-NEXT: DirectiveNameModifier = Taskloop
+  ! CHECK-NEXT: DirectiveNameModifier -> llvm::omp::Directive = taskloop
   !$omp taskloop if(taskloop: cond)
   do i = 1, 10
   end do
diff --git a/flang/test/Parser/OpenMP/lastprivate-clause.f90 b/flang/test/Parser/OpenMP/lastprivate-clause.f90
index 382f02c75e7f14..ac25174f3cc427 100644
--- a/flang/test/Parser/OpenMP/lastprivate-clause.f90
+++ b/flang/test/Parser/OpenMP/lastprivate-clause.f90
@@ -39,7 +39,7 @@ subroutine foo2()
 !UNPARSE: SUBROUTINE foo2
 !UNPARSE:  INTEGER x, i
 !UNPARSE:   x=1_4
-!UNPARSE: !$OMP PARALLEL DO  LASTPRIVATE(CONDITIONAL:x)
+!UNPARSE: !$OMP PARALLEL DO  LASTPRIVATE(CONDITIONAL: x)
 !UNPARSE:  DO i=1_4,100_4
 !UNPARSE:    x=x+1_4
 !UNPARSE:  END DO
@@ -49,6 +49,6 @@ subroutine foo2()
 !PARSE-TREE:   Name = 'foo2'
 !PARSE-TREE: OmpLoopDirective -> llvm::omp::Directive = parallel do
 !PARSE-TREE: OmpClauseList -> OmpClause -> Lastprivate -> OmpLastprivateClause
-!PARSE-TREE:   LastprivateModifier = Conditional
+!PARSE-TREE:   Modifier -> OmpLastprivateModifier -> Value = Conditional
 !PARSE-TREE:   OmpObjectList -> OmpObject -> Designator -> DataRef -> Name = 'x'
 !PARSE-TREE: EndSubroutineStmt
diff --git a/flang/test/Semantics/OpenMP/clause-validity01.f90 b/flang/test/Semantics/OpenMP/clause-validity01.f90
index bc9d2d37060fc2..66e11e4b540f0f 100644
--- a/flang/test/Semantics/OpenMP/clause-validity01.f90
+++ b/flang/test/Semantics/OpenMP/clause-validity01.f90
@@ -241,7 +241,7 @@
   enddo
   !$omp end parallel do simd
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the PARALLEL DO directive
   !$omp parallel do if(target:a>1.)
   do i = 1, N
   enddo
@@ -532,7 +532,7 @@
   a = 1.
   !$omp end task
 
-  !ERROR: Unmatched directive name modifier TASKLOOP on the IF clause
+  !ERROR: TASKLOOP is not a constituent of the TASK directive
   !$omp task private(a) if(taskloop:a.eq.1)
   a = 1.
   !$omp end task
diff --git a/flang/test/Semantics/OpenMP/if-clause.f90 b/flang/test/Semantics/OpenMP/if-clause.f90
index 7aeb617e535630..23be4a751c8926 100644
--- a/flang/test/Semantics/OpenMP/if-clause.f90
+++ b/flang/test/Semantics/OpenMP/if-clause.f90
@@ -18,7 +18,7 @@ program main
   end do
   !$omp end distribute parallel do
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the DISTRIBUTE PARALLEL DO directive
   !$omp distribute parallel do if(target: .true.)
   do i = 1, 10
   end do
@@ -45,7 +45,7 @@ program main
   end do
   !$omp end distribute parallel do simd
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the DISTRIBUTE PARALLEL DO SIMD directive
   !$omp distribute parallel do simd if(target: .true.)
   do i = 1, 10
   end do
@@ -66,7 +66,7 @@ program main
   end do
   !$omp end distribute simd
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the DISTRIBUTE SIMD directive
   !$omp distribute simd if(target: .true.)
   do i = 1, 10
   end do
@@ -92,7 +92,7 @@ program main
   end do
   !$omp end do simd
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the DO SIMD directive
   !$omp do simd if(target: .true.)
   do i = 1, 10
   end do
@@ -113,7 +113,7 @@ program main
   !$omp parallel if(parallel: .true.)
   !$omp end parallel
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the PARALLEL directive
   !$omp parallel if(target: .true.)
   !$omp end parallel
 
@@ -134,7 +134,7 @@ program main
   end do
   !$omp end parallel do
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the PARALLEL DO directive
   !$omp parallel do if(target: .true.)
   do i = 1, 10
   end do
@@ -159,7 +159,7 @@ program main
   end do
   !$omp end parallel do simd
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the PARALLEL DO SIMD directive
   !$omp parallel do simd if(target: .true.)
   do i = 1, 10
   end do
@@ -174,7 +174,7 @@ program main
   !$omp parallel sections if(parallel: .true.)
   !$omp end parallel sections
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the PARALLEL SECTIONS directive
   !$omp parallel sections if(target: .true.)
   !$omp end parallel sections
 
@@ -191,7 +191,7 @@ program main
   !$omp parallel workshare if(parallel: .true.)
   !$omp end parallel workshare
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the PARALLEL WORKSHARE directive
   !$omp parallel workshare if(target: .true.)
   !$omp end parallel workshare
 
@@ -212,7 +212,7 @@ program main
   end do
   !$omp end simd
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the SIMD directive
   !$omp simd if(target: .true.)
   do i = 1, 10
   end do
@@ -233,7 +233,7 @@ program main
   !$omp target if(target: .true.)
   !$omp end target
 
-  !ERROR: Unmatched directive name modifier PARALLEL on the IF clause
+  !ERROR: PARALLEL is not a constituent of the TARGET directive
   !$omp target if(parallel: .true.)
   !$omp end target
 
@@ -250,7 +250,7 @@ program main
   !$omp target data map(tofrom: i) if(target data: .true.)
   !$omp end target data
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the TARGET DATA directive
   !$omp target data map(tofrom: i) if(target: .true.)
   !$omp end target data
 
@@ -265,7 +265,7 @@ program main
 
   !$omp target enter data map(to: i) if(target enter data: .true.)
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the TARGET ENTER DATA directive
   !$omp target enter data map(to: i) if(target: .true.)
 
   !ERROR: At most one IF clause can appear on the TARGET ENTER DATA directive
@@ -278,7 +278,7 @@ program main
 
   !$omp target exit data map(from: i) if(target exit data: .true.)
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the TARGET EXIT DATA directive
   !$omp target exit data map(from: i) if(target: .true.)
   
   !ERROR: At most one IF clause can appear on the TARGET EXIT DATA directive
@@ -293,7 +293,7 @@ program main
   !$omp target parallel if(target: .true.) if(parallel: .false.)
   !$omp end target parallel
 
-  !ERROR: Unmatched directive name modifier SIMD on the IF clause
+  !ERROR: SIMD is not a constituent of the TARGET PARALLEL directive
   !$omp target parallel if(simd: .true.)
   !$omp end target parallel
 
@@ -310,7 +310,7 @@ program main
   end do
   !$omp end target parallel do
 
-  !ERROR: Unmatched directive name modifier SIMD on the IF clause
+  !ERROR: SIMD is not a constituent of the TARGET PARALLEL DO directive
   !$omp target parallel do if(simd: .true.)
   do i = 1, 10
   end do
@@ -330,7 +330,7 @@ program main
   end do
   !$omp end target parallel do simd
 
-  !ERROR: Unmatched directive name modifier TEAMS on the IF clause
+  !ERROR: TEAMS is not a constituent of the TARGET PARALLEL DO SIMD directive
   !$omp target parallel do simd if(teams: .true.)
   do i = 1, 10
   end do
@@ -349,7 +349,7 @@ program main
   end do
   !$omp end target simd
 
-  !ERROR: Unmatched directive name modifier PARALLEL on the IF clause
+  !ERROR: PARALLEL is not a constituent of the TARGET SIMD directive
   !$omp target simd if(parallel: .true.)
   do i = 1, 10
   end do
@@ -364,7 +364,7 @@ program main
   !$omp target teams if(target: .true.) if(teams: .false.)
   !$omp end target teams
 
-  !ERROR: Unmatched directive name modifier PARALLEL on the IF clause
+  !ERROR: PARALLEL is not a constituent of the TARGET TEAMS directive
   !$omp target teams if(parallel: .true.)
   !$omp end target teams
 
@@ -381,7 +381,7 @@ program main
   end do
   !$omp end target teams distribute
 
-  !ERROR: Unmatched directive name modifier PARALLEL on the IF clause
+  !ERROR: PARALLEL is not a constituent of the TARGET TEAMS DISTRIBUTE directive
   !$omp target teams distribute if(parallel: .true.)
   do i = 1, 10
   end do
@@ -401,7 +401,7 @@ program main
   end do
   !$omp end target teams distribute parallel do
 
-  !ERROR: Unmatched directive name modifier SIMD on the IF clause
+  !ERROR: SIMD is not a constituent of the TARGET TEAMS DISTRIBUTE PARALLEL DO directive
   !$omp target teams distribute parallel do if(simd: .true.)
   do i = 1, 10
   end do
@@ -422,7 +422,7 @@ program main
   end do
   !$omp end target teams distribute parallel do simd
 
-  !ERROR: Unmatched directive name modifier TASK on the IF clause
+  !ERROR: TASK is not a constituent of the TARGET TEAMS DISTRIBUTE PARALLEL DO SIMD directive
   !$omp target teams distribute parallel do simd if(task: .true.)
   do i = 1, 10
   end do
@@ -442,7 +442,7 @@ program main
   end do
   !$omp end target teams distribute simd
 
-  !ERROR: Unmatched directive name modifier PARALLEL on the IF clause
+  !ERROR: PARALLEL is not a constituent of the TARGET TEAMS DISTRIBUTE SIMD directive
   !$omp target teams distribute simd if(parallel: .true.)
   do i = 1, 10
   end do
@@ -455,7 +455,7 @@ program main
   
   !$omp target update to(i) if(target update: .true.)
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the TARGET UPDATE directive
   !$omp target update to(i) if(target: .true.)
 
   !ERROR: At most one IF clause can appear on the TARGET UPDATE directive
@@ -470,7 +470,7 @@ program main
   !$omp task if(task: .true.)
   !$omp end task
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the TASK directive
   !$omp task if(target: .true.)
   !$omp end task
 
@@ -491,7 +491,7 @@ program main
   end do
   !$omp end taskloop
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the TASKLOOP directive
   !$omp taskloop if(target: .true.)
   do i = 1, 10
   end do
@@ -516,7 +516,7 @@ program main
   end do
   !$omp end taskloop simd
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the TASKLOOP SIMD directive
   !$omp taskloop simd if(target: .true.)
   do i = 1, 10
   end do
@@ -531,7 +531,7 @@ program main
   !$omp teams if(teams: .true.)
   !$omp end teams
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the TEAMS directive
   !$omp teams if(target: .true.)
   !$omp end teams
 
@@ -552,7 +552,7 @@ program main
   end do
   !$omp end teams distribute
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the TEAMS DISTRIBUTE directive
   !$omp teams distribute if(target: .true.)
   do i = 1, 10
   end do
@@ -577,7 +577,7 @@ program main
   end do
   !$omp end teams distribute parallel do
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the TEAMS DISTRIBUTE PARALLEL DO directive
   !$omp teams distribute parallel do if(target: .true.)
   do i = 1, 10
   end do
@@ -597,7 +597,7 @@ program main
   end do
   !$omp end teams distribute parallel do simd
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the TEAMS DISTRIBUTE PARALLEL DO SIMD directive
   !$omp teams distribute parallel do simd if(target: .true.)
   do i = 1, 10
   end do
@@ -616,7 +616,7 @@ program main
   end do
   !$omp end teams distribute simd
 
-  !ERROR: Unmatched directive name modifier TARGET on the IF clause
+  !ERROR: TARGET is not a constituent of the TEAMS DISTRIBUTE SIMD directive
   !$omp teams distribute simd if(target: .true.)
   do i = 1, 10
   end do

>From 1292aebcbf29e3c2740d86b399b0a2b1c8b1d225 Mon Sep 17 00:00:00 2001
From: Krzysztof Parzyszek <Krzysztof.Parzyszek at amd.com>
Date: Fri, 29 Nov 2024 14:27:55 -0600
Subject: [PATCH 11/11] add explicit cast

---
 flang/lib/Semantics/check-omp-structure.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/flang/lib/Semantics/check-omp-structure.cpp b/flang/lib/Semantics/check-omp-structure.cpp
index a88aba5a54a023..d722067d27bfd1 100644
--- a/flang/lib/Semantics/check-omp-structure.cpp
+++ b/flang/lib/Semantics/check-omp-structure.cpp
@@ -3444,7 +3444,8 @@ void OmpStructureChecker::Enter(const parser::OmpClause::If &x) {
         first != dirLeafs.end()) {
       // A leaf can only appear once in a compound directive, so if `part`
       // is a subsequence of `dir`, it must start here.
-      long firstPos{std::distance(dirLeafs.begin(), first)};
+      size_t firstPos{
+          static_cast<size_t>(std::distance(dirLeafs.begin(), first))};
       llvm::ArrayRef<Directive> subSeq{
           first, std::min<size_t>(dirSize - firstPos, partSize)};
       return subSeq == partLeafs;



More information about the flang-commits mailing list