[flang][OpenMP] Use new modifiers in ALLOCATE clause (#117627)

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.

21 files changed, 231 insertions, 206 deletions

diff --git a/flang/examples/FeatureList/FeatureList.cpp b/flang/examples/FeatureList/FeatureList.cpp
index 6ae92ac..f7086b9 100644
--- a/flang/examples/FeatureList/FeatureList.cpp
+++ b/flang/examples/FeatureList/FeatureList.cpp
@@ -516,10 +516,10 @@ public:
   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)
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 68f9406..d499b41 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -586,10 +586,10 @@ public:
   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 8d7119a..e9a02a8 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 60f116e..a6316cf 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 ace21c1..c251d8b 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 bf20f42..ddc91ef 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 5fac5c2..562685e 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 2040a3e..f231290 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 e170815..49e5d91 100644
--- a/flang/lib/Parser/unparse.cpp
+++ b/flang/lib/Parser/unparse.cpp
@@ -2148,35 +2148,21 @@ public:
     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 3733ebf..b49258d 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 1fd2358..18863b7 100644
--- a/flang/lib/Semantics/openmp-modifiers.cpp
+++ b/flang/lib/Semantics/openmp-modifiers.cpp
@@ -75,6 +75,56 @@ unsigned OmpModifierDescriptor::since(llvm::omp::Clause id) const {
 // 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{
       /*name=*/"chunk-modifier",
diff --git a/flang/lib/Semantics/resolve-directives.cpp b/flang/lib/Semantics/resolve-directives.cpp
index 55dd94c..7bee8ad 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 062a48b..5cd0230 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 2b9a72e9..a014b54 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 c75c522..ff92fa3 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 1d2e964..212e48f 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 d0e11ca..0e8366a 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 a975204..8e63512 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 358b1b1..cf544df 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 ce09bb5..07efd6fd 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
diff --git a/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp b/llvm/unittests/Frontend/OpenMPDecompositionTest.cpp
index f954113..8f195c4 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