Skip to content

[flang][OpenMP] Parse DOACROSS clause #115396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 11, 2024

Conversation

kparzysz
Copy link
Contributor

@kparzysz kparzysz commented Nov 7, 2024

Extract the SINK/SOURCE parse tree elements into a separate class OmpDoacross, share them between DEPEND and DOACROSS clauses. Most of the changes in Semantics are to accommodate the new contents of OmpDependClause, and a mere introduction of OmpDoacrossClause.

There are no semantic checks specifically for DOACROSS.

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp flang:semantics flang:parser clang:openmp OpenMP related changes to Clang labels Nov 7, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 7, 2024

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

@llvm/pr-subscribers-flang-openmp

Author: Krzysztof Parzyszek (kparzysz)

Changes

Extract the SINK/SOURCE parse tree elements into a separate class OmpDoacross, share them between DEPEND and DOACROSS clauses. Most of the changes in Semantics are to accommodate the new contents of OmpDependClause, and a mere introduction of OmpDoacrossClause.

There are no semantic checks specifically for DOACROSS.


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

23 Files Affected:

  • (modified) flang/include/flang/Parser/dump-parse-tree.h (+12-7)
  • (modified) flang/include/flang/Parser/parse-tree.h (+58-21)
  • (modified) flang/lib/Lower/OpenMP/ClauseProcessor.cpp (+15-15)
  • (modified) flang/lib/Lower/OpenMP/Clauses.cpp (+61-51)
  • (modified) flang/lib/Lower/OpenMP/Clauses.h (+1-1)
  • (modified) flang/lib/Parser/openmp-parsers.cpp (+32-16)
  • (modified) flang/lib/Parser/parse-tree.cpp (+10-9)
  • (modified) flang/lib/Parser/unparse.cpp (+5-25)
  • (modified) flang/lib/Semantics/check-omp-structure.cpp (+113-82)
  • (modified) flang/lib/Semantics/check-omp-structure.h (+1)
  • (modified) flang/lib/Semantics/resolve-directives.cpp (+2-2)
  • (modified) flang/test/Parser/OpenMP/depobj-construct.f90 (+1-1)
  • (added) flang/test/Parser/OpenMP/doacross-clause.f90 (+90)
  • (added) flang/test/Parser/OpenMP/ordered-depend.f90 (+90)
  • (modified) flang/test/Semantics/OpenMP/clause-validity01.f90 (+1-1)
  • (modified) flang/test/Semantics/OpenMP/depend06.f90 (+2-2)
  • (modified) flang/test/Semantics/OpenMP/depobj-construct-v50.f90 (+1-1)
  • (modified) flang/test/Semantics/OpenMP/depobj-construct-v51.f90 (+2-2)
  • (modified) flang/test/Semantics/OpenMP/depobj-construct-v52.f90 (+2-2)
  • (modified) flang/test/Semantics/OpenMP/ordered01.f90 (+6-7)
  • (modified) flang/test/Semantics/OpenMP/ordered03.f90 (+2-2)
  • (modified) llvm/include/llvm/Frontend/OpenMP/ClauseT.h (+9-8)
  • (modified) llvm/include/llvm/Frontend/OpenMP/OMP.td (+1)
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index bfeb23de535392..f7730141ecf928 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -509,15 +509,20 @@ class ParseTreeDumper {
   NODE(parser, OmpDefaultmapClause)
   NODE_ENUM(OmpDefaultmapClause, ImplicitBehavior)
   NODE_ENUM(OmpDefaultmapClause, VariableCategory)
-  NODE(parser, OmpDependClause)
-  NODE(parser, OmpDetachClause)
-  NODE(OmpDependClause, InOut)
-  NODE(OmpDependClause, Sink)
-  NODE(OmpDependClause, Source)
+  NODE(parser, OmpDependenceType)
+  NODE_ENUM(OmpDependenceType, Type)
   NODE(parser, OmpTaskDependenceType)
   NODE_ENUM(OmpTaskDependenceType, Type)
-  NODE(parser, OmpDependSinkVec)
-  NODE(parser, OmpDependSinkVecLength)
+  NODE(parser, OmpIterationOffset)
+  NODE(parser, OmpIteration)
+  NODE(parser, OmpIterationVector)
+  NODE(parser, OmpDoacross)
+  NODE(OmpDoacross, Sink)
+  NODE(OmpDoacross, Source)
+  NODE(parser, OmpDependClause)
+  NODE(OmpDependClause, TaskDep)
+  NODE(parser, OmpDetachClause)
+  NODE(parser, OmpDoacrossClause)
   NODE(parser, OmpDestroyClause)
   NODE(parser, OmpEndAllocators)
   NODE(parser, OmpEndAtomic)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index d2c5b45d995813..e0369426364ede 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3439,16 +3439,35 @@ struct OmpObject {
 
 WRAPPER_CLASS(OmpObjectList, std::list<OmpObject>);
 
+// Ref: [4.5:169-170], [5.0:255-256], [5.1:288-289]
+//
+// dependence-type ->
+//    SINK | SOURCE |           // since 4.5
+//    IN | OUT | INOUT |        // since 4.5, until 5.1
+//    MUTEXINOUTSET | DEPOBJ |  // since 5.0, until 5.1
+//    INOUTSET                  // since 5.1, until 5.1
+//
+// All of these, except SINK and SOURCE became task-dependence-type in 5.2.
+//
+// Keeping these two as separate types, since having them all together
+// creates conflicts when parsing the DEPEND clause. For DEPEND(SINK: ...),
+// the SINK may be parsed as 'task-dependence-type', and the list after
+// the ':' would then be parsed as OmpObjectList (instead of the iteration
+// vector). This would accept the vector "i, j, k" (although interpreted
+// incorrectly), while flagging a syntax error for "i+1, j, k".
+struct OmpDependenceType {
+  ENUM_CLASS(Type, Sink, Source);
+  WRAPPER_CLASS_BOILERPLATE(OmpDependenceType, Type);
+};
+
 // Ref: [4.5:169-170], [5.0:254-256], [5.1:287-289], [5.2:321]
 //
 // task-dependence-type -> // "dependence-type" in 5.1 and before
 //    IN | OUT | INOUT |        // since 4.5
-//    SOURCE | SINK |           // since 4.5, until 5.1
 //    MUTEXINOUTSET | DEPOBJ |  // since 5.0
 //    INOUTSET                  // since 5.2
 struct OmpTaskDependenceType {
-  ENUM_CLASS(
-      Type, In, Out, Inout, Inoutset, Mutexinoutset, Source, Sink, Depobj)
+  ENUM_CLASS(Type, In, Out, Inout, Inoutset, Mutexinoutset, Depobj)
   WRAPPER_CLASS_BOILERPLATE(OmpTaskDependenceType, Type);
 };
 
@@ -3528,41 +3547,55 @@ struct OmpDefaultmapClause {
   std::tuple<ImplicitBehavior, std::optional<VariableCategory>> t;
 };
 
-// 2.13.9 depend-vec-length -> +/- non-negative-constant
-struct OmpDependSinkVecLength {
-  TUPLE_CLASS_BOILERPLATE(OmpDependSinkVecLength);
+// 2.13.9 iteration-offset -> +/- non-negative-constant
+struct OmpIterationOffset {
+  TUPLE_CLASS_BOILERPLATE(OmpIterationOffset);
   std::tuple<DefinedOperator, ScalarIntConstantExpr> t;
 };
 
-// 2.13.9 depend-vec -> induction-variable [depend-vec-length], ...
-struct OmpDependSinkVec {
-  TUPLE_CLASS_BOILERPLATE(OmpDependSinkVec);
-  std::tuple<Name, std::optional<OmpDependSinkVecLength>> t;
+// 2.13.9 iteration -> induction-variable [iteration-offset]
+struct OmpIteration {
+  TUPLE_CLASS_BOILERPLATE(OmpIteration);
+  std::tuple<Name, std::optional<OmpIterationOffset>> t;
+};
+
+WRAPPER_CLASS(OmpIterationVector, std::list<OmpIteration>);
+
+// Extract this into a separate structure (instead of having it directly in
+// OmpDoacrossClause), so that the context in TYPE_CONTEXT_PARSER can be set
+// separately for OmpDependClause and OmpDoacrossClause.
+struct OmpDoacross {
+  OmpDependenceType::Type GetDepType() const;
+
+  WRAPPER_CLASS(Sink, OmpIterationVector);
+  EMPTY_CLASS(Source);
+  UNION_CLASS_BOILERPLATE(OmpDoacross);
+  std::variant<Sink, Source> u;
 };
 
 // Ref: [4.5:169-170], [5.0:255-256], [5.1:288-289], [5.2:323-324]
 //
 // depend-clause ->
 //    DEPEND(SOURCE) |                               // since 4.5, until 5.1
-//    DEPEND(SINK: depend-vec) |                     // since 4.5, until 5.1
-//    DEPEND([depend-modifier,]dependence-type: locator-list)   // since 4.5
+//    DEPEND(SINK: iteration-vector) |               // since 4.5, until 5.1
+//    DEPEND([depend-modifier,]
+//           task-dependence-type: locator-list)     // since 4.5
 //
 // depend-modifier -> iterator-modifier              // since 5.0
 struct OmpDependClause {
-  OmpTaskDependenceType::Type GetDepType() const;
-
   UNION_CLASS_BOILERPLATE(OmpDependClause);
-  EMPTY_CLASS(Source);
-  WRAPPER_CLASS(Sink, std::list<OmpDependSinkVec>);
-  struct InOut {
-    TUPLE_CLASS_BOILERPLATE(InOut);
+  struct TaskDep {
+    OmpTaskDependenceType::Type GetTaskDepType() const;
+    TUPLE_CLASS_BOILERPLATE(TaskDep);
     std::tuple<std::optional<OmpIteratorModifier>, OmpTaskDependenceType,
         OmpObjectList>
         t;
   };
-  std::variant<Source, Sink, InOut> u;
+  std::variant<TaskDep, OmpDoacross> u;
 };
 
+WRAPPER_CLASS(OmpDoacrossClause, OmpDoacross);
+
 // Ref: [5.0:254-255], [5.1:287-288], [5.2:73]
 //
 // destroy-clause ->
@@ -3775,8 +3808,12 @@ struct OmpNumTasksClause {
 
 // Ref: [5.0:254-255], [5.1:287-288], [5.2:321-322]
 //
-// update-clause -> UPDATE(task-dependence-type)    // since 5.0
-WRAPPER_CLASS(OmpUpdateClause, OmpTaskDependenceType);
+// update-clause -> UPDATE(dependence-type)       // since 5.0, until 5.1
+// update-clause -> UPDATE(task-dependence-type)  // since 5.2
+struct OmpUpdateClause {
+  UNION_CLASS_BOILERPLATE(OmpUpdateClause);
+  std::variant<OmpDependenceType, OmpTaskDependenceType> u;
+};
 
 // OpenMP Clauses
 struct OmpClause {
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index e768c1cbc0784a..72b9018f2d2808 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -122,28 +122,28 @@ genProcBindKindAttr(fir::FirOpBuilder &firOpBuilder,
 
 static mlir::omp::ClauseTaskDependAttr
 genDependKindAttr(lower::AbstractConverter &converter,
-                  const omp::clause::Depend::TaskDependenceType kind) {
+                  const omp::clause::DependenceType kind) {
   fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
   mlir::Location currentLocation = converter.getCurrentLocation();
 
   mlir::omp::ClauseTaskDepend pbKind;
   switch (kind) {
-  case omp::clause::Depend::TaskDependenceType::In:
+  case omp::clause::DependenceType::In:
     pbKind = mlir::omp::ClauseTaskDepend::taskdependin;
     break;
-  case omp::clause::Depend::TaskDependenceType::Out:
+  case omp::clause::DependenceType::Out:
     pbKind = mlir::omp::ClauseTaskDepend::taskdependout;
     break;
-  case omp::clause::Depend::TaskDependenceType::Inout:
+  case omp::clause::DependenceType::Inout:
     pbKind = mlir::omp::ClauseTaskDepend::taskdependinout;
     break;
-  case omp::clause::Depend::TaskDependenceType::Mutexinoutset:
-  case omp::clause::Depend::TaskDependenceType::Inoutset:
+  case omp::clause::DependenceType::Mutexinoutset:
+  case omp::clause::DependenceType::Inoutset:
     TODO(currentLocation, "INOUTSET and MUTEXINOUTSET are not supported yet");
     break;
-  case omp::clause::Depend::TaskDependenceType::Depobj:
-  case omp::clause::Depend::TaskDependenceType::Sink:
-  case omp::clause::Depend::TaskDependenceType::Source:
+  case omp::clause::DependenceType::Depobj:
+  case omp::clause::DependenceType::Sink:
+  case omp::clause::DependenceType::Source:
     llvm_unreachable("unhandled parser task dependence type");
     break;
   }
@@ -803,20 +803,20 @@ bool ClauseProcessor::processDepend(mlir::omp::DependClauseOps &result) const {
   auto process = [&](const omp::clause::Depend &clause,
                      const parser::CharBlock &) {
     using Depend = omp::clause::Depend;
-    if (!std::holds_alternative<Depend::DepType>(clause.u)) {
+    if (!std::holds_alternative<Depend::TaskDep>(clause.u)) {
       TODO(converter.getCurrentLocation(),
            "DEPEND clause with SINK or SOURCE is not supported yet");
     }
-    auto &depType = std::get<Depend::DepType>(clause.u);
-    auto kind = std::get<Depend::TaskDependenceType>(depType.t);
-    auto &objects = std::get<omp::ObjectList>(depType.t);
+    auto &taskDep = std::get<Depend::TaskDep>(clause.u);
+    auto depType = std::get<clause::DependenceType>(taskDep.t);
+    auto &objects = std::get<omp::ObjectList>(taskDep.t);
 
-    if (std::get<std::optional<omp::clause::Iterator>>(depType.t)) {
+    if (std::get<std::optional<omp::clause::Iterator>>(taskDep.t)) {
       TODO(converter.getCurrentLocation(),
            "Support for iterator modifiers is not implemented yet");
     }
     mlir::omp::ClauseTaskDependAttr dependTypeOperand =
-        genDependKindAttr(converter, kind);
+        genDependKindAttr(converter, depType);
     result.dependKinds.append(objects.size(), dependTypeOperand);
 
     for (const omp::Object &object : objects) {
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index 46caafeef8e4a8..f6633dd53f6f23 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -338,27 +338,32 @@ ReductionOperator makeReductionOperator(const parser::OmpReductionOperator &inp,
       inp.u);
 }
 
-clause::TaskDependenceType
-makeDepType(const parser::OmpTaskDependenceType &inp) {
+clause::DependenceType makeDepType(const parser::OmpDependenceType &inp) {
+  switch (inp.v) {
+  case parser::OmpDependenceType::Type::Sink:
+    return clause::DependenceType::Sink;
+  case parser::OmpDependenceType::Type::Source:
+    return clause::DependenceType::Source;
+  }
+  llvm_unreachable("Unexpected dependence type");
+}
+
+clause::DependenceType makeDepType(const parser::OmpTaskDependenceType &inp) {
   switch (inp.v) {
   case parser::OmpTaskDependenceType::Type::Depobj:
-    return clause::TaskDependenceType::Depobj;
+    return clause::DependenceType::Depobj;
   case parser::OmpTaskDependenceType::Type::In:
-    return clause::TaskDependenceType::In;
+    return clause::DependenceType::In;
   case parser::OmpTaskDependenceType::Type::Inout:
-    return clause::TaskDependenceType::Inout;
+    return clause::DependenceType::Inout;
   case parser::OmpTaskDependenceType::Type::Inoutset:
-    return clause::TaskDependenceType::Inoutset;
+    return clause::DependenceType::Inoutset;
   case parser::OmpTaskDependenceType::Type::Mutexinoutset:
-    return clause::TaskDependenceType::Mutexinoutset;
+    return clause::DependenceType::Mutexinoutset;
   case parser::OmpTaskDependenceType::Type::Out:
-    return clause::TaskDependenceType::Out;
-  case parser::OmpTaskDependenceType::Type::Sink:
-    return clause::TaskDependenceType::Sink;
-  case parser::OmpTaskDependenceType::Type::Source:
-    return clause::TaskDependenceType::Source;
+    return clause::DependenceType::Out;
   }
-  llvm_unreachable("Unexpected dependence type");
+  llvm_unreachable("Unexpected task dependence type");
 }
 
 // --------------------------------------------------------------------
@@ -574,49 +579,52 @@ Depend make(const parser::OmpClause::Depend &inp,
   // inp.v -> parser::OmpDependClause
   using wrapped = parser::OmpDependClause;
   using Variant = decltype(Depend::u);
-  // Iteration is the equivalent of parser::OmpDependSinkVec
+  // Iteration is the equivalent of parser::OmpIteration
   using Iteration = Doacross::Vector::value_type; // LoopIterationT
 
+  auto visitSource = [&](const parser::OmpDoacross::Source &) -> Variant {
+    return Doacross{{/*DependenceType=*/Doacross::DependenceType::Source,
+                     /*Vector=*/{}}};
+  };
+
+  auto visitSink = [&](const parser::OmpDoacross::Sink &s) -> Variant {
+    using IterOffset = parser::OmpIterationOffset;
+    auto convert2 = [&](const parser::OmpIteration &v) {
+      auto &t0 = std::get<parser::Name>(v.t);
+      auto &t1 = std::get<std::optional<IterOffset>>(v.t);
+
+      auto convert3 = [&](const IterOffset &u) {
+        auto &s0 = std::get<parser::DefinedOperator>(u.t);
+        auto &s1 = std::get<parser::ScalarIntConstantExpr>(u.t);
+        return Iteration::Distance{
+            {makeDefinedOperator(s0, semaCtx), makeExpr(s1, semaCtx)}};
+      };
+      return Iteration{{makeObject(t0, semaCtx), maybeApply(convert3, t1)}};
+    };
+    return Doacross{{/*DependenceType=*/Doacross::DependenceType::Sink,
+                     /*Vector=*/makeList(s.v.v, convert2)}};
+  };
+
+  auto visitTaskDep = [&](const wrapped::TaskDep &s) -> Variant {
+    auto &t0 = std::get<std::optional<parser::OmpIteratorModifier>>(s.t);
+    auto &t1 = std::get<parser::OmpTaskDependenceType>(s.t);
+    auto &t2 = std::get<parser::OmpObjectList>(s.t);
+
+    auto &&maybeIter =
+        maybeApply([&](auto &&s) { return makeIterator(s, semaCtx); }, t0);
+    return Depend::TaskDep{{/*DependenceType=*/makeDepType(t1),
+                            /*Iterator=*/std::move(maybeIter),
+                            /*LocatorList=*/makeObjects(t2, semaCtx)}};
+  };
+
   return Depend{Fortran::common::visit( //
       common::visitors{
           // Doacross
-          [&](const wrapped::Source &s) -> Variant {
-            return Doacross{
-                {/*DependenceType=*/Doacross::DependenceType::Source,
-                 /*Vector=*/{}}};
-          },
-          // Doacross
-          [&](const wrapped::Sink &s) -> Variant {
-            using DependLength = parser::OmpDependSinkVecLength;
-            auto convert2 = [&](const parser::OmpDependSinkVec &v) {
-              auto &t0 = std::get<parser::Name>(v.t);
-              auto &t1 = std::get<std::optional<DependLength>>(v.t);
-
-              auto convert3 = [&](const DependLength &u) {
-                auto &s0 = std::get<parser::DefinedOperator>(u.t);
-                auto &s1 = std::get<parser::ScalarIntConstantExpr>(u.t);
-                return Iteration::Distance{
-                    {makeDefinedOperator(s0, semaCtx), makeExpr(s1, semaCtx)}};
-              };
-              return Iteration{
-                  {makeObject(t0, semaCtx), maybeApply(convert3, t1)}};
-            };
-            return Doacross{{/*DependenceType=*/Doacross::DependenceType::Sink,
-                             /*Vector=*/makeList(s.v, convert2)}};
-          },
-          // Depend::DepType
-          [&](const wrapped::InOut &s) -> Variant {
-            auto &t0 =
-                std::get<std::optional<parser::OmpIteratorModifier>>(s.t);
-            auto &t1 = std::get<parser::OmpTaskDependenceType>(s.t);
-            auto &t2 = std::get<parser::OmpObjectList>(s.t);
-
-            auto &&maybeIter = maybeApply(
-                [&](auto &&s) { return makeIterator(s, semaCtx); }, t0);
-            return Depend::DepType{{/*TaskDependenceType=*/makeDepType(t1),
-                                    /*Iterator=*/std::move(maybeIter),
-                                    /*LocatorList=*/makeObjects(t2, semaCtx)}};
+          [&](const parser::OmpDoacross &s) -> Variant {
+            return common::visit(common::visitors{visitSink, visitSource}, s.u);
           },
+          // Depend::TaskDep
+          visitTaskDep,
       },
       inp.v.u)};
 }
@@ -1356,7 +1364,9 @@ Uniform make(const parser::OmpClause::Uniform &inp,
 Update make(const parser::OmpClause::Update &inp,
             semantics::SemanticsContext &semaCtx) {
   // inp.v -> parser::OmpUpdateClause
-  return Update{/*TaskDependenceType=*/makeDepType(inp.v.v)};
+  auto depType =
+      common::visit([](auto &&s) { return makeDepType(s); }, inp.v.u);
+  return Update{/*DependenceType=*/depType};
 }
 
 Use make(const parser::OmpClause::Use &inp,
diff --git a/flang/lib/Lower/OpenMP/Clauses.h b/flang/lib/Lower/OpenMP/Clauses.h
index 51180ebfe5745e..514f0d1ee466ac 100644
--- a/flang/lib/Lower/OpenMP/Clauses.h
+++ b/flang/lib/Lower/OpenMP/Clauses.h
@@ -152,7 +152,7 @@ using IteratorSpecifier = tomp::type::IteratorSpecifierT<TypeTy, IdTy, ExprTy>;
 using DefinedOperator = tomp::type::DefinedOperatorT<IdTy, ExprTy>;
 using ProcedureDesignator = tomp::type::ProcedureDesignatorT<IdTy, ExprTy>;
 using ReductionOperator = tomp::type::ReductionIdentifierT<IdTy, ExprTy>;
-using TaskDependenceType = tomp::type::TaskDependenceType;
+using DependenceType = tomp::type::DependenceType;
 
 // "Requires" clauses are handled early on, and the aggregated information
 // is stored in the Symbol details of modules, programs, and subprograms.
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 7a0ecc59a2c5c5..1a0f8acae4948b 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -392,12 +392,9 @@ TYPE_PARSER(construct<OmpAllocateClause>(
         ":"),
     Parser<OmpObjectList>{}))
 
-// 2.13.9 DEPEND (SOURCE | SINK : vec | (IN | OUT | INOUT) : list
-TYPE_PARSER(construct<OmpDependSinkVecLength>(
-    Parser<DefinedOperator>{}, scalarIntConstantExpr))
-
-TYPE_PARSER(
-    construct<OmpDependSinkVec>(name, maybe(Parser<OmpDependSinkVecLength>{})))
+TYPE_PARSER(construct<OmpDependenceType>(
+    "SINK" >> pure(OmpDependenceType::Type::Sink) ||
+    "SOURCE" >> pure(OmpDependenceType::Type::Source)))
 
 TYPE_PARSER(construct<OmpTaskDependenceType>(
     "DEPOBJ" >> pure(OmpTaskDependenceType::Type::Depobj) ||
@@ -405,18 +402,31 @@ TYPE_PARSER(construct<OmpTaskDependenceType>(
     "INOUT"_id >> pure(OmpTaskDependenceType::Type::Inout) ||
     "INOUTSET"_id >> pure(OmpTaskDependenceType::Type::Inoutset) ||
     "MUTEXINOUTSET" >> pure(OmpTaskDependenceType::Type::Mutexinoutset) ||
-    "OUT" >> pure(OmpTaskDependenceType::Type::Out) ||
-    "SINK" >> pure(OmpTaskDependenceType::Type::Sink) ||
-    "SOURCE" >> pure(OmpTaskDependenceType::Type::Source)))
+    "OUT" >> pure(OmpTaskDependenceType::Type::Out)))
+
+// iteration-offset -> +/- non-negative-constant-expr
+TYPE_PARSER(construct<OmpIterationOffset>(
+    Parser<DefinedOperator>{}, scalarIntConstantExpr))
+
+// iteration -> iteration-variable [+/- nonnegative-scalar-integer-constant]
+TYPE_PARSER(construct<OmpIteration>(name, maybe(Parser<OmpIterationOffset>{})))
+
+TYPE_PARSER(construct<OmpIterationVector>(nonemptyList(Parser<OmpIteration>{})))
+
+TYPE_PARSER(construct<OmpDoacross>(
+    construct<OmpDoacross>(construct<OmpDoacross::Sink>(
+        "SINK"_tok >> ":"_tok >> Parser<OmpIterationVector>{})) ||
+    construct<OmpDoacross>(construct<OmpDoacross::Source>("SOURCE"_tok))))
 
 TYPE_CONTEXT_PARSER("Omp Depend clause"_en_US,
-    construct<OmpDependClause>(construct<OmpDependClause::Sink>(
-        "SINK :" >> nonemptyList(Parser<OmpDependSinkVec>{}))) ||
-        construct<OmpDependClause>(
-            construct<OmpDependClause::Source>("SOURCE"_tok)) ||
-        construct<OmpDependClause>(construct<OmpDependClause::InOut>(
+    construct<OmpDependClause>(
+        construct<OmpDependClause>(construct<OmpDependClause::TaskDep>(
             maybe(Parser<OmpIteratorModifier>{} / ","_tok),
-            Parser<OmpTaskDependenceType>{} / ":", Parser<OmpObjectList>{})))
+            Parser<OmpTaskDependenceType>{} / ":", Parser<OmpObjectList>{})) ||
+        construct<OmpDependClause>(Parser<OmpDoacross>{})))
+
+TYPE_CONTEXT_PARSER("Omp Doacross clause"_en_US,
+    construct<OmpDoacrossClause>(Parser<OmpDoacross>{}))
 
 TYPE_PARSER(construct<OmpFromClause::Expectation>(
     "PRESENT" >> pure(OmpFromClause::Expectation::Present)))
@@ -466,6 +476,10 @@ TYPE_PARSER(construct<OmpDetachClause>(Parser<OmpObject>{}))
 TYPE_PARSER(construct<OmpAlignedClause>(
     Parser<OmpObjectList>{}, maybe(":" >> scalarIntConstantExpr)))
 
+TYPE_PARSER(construct<OmpUpdateClause>(
+    construct<OmpUpdateClause>(Parser<OmpDependenceType>{}) ||
+    construct<OmpUpdateClause>(Parser<OmpTaskDependenceType...
[truncated]

@llvmbot
Copy link
Member

llvmbot commented Nov 7, 2024

@llvm/pr-subscribers-flang-parser

Author: Krzysztof Parzyszek (kparzysz)

Changes

Extract the SINK/SOURCE parse tree elements into a separate class OmpDoacross, share them between DEPEND and DOACROSS clauses. Most of the changes in Semantics are to accommodate the new contents of OmpDependClause, and a mere introduction of OmpDoacrossClause.

There are no semantic checks specifically for DOACROSS.


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

23 Files Affected:

  • (modified) flang/include/flang/Parser/dump-parse-tree.h (+12-7)
  • (modified) flang/include/flang/Parser/parse-tree.h (+58-21)
  • (modified) flang/lib/Lower/OpenMP/ClauseProcessor.cpp (+15-15)
  • (modified) flang/lib/Lower/OpenMP/Clauses.cpp (+61-51)
  • (modified) flang/lib/Lower/OpenMP/Clauses.h (+1-1)
  • (modified) flang/lib/Parser/openmp-parsers.cpp (+32-16)
  • (modified) flang/lib/Parser/parse-tree.cpp (+10-9)
  • (modified) flang/lib/Parser/unparse.cpp (+5-25)
  • (modified) flang/lib/Semantics/check-omp-structure.cpp (+113-82)
  • (modified) flang/lib/Semantics/check-omp-structure.h (+1)
  • (modified) flang/lib/Semantics/resolve-directives.cpp (+2-2)
  • (modified) flang/test/Parser/OpenMP/depobj-construct.f90 (+1-1)
  • (added) flang/test/Parser/OpenMP/doacross-clause.f90 (+90)
  • (added) flang/test/Parser/OpenMP/ordered-depend.f90 (+90)
  • (modified) flang/test/Semantics/OpenMP/clause-validity01.f90 (+1-1)
  • (modified) flang/test/Semantics/OpenMP/depend06.f90 (+2-2)
  • (modified) flang/test/Semantics/OpenMP/depobj-construct-v50.f90 (+1-1)
  • (modified) flang/test/Semantics/OpenMP/depobj-construct-v51.f90 (+2-2)
  • (modified) flang/test/Semantics/OpenMP/depobj-construct-v52.f90 (+2-2)
  • (modified) flang/test/Semantics/OpenMP/ordered01.f90 (+6-7)
  • (modified) flang/test/Semantics/OpenMP/ordered03.f90 (+2-2)
  • (modified) llvm/include/llvm/Frontend/OpenMP/ClauseT.h (+9-8)
  • (modified) llvm/include/llvm/Frontend/OpenMP/OMP.td (+1)
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index bfeb23de535392..f7730141ecf928 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -509,15 +509,20 @@ class ParseTreeDumper {
   NODE(parser, OmpDefaultmapClause)
   NODE_ENUM(OmpDefaultmapClause, ImplicitBehavior)
   NODE_ENUM(OmpDefaultmapClause, VariableCategory)
-  NODE(parser, OmpDependClause)
-  NODE(parser, OmpDetachClause)
-  NODE(OmpDependClause, InOut)
-  NODE(OmpDependClause, Sink)
-  NODE(OmpDependClause, Source)
+  NODE(parser, OmpDependenceType)
+  NODE_ENUM(OmpDependenceType, Type)
   NODE(parser, OmpTaskDependenceType)
   NODE_ENUM(OmpTaskDependenceType, Type)
-  NODE(parser, OmpDependSinkVec)
-  NODE(parser, OmpDependSinkVecLength)
+  NODE(parser, OmpIterationOffset)
+  NODE(parser, OmpIteration)
+  NODE(parser, OmpIterationVector)
+  NODE(parser, OmpDoacross)
+  NODE(OmpDoacross, Sink)
+  NODE(OmpDoacross, Source)
+  NODE(parser, OmpDependClause)
+  NODE(OmpDependClause, TaskDep)
+  NODE(parser, OmpDetachClause)
+  NODE(parser, OmpDoacrossClause)
   NODE(parser, OmpDestroyClause)
   NODE(parser, OmpEndAllocators)
   NODE(parser, OmpEndAtomic)
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index d2c5b45d995813..e0369426364ede 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -3439,16 +3439,35 @@ struct OmpObject {
 
 WRAPPER_CLASS(OmpObjectList, std::list<OmpObject>);
 
+// Ref: [4.5:169-170], [5.0:255-256], [5.1:288-289]
+//
+// dependence-type ->
+//    SINK | SOURCE |           // since 4.5
+//    IN | OUT | INOUT |        // since 4.5, until 5.1
+//    MUTEXINOUTSET | DEPOBJ |  // since 5.0, until 5.1
+//    INOUTSET                  // since 5.1, until 5.1
+//
+// All of these, except SINK and SOURCE became task-dependence-type in 5.2.
+//
+// Keeping these two as separate types, since having them all together
+// creates conflicts when parsing the DEPEND clause. For DEPEND(SINK: ...),
+// the SINK may be parsed as 'task-dependence-type', and the list after
+// the ':' would then be parsed as OmpObjectList (instead of the iteration
+// vector). This would accept the vector "i, j, k" (although interpreted
+// incorrectly), while flagging a syntax error for "i+1, j, k".
+struct OmpDependenceType {
+  ENUM_CLASS(Type, Sink, Source);
+  WRAPPER_CLASS_BOILERPLATE(OmpDependenceType, Type);
+};
+
 // Ref: [4.5:169-170], [5.0:254-256], [5.1:287-289], [5.2:321]
 //
 // task-dependence-type -> // "dependence-type" in 5.1 and before
 //    IN | OUT | INOUT |        // since 4.5
-//    SOURCE | SINK |           // since 4.5, until 5.1
 //    MUTEXINOUTSET | DEPOBJ |  // since 5.0
 //    INOUTSET                  // since 5.2
 struct OmpTaskDependenceType {
-  ENUM_CLASS(
-      Type, In, Out, Inout, Inoutset, Mutexinoutset, Source, Sink, Depobj)
+  ENUM_CLASS(Type, In, Out, Inout, Inoutset, Mutexinoutset, Depobj)
   WRAPPER_CLASS_BOILERPLATE(OmpTaskDependenceType, Type);
 };
 
@@ -3528,41 +3547,55 @@ struct OmpDefaultmapClause {
   std::tuple<ImplicitBehavior, std::optional<VariableCategory>> t;
 };
 
-// 2.13.9 depend-vec-length -> +/- non-negative-constant
-struct OmpDependSinkVecLength {
-  TUPLE_CLASS_BOILERPLATE(OmpDependSinkVecLength);
+// 2.13.9 iteration-offset -> +/- non-negative-constant
+struct OmpIterationOffset {
+  TUPLE_CLASS_BOILERPLATE(OmpIterationOffset);
   std::tuple<DefinedOperator, ScalarIntConstantExpr> t;
 };
 
-// 2.13.9 depend-vec -> induction-variable [depend-vec-length], ...
-struct OmpDependSinkVec {
-  TUPLE_CLASS_BOILERPLATE(OmpDependSinkVec);
-  std::tuple<Name, std::optional<OmpDependSinkVecLength>> t;
+// 2.13.9 iteration -> induction-variable [iteration-offset]
+struct OmpIteration {
+  TUPLE_CLASS_BOILERPLATE(OmpIteration);
+  std::tuple<Name, std::optional<OmpIterationOffset>> t;
+};
+
+WRAPPER_CLASS(OmpIterationVector, std::list<OmpIteration>);
+
+// Extract this into a separate structure (instead of having it directly in
+// OmpDoacrossClause), so that the context in TYPE_CONTEXT_PARSER can be set
+// separately for OmpDependClause and OmpDoacrossClause.
+struct OmpDoacross {
+  OmpDependenceType::Type GetDepType() const;
+
+  WRAPPER_CLASS(Sink, OmpIterationVector);
+  EMPTY_CLASS(Source);
+  UNION_CLASS_BOILERPLATE(OmpDoacross);
+  std::variant<Sink, Source> u;
 };
 
 // Ref: [4.5:169-170], [5.0:255-256], [5.1:288-289], [5.2:323-324]
 //
 // depend-clause ->
 //    DEPEND(SOURCE) |                               // since 4.5, until 5.1
-//    DEPEND(SINK: depend-vec) |                     // since 4.5, until 5.1
-//    DEPEND([depend-modifier,]dependence-type: locator-list)   // since 4.5
+//    DEPEND(SINK: iteration-vector) |               // since 4.5, until 5.1
+//    DEPEND([depend-modifier,]
+//           task-dependence-type: locator-list)     // since 4.5
 //
 // depend-modifier -> iterator-modifier              // since 5.0
 struct OmpDependClause {
-  OmpTaskDependenceType::Type GetDepType() const;
-
   UNION_CLASS_BOILERPLATE(OmpDependClause);
-  EMPTY_CLASS(Source);
-  WRAPPER_CLASS(Sink, std::list<OmpDependSinkVec>);
-  struct InOut {
-    TUPLE_CLASS_BOILERPLATE(InOut);
+  struct TaskDep {
+    OmpTaskDependenceType::Type GetTaskDepType() const;
+    TUPLE_CLASS_BOILERPLATE(TaskDep);
     std::tuple<std::optional<OmpIteratorModifier>, OmpTaskDependenceType,
         OmpObjectList>
         t;
   };
-  std::variant<Source, Sink, InOut> u;
+  std::variant<TaskDep, OmpDoacross> u;
 };
 
+WRAPPER_CLASS(OmpDoacrossClause, OmpDoacross);
+
 // Ref: [5.0:254-255], [5.1:287-288], [5.2:73]
 //
 // destroy-clause ->
@@ -3775,8 +3808,12 @@ struct OmpNumTasksClause {
 
 // Ref: [5.0:254-255], [5.1:287-288], [5.2:321-322]
 //
-// update-clause -> UPDATE(task-dependence-type)    // since 5.0
-WRAPPER_CLASS(OmpUpdateClause, OmpTaskDependenceType);
+// update-clause -> UPDATE(dependence-type)       // since 5.0, until 5.1
+// update-clause -> UPDATE(task-dependence-type)  // since 5.2
+struct OmpUpdateClause {
+  UNION_CLASS_BOILERPLATE(OmpUpdateClause);
+  std::variant<OmpDependenceType, OmpTaskDependenceType> u;
+};
 
 // OpenMP Clauses
 struct OmpClause {
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index e768c1cbc0784a..72b9018f2d2808 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -122,28 +122,28 @@ genProcBindKindAttr(fir::FirOpBuilder &firOpBuilder,
 
 static mlir::omp::ClauseTaskDependAttr
 genDependKindAttr(lower::AbstractConverter &converter,
-                  const omp::clause::Depend::TaskDependenceType kind) {
+                  const omp::clause::DependenceType kind) {
   fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
   mlir::Location currentLocation = converter.getCurrentLocation();
 
   mlir::omp::ClauseTaskDepend pbKind;
   switch (kind) {
-  case omp::clause::Depend::TaskDependenceType::In:
+  case omp::clause::DependenceType::In:
     pbKind = mlir::omp::ClauseTaskDepend::taskdependin;
     break;
-  case omp::clause::Depend::TaskDependenceType::Out:
+  case omp::clause::DependenceType::Out:
     pbKind = mlir::omp::ClauseTaskDepend::taskdependout;
     break;
-  case omp::clause::Depend::TaskDependenceType::Inout:
+  case omp::clause::DependenceType::Inout:
     pbKind = mlir::omp::ClauseTaskDepend::taskdependinout;
     break;
-  case omp::clause::Depend::TaskDependenceType::Mutexinoutset:
-  case omp::clause::Depend::TaskDependenceType::Inoutset:
+  case omp::clause::DependenceType::Mutexinoutset:
+  case omp::clause::DependenceType::Inoutset:
     TODO(currentLocation, "INOUTSET and MUTEXINOUTSET are not supported yet");
     break;
-  case omp::clause::Depend::TaskDependenceType::Depobj:
-  case omp::clause::Depend::TaskDependenceType::Sink:
-  case omp::clause::Depend::TaskDependenceType::Source:
+  case omp::clause::DependenceType::Depobj:
+  case omp::clause::DependenceType::Sink:
+  case omp::clause::DependenceType::Source:
     llvm_unreachable("unhandled parser task dependence type");
     break;
   }
@@ -803,20 +803,20 @@ bool ClauseProcessor::processDepend(mlir::omp::DependClauseOps &result) const {
   auto process = [&](const omp::clause::Depend &clause,
                      const parser::CharBlock &) {
     using Depend = omp::clause::Depend;
-    if (!std::holds_alternative<Depend::DepType>(clause.u)) {
+    if (!std::holds_alternative<Depend::TaskDep>(clause.u)) {
       TODO(converter.getCurrentLocation(),
            "DEPEND clause with SINK or SOURCE is not supported yet");
     }
-    auto &depType = std::get<Depend::DepType>(clause.u);
-    auto kind = std::get<Depend::TaskDependenceType>(depType.t);
-    auto &objects = std::get<omp::ObjectList>(depType.t);
+    auto &taskDep = std::get<Depend::TaskDep>(clause.u);
+    auto depType = std::get<clause::DependenceType>(taskDep.t);
+    auto &objects = std::get<omp::ObjectList>(taskDep.t);
 
-    if (std::get<std::optional<omp::clause::Iterator>>(depType.t)) {
+    if (std::get<std::optional<omp::clause::Iterator>>(taskDep.t)) {
       TODO(converter.getCurrentLocation(),
            "Support for iterator modifiers is not implemented yet");
     }
     mlir::omp::ClauseTaskDependAttr dependTypeOperand =
-        genDependKindAttr(converter, kind);
+        genDependKindAttr(converter, depType);
     result.dependKinds.append(objects.size(), dependTypeOperand);
 
     for (const omp::Object &object : objects) {
diff --git a/flang/lib/Lower/OpenMP/Clauses.cpp b/flang/lib/Lower/OpenMP/Clauses.cpp
index 46caafeef8e4a8..f6633dd53f6f23 100644
--- a/flang/lib/Lower/OpenMP/Clauses.cpp
+++ b/flang/lib/Lower/OpenMP/Clauses.cpp
@@ -338,27 +338,32 @@ ReductionOperator makeReductionOperator(const parser::OmpReductionOperator &inp,
       inp.u);
 }
 
-clause::TaskDependenceType
-makeDepType(const parser::OmpTaskDependenceType &inp) {
+clause::DependenceType makeDepType(const parser::OmpDependenceType &inp) {
+  switch (inp.v) {
+  case parser::OmpDependenceType::Type::Sink:
+    return clause::DependenceType::Sink;
+  case parser::OmpDependenceType::Type::Source:
+    return clause::DependenceType::Source;
+  }
+  llvm_unreachable("Unexpected dependence type");
+}
+
+clause::DependenceType makeDepType(const parser::OmpTaskDependenceType &inp) {
   switch (inp.v) {
   case parser::OmpTaskDependenceType::Type::Depobj:
-    return clause::TaskDependenceType::Depobj;
+    return clause::DependenceType::Depobj;
   case parser::OmpTaskDependenceType::Type::In:
-    return clause::TaskDependenceType::In;
+    return clause::DependenceType::In;
   case parser::OmpTaskDependenceType::Type::Inout:
-    return clause::TaskDependenceType::Inout;
+    return clause::DependenceType::Inout;
   case parser::OmpTaskDependenceType::Type::Inoutset:
-    return clause::TaskDependenceType::Inoutset;
+    return clause::DependenceType::Inoutset;
   case parser::OmpTaskDependenceType::Type::Mutexinoutset:
-    return clause::TaskDependenceType::Mutexinoutset;
+    return clause::DependenceType::Mutexinoutset;
   case parser::OmpTaskDependenceType::Type::Out:
-    return clause::TaskDependenceType::Out;
-  case parser::OmpTaskDependenceType::Type::Sink:
-    return clause::TaskDependenceType::Sink;
-  case parser::OmpTaskDependenceType::Type::Source:
-    return clause::TaskDependenceType::Source;
+    return clause::DependenceType::Out;
   }
-  llvm_unreachable("Unexpected dependence type");
+  llvm_unreachable("Unexpected task dependence type");
 }
 
 // --------------------------------------------------------------------
@@ -574,49 +579,52 @@ Depend make(const parser::OmpClause::Depend &inp,
   // inp.v -> parser::OmpDependClause
   using wrapped = parser::OmpDependClause;
   using Variant = decltype(Depend::u);
-  // Iteration is the equivalent of parser::OmpDependSinkVec
+  // Iteration is the equivalent of parser::OmpIteration
   using Iteration = Doacross::Vector::value_type; // LoopIterationT
 
+  auto visitSource = [&](const parser::OmpDoacross::Source &) -> Variant {
+    return Doacross{{/*DependenceType=*/Doacross::DependenceType::Source,
+                     /*Vector=*/{}}};
+  };
+
+  auto visitSink = [&](const parser::OmpDoacross::Sink &s) -> Variant {
+    using IterOffset = parser::OmpIterationOffset;
+    auto convert2 = [&](const parser::OmpIteration &v) {
+      auto &t0 = std::get<parser::Name>(v.t);
+      auto &t1 = std::get<std::optional<IterOffset>>(v.t);
+
+      auto convert3 = [&](const IterOffset &u) {
+        auto &s0 = std::get<parser::DefinedOperator>(u.t);
+        auto &s1 = std::get<parser::ScalarIntConstantExpr>(u.t);
+        return Iteration::Distance{
+            {makeDefinedOperator(s0, semaCtx), makeExpr(s1, semaCtx)}};
+      };
+      return Iteration{{makeObject(t0, semaCtx), maybeApply(convert3, t1)}};
+    };
+    return Doacross{{/*DependenceType=*/Doacross::DependenceType::Sink,
+                     /*Vector=*/makeList(s.v.v, convert2)}};
+  };
+
+  auto visitTaskDep = [&](const wrapped::TaskDep &s) -> Variant {
+    auto &t0 = std::get<std::optional<parser::OmpIteratorModifier>>(s.t);
+    auto &t1 = std::get<parser::OmpTaskDependenceType>(s.t);
+    auto &t2 = std::get<parser::OmpObjectList>(s.t);
+
+    auto &&maybeIter =
+        maybeApply([&](auto &&s) { return makeIterator(s, semaCtx); }, t0);
+    return Depend::TaskDep{{/*DependenceType=*/makeDepType(t1),
+                            /*Iterator=*/std::move(maybeIter),
+                            /*LocatorList=*/makeObjects(t2, semaCtx)}};
+  };
+
   return Depend{Fortran::common::visit( //
       common::visitors{
           // Doacross
-          [&](const wrapped::Source &s) -> Variant {
-            return Doacross{
-                {/*DependenceType=*/Doacross::DependenceType::Source,
-                 /*Vector=*/{}}};
-          },
-          // Doacross
-          [&](const wrapped::Sink &s) -> Variant {
-            using DependLength = parser::OmpDependSinkVecLength;
-            auto convert2 = [&](const parser::OmpDependSinkVec &v) {
-              auto &t0 = std::get<parser::Name>(v.t);
-              auto &t1 = std::get<std::optional<DependLength>>(v.t);
-
-              auto convert3 = [&](const DependLength &u) {
-                auto &s0 = std::get<parser::DefinedOperator>(u.t);
-                auto &s1 = std::get<parser::ScalarIntConstantExpr>(u.t);
-                return Iteration::Distance{
-                    {makeDefinedOperator(s0, semaCtx), makeExpr(s1, semaCtx)}};
-              };
-              return Iteration{
-                  {makeObject(t0, semaCtx), maybeApply(convert3, t1)}};
-            };
-            return Doacross{{/*DependenceType=*/Doacross::DependenceType::Sink,
-                             /*Vector=*/makeList(s.v, convert2)}};
-          },
-          // Depend::DepType
-          [&](const wrapped::InOut &s) -> Variant {
-            auto &t0 =
-                std::get<std::optional<parser::OmpIteratorModifier>>(s.t);
-            auto &t1 = std::get<parser::OmpTaskDependenceType>(s.t);
-            auto &t2 = std::get<parser::OmpObjectList>(s.t);
-
-            auto &&maybeIter = maybeApply(
-                [&](auto &&s) { return makeIterator(s, semaCtx); }, t0);
-            return Depend::DepType{{/*TaskDependenceType=*/makeDepType(t1),
-                                    /*Iterator=*/std::move(maybeIter),
-                                    /*LocatorList=*/makeObjects(t2, semaCtx)}};
+          [&](const parser::OmpDoacross &s) -> Variant {
+            return common::visit(common::visitors{visitSink, visitSource}, s.u);
           },
+          // Depend::TaskDep
+          visitTaskDep,
       },
       inp.v.u)};
 }
@@ -1356,7 +1364,9 @@ Uniform make(const parser::OmpClause::Uniform &inp,
 Update make(const parser::OmpClause::Update &inp,
             semantics::SemanticsContext &semaCtx) {
   // inp.v -> parser::OmpUpdateClause
-  return Update{/*TaskDependenceType=*/makeDepType(inp.v.v)};
+  auto depType =
+      common::visit([](auto &&s) { return makeDepType(s); }, inp.v.u);
+  return Update{/*DependenceType=*/depType};
 }
 
 Use make(const parser::OmpClause::Use &inp,
diff --git a/flang/lib/Lower/OpenMP/Clauses.h b/flang/lib/Lower/OpenMP/Clauses.h
index 51180ebfe5745e..514f0d1ee466ac 100644
--- a/flang/lib/Lower/OpenMP/Clauses.h
+++ b/flang/lib/Lower/OpenMP/Clauses.h
@@ -152,7 +152,7 @@ using IteratorSpecifier = tomp::type::IteratorSpecifierT<TypeTy, IdTy, ExprTy>;
 using DefinedOperator = tomp::type::DefinedOperatorT<IdTy, ExprTy>;
 using ProcedureDesignator = tomp::type::ProcedureDesignatorT<IdTy, ExprTy>;
 using ReductionOperator = tomp::type::ReductionIdentifierT<IdTy, ExprTy>;
-using TaskDependenceType = tomp::type::TaskDependenceType;
+using DependenceType = tomp::type::DependenceType;
 
 // "Requires" clauses are handled early on, and the aggregated information
 // is stored in the Symbol details of modules, programs, and subprograms.
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 7a0ecc59a2c5c5..1a0f8acae4948b 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -392,12 +392,9 @@ TYPE_PARSER(construct<OmpAllocateClause>(
         ":"),
     Parser<OmpObjectList>{}))
 
-// 2.13.9 DEPEND (SOURCE | SINK : vec | (IN | OUT | INOUT) : list
-TYPE_PARSER(construct<OmpDependSinkVecLength>(
-    Parser<DefinedOperator>{}, scalarIntConstantExpr))
-
-TYPE_PARSER(
-    construct<OmpDependSinkVec>(name, maybe(Parser<OmpDependSinkVecLength>{})))
+TYPE_PARSER(construct<OmpDependenceType>(
+    "SINK" >> pure(OmpDependenceType::Type::Sink) ||
+    "SOURCE" >> pure(OmpDependenceType::Type::Source)))
 
 TYPE_PARSER(construct<OmpTaskDependenceType>(
     "DEPOBJ" >> pure(OmpTaskDependenceType::Type::Depobj) ||
@@ -405,18 +402,31 @@ TYPE_PARSER(construct<OmpTaskDependenceType>(
     "INOUT"_id >> pure(OmpTaskDependenceType::Type::Inout) ||
     "INOUTSET"_id >> pure(OmpTaskDependenceType::Type::Inoutset) ||
     "MUTEXINOUTSET" >> pure(OmpTaskDependenceType::Type::Mutexinoutset) ||
-    "OUT" >> pure(OmpTaskDependenceType::Type::Out) ||
-    "SINK" >> pure(OmpTaskDependenceType::Type::Sink) ||
-    "SOURCE" >> pure(OmpTaskDependenceType::Type::Source)))
+    "OUT" >> pure(OmpTaskDependenceType::Type::Out)))
+
+// iteration-offset -> +/- non-negative-constant-expr
+TYPE_PARSER(construct<OmpIterationOffset>(
+    Parser<DefinedOperator>{}, scalarIntConstantExpr))
+
+// iteration -> iteration-variable [+/- nonnegative-scalar-integer-constant]
+TYPE_PARSER(construct<OmpIteration>(name, maybe(Parser<OmpIterationOffset>{})))
+
+TYPE_PARSER(construct<OmpIterationVector>(nonemptyList(Parser<OmpIteration>{})))
+
+TYPE_PARSER(construct<OmpDoacross>(
+    construct<OmpDoacross>(construct<OmpDoacross::Sink>(
+        "SINK"_tok >> ":"_tok >> Parser<OmpIterationVector>{})) ||
+    construct<OmpDoacross>(construct<OmpDoacross::Source>("SOURCE"_tok))))
 
 TYPE_CONTEXT_PARSER("Omp Depend clause"_en_US,
-    construct<OmpDependClause>(construct<OmpDependClause::Sink>(
-        "SINK :" >> nonemptyList(Parser<OmpDependSinkVec>{}))) ||
-        construct<OmpDependClause>(
-            construct<OmpDependClause::Source>("SOURCE"_tok)) ||
-        construct<OmpDependClause>(construct<OmpDependClause::InOut>(
+    construct<OmpDependClause>(
+        construct<OmpDependClause>(construct<OmpDependClause::TaskDep>(
             maybe(Parser<OmpIteratorModifier>{} / ","_tok),
-            Parser<OmpTaskDependenceType>{} / ":", Parser<OmpObjectList>{})))
+            Parser<OmpTaskDependenceType>{} / ":", Parser<OmpObjectList>{})) ||
+        construct<OmpDependClause>(Parser<OmpDoacross>{})))
+
+TYPE_CONTEXT_PARSER("Omp Doacross clause"_en_US,
+    construct<OmpDoacrossClause>(Parser<OmpDoacross>{}))
 
 TYPE_PARSER(construct<OmpFromClause::Expectation>(
     "PRESENT" >> pure(OmpFromClause::Expectation::Present)))
@@ -466,6 +476,10 @@ TYPE_PARSER(construct<OmpDetachClause>(Parser<OmpObject>{}))
 TYPE_PARSER(construct<OmpAlignedClause>(
     Parser<OmpObjectList>{}, maybe(":" >> scalarIntConstantExpr)))
 
+TYPE_PARSER(construct<OmpUpdateClause>(
+    construct<OmpUpdateClause>(Parser<OmpDependenceType>{}) ||
+    construct<OmpUpdateClause>(Parser<OmpTaskDependenceType...
[truncated]

@kparzysz
Copy link
Contributor Author

kparzysz commented Nov 7, 2024

The successor PR: #115397

Copy link
Contributor

@kiranchandramohan kiranchandramohan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LG. Thanks Kryztof.

@@ -1138,7 +1138,7 @@ bool AccAttributeVisitor::Pre(const parser::OpenACCCombinedConstruct &x) {
static bool IsLastNameArray(const parser::Designator &designator) {
const auto &name{GetLastName(designator)};
const evaluate::DataRef dataRef{*(name.symbol)};
return common::visit(
return common::visit( //
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Is this for formatting?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but it looks like a leftover from some wip changes. Nothing else is changed in this function in the PR, so I'll remove it.

Extract the SINK/SOURCE parse tree elements into a separate class
`OmpDoacross`, share them between DEPEND and DOACROSS clauses.
Most of the changes in Semantics are to accommodate the new contents
of OmpDependClause, and a mere introduction of OmpDoacrossClause.

There are no semantic checks specifically for DOACROSS.
@kparzysz kparzysz force-pushed the users/kparzysz/spr/d01-flang-doacross-parse branch from b1fd415 to c5f29cd Compare November 11, 2024 13:41
@kparzysz
Copy link
Contributor Author

The code looked the same (except for the //) in the committed version, but when I ran clang-format on it, it reformatted this function. If I added a commit that simply deleted the extra comment, the pre-commit clang-format would complain about formatting of this function, so I squashed/force-pushed the PR to make it look like this function wasn't modified at all. Hopefully clang-format will leave it alone.

@kparzysz kparzysz merged commit f87737f into main Nov 11, 2024
8 checks passed
@kparzysz kparzysz deleted the users/kparzysz/spr/d01-flang-doacross-parse branch November 11, 2024 14:48
Groverkss pushed a commit to iree-org/llvm-project that referenced this pull request Nov 15, 2024
Extract the SINK/SOURCE parse tree elements into a separate class
`OmpDoacross`, share them between DEPEND and DOACROSS clauses. Most of
the changes in Semantics are to accommodate the new contents of
OmpDependClause, and a mere introduction of OmpDoacrossClause.

There are no semantic checks specifically for DOACROSS.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:openmp OpenMP related changes to Clang flang:fir-hlfir flang:openmp flang:parser flang:semantics flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants