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

kparzysz · 2024-11-26T20:41:42Z

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.

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.

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.

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.

llvmbot · 2024-11-26T20:42:16Z

@llvm/pr-subscribers-flang-fir-hlfir

@llvm/pr-subscribers-flang-openmp

Author: Krzysztof Parzyszek (kparzysz)

Changes

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.

Patch is 24.39 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/117786.diff

12 Files Affected:

(modified) flang/include/flang/Parser/dump-parse-tree.h (+6-1)
(modified) flang/include/flang/Parser/parse-tree.h (+35-10)
(modified) flang/include/flang/Semantics/openmp-modifiers.h (+2)
(modified) flang/lib/Lower/OpenMP/Clauses.cpp (+11-9)
(modified) flang/lib/Parser/openmp-parsers.cpp (+20-7)
(modified) flang/lib/Parser/unparse.cpp (+7-5)
(modified) flang/lib/Semantics/check-omp-structure.cpp (+26-15)
(modified) flang/lib/Semantics/openmp-modifiers.cpp (+32)
(modified) flang/test/Parser/OpenMP/affinity-clause.f90 (+2-2)
(modified) flang/test/Parser/OpenMP/target_device_parse.f90 (+8-8)
(modified) flang/test/Parser/OpenMP/target_device_unparse.f90 (+4-4)
(modified) flang/test/Semantics/OpenMP/clause-validity01.f90 (+2-2)

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: OmpBlock...
[truncated]

llvmbot · 2024-11-26T20:42:16Z

@llvm/pr-subscribers-flang-semantics

Author: Krzysztof Parzyszek (kparzysz)

Changes

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.

Patch is 24.39 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/117786.diff

12 Files Affected:

(modified) flang/include/flang/Parser/dump-parse-tree.h (+6-1)
(modified) flang/include/flang/Parser/parse-tree.h (+35-10)
(modified) flang/include/flang/Semantics/openmp-modifiers.h (+2)
(modified) flang/lib/Lower/OpenMP/Clauses.cpp (+11-9)
(modified) flang/lib/Parser/openmp-parsers.cpp (+20-7)
(modified) flang/lib/Parser/unparse.cpp (+7-5)
(modified) flang/lib/Semantics/check-omp-structure.cpp (+26-15)
(modified) flang/lib/Semantics/openmp-modifiers.cpp (+32)
(modified) flang/test/Parser/OpenMP/affinity-clause.f90 (+2-2)
(modified) flang/test/Parser/OpenMP/target_device_parse.f90 (+8-8)
(modified) flang/test/Parser/OpenMP/target_device_unparse.f90 (+4-4)
(modified) flang/test/Semantics/OpenMP/clause-validity01.f90 (+2-2)

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: OmpBlock...
[truncated]

llvmbot · 2024-11-26T20:42:17Z

@llvm/pr-subscribers-flang-parser

Author: Krzysztof Parzyszek (kparzysz)

Changes

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.

Patch is 24.39 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/117786.diff

12 Files Affected:

(modified) flang/include/flang/Parser/dump-parse-tree.h (+6-1)
(modified) flang/include/flang/Parser/parse-tree.h (+35-10)
(modified) flang/include/flang/Semantics/openmp-modifiers.h (+2)
(modified) flang/lib/Lower/OpenMP/Clauses.cpp (+11-9)
(modified) flang/lib/Parser/openmp-parsers.cpp (+20-7)
(modified) flang/lib/Parser/unparse.cpp (+7-5)
(modified) flang/lib/Semantics/check-omp-structure.cpp (+26-15)
(modified) flang/lib/Semantics/openmp-modifiers.cpp (+32)
(modified) flang/test/Parser/OpenMP/affinity-clause.f90 (+2-2)
(modified) flang/test/Parser/OpenMP/target_device_parse.f90 (+8-8)
(modified) flang/test/Parser/OpenMP/target_device_unparse.f90 (+4-4)
(modified) flang/test/Semantics/OpenMP/clause-validity01.f90 (+2-2)

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: OmpBlock...
[truncated]

…/spr/m09-affinity-etc

tblah

Thanks

kparzysz added 5 commits November 25, 2024 14:49

fix examples

af029c8

fix llvm unittests

88ac6d0

[flang][OpenMP] Use new modifiers with AFFINITY/ALIGNED/DEVICE

09b1270

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.

kparzysz requested review from tblah, kiranchandramohan and Leporacanthicus November 26, 2024 20:41

llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp flang:semantics flang:parser labels Nov 26, 2024

kparzysz added 3 commits November 26, 2024 16:03

fix examples

d520f1e

Merge branch 'users/kparzysz/spr/m08-rename-type' into users/kparzysz…

b19f12e

…/spr/m09-affinity-etc

fix examples

923638f

tblah approved these changes Dec 2, 2024

View reviewed changes

Base automatically changed from users/kparzysz/spr/m08-rename-type to main December 2, 2024 15:22

Merge branch 'main' into users/kparzysz/spr/m09-affinity-etc

9c3149c

kparzysz merged commit 0509659 into main Dec 2, 2024
8 checks passed

kparzysz deleted the users/kparzysz/spr/m09-affinity-etc branch December 2, 2024 16:46

kparzysz mentioned this pull request Dec 19, 2024

[flang][openmp] Adds Parser and Semantic Support for Interop Construct, and Init and Use Clauses. #120584

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

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

Uh oh!

kparzysz commented Nov 26, 2024

Uh oh!

llvmbot commented Nov 26, 2024 •

edited

Loading

Uh oh!

llvmbot commented Nov 26, 2024

Uh oh!

llvmbot commented Nov 26, 2024

Uh oh!

tblah left a comment

Uh oh!

Uh oh!

Uh oh!

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

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

Uh oh!

Conversation

kparzysz commented Nov 26, 2024

Uh oh!

llvmbot commented Nov 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Nov 26, 2024

Uh oh!

llvmbot commented Nov 26, 2024

Uh oh!

tblah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

llvmbot commented Nov 26, 2024 •

edited

Loading