[flang][OpenMP] Use new modifiers with AFFINITY/ALIGNED/DEVICE (#117786)

kparzysz · web-flow · commit 05096590e0ce · 2024-12-02T10:46:06.000-06:00
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.
diff --git 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)
diff --git 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)
@@ -596,7 +599,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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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;
@@ -2842,7 +2844,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
@@ -3399,10 +3399,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
 }
@@ -3624,19 +3629,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
@@ -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
diff --git a/flang/test/Parser/OpenMP/target_device_parse.f90 b/flang/test/Parser/OpenMP/target_device_parse.f90
diff --git a/flang/test/Parser/OpenMP/target_device_unparse.f90 b/flang/test/Parser/OpenMP/target_device_unparse.f90
diff --git a/flang/test/Semantics/OpenMP/clause-validity01.f90 b/flang/test/Semantics/OpenMP/clause-validity01.f90
