Skip to content

[flang][OpenMP] Make all block constructs share the same structure #150486

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

Closed

Conversation

kparzysz
Copy link
Contributor

@kparzysz kparzysz commented Jul 24, 2025

The structure is

  • OmpBeginDirective (aka OmpDirectiveSpecification)
  • Block
  • optional (aka optional)

The OmpBeginDirective and OmpEndDirective are effectively different names for OmpDirectiveSpecification. They exist to allow the semantic analyses to distinguish between the beginning and the ending of a block construct without maintaining additional context.

The actual changes are in the parser: parse-tree.h and openmp-parser.cpp in particular. The rest is simply changing the way the directive/clause information is accessed (typically for the simpler).

All standalone and block constructs (except "critical") now use OmpDirectiveSpecification to store the directive/clause information.

The structure is
- OmpBeginDirective (aka OmpDirectiveSpecification)
- Block
- optional<OmpEndDirective> (aka optional<OmpDirectiveSpecification>)

The OmpBeginDirective and OmpEndDirective are effectively different
names for OmpDirectiveSpecification. They exist to allow the semantic
analyses to distinguish between the beginning and the ending of a block
construct without maintaining additional context.

The actual changes are in the parser: parse-tree.h and openmp-parser.cpp
in particular. The rest is simply changing the way the directive/clause
information is accessed (typically for the simpler).

All standalone and block constructs now use OmpDirectiveSpecification
to store the directive/clause information.
@kparzysz kparzysz requested review from tblah and Stylie777 July 24, 2025 18:14
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp flang:semantics flang:parser labels Jul 24, 2025
@llvmbot
Copy link
Member

llvmbot commented Jul 24, 2025

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

@llvm/pr-subscribers-flang-semantics

Author: Krzysztof Parzyszek (kparzysz)

Changes

The structure is

  • OmpBeginDirective (aka OmpDirectiveSpecification)
  • Block
  • optional<OmpEndDirective> (aka optional<OmpDirectiveSpecification>)

The OmpBeginDirective and OmpEndDirective are effectively different names for OmpDirectiveSpecification. They exist to allow the semantic analyses to distinguish between the beginning and the ending of a block construct without maintaining additional context.

The actual changes are in the parser: parse-tree.h and openmp-parser.cpp in particular. The rest is simply changing the way the directive/clause information is accessed (typically for the simpler).

All standalone and block constructs now use OmpDirectiveSpecification to store the directive/clause information.


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

38 Files Affected:

  • (modified) flang/include/flang/Parser/dump-parse-tree.h (+2-3)
  • (modified) flang/include/flang/Parser/openmp-utils.h (+2-9)
  • (modified) flang/include/flang/Parser/parse-tree.h (+43-40)
  • (modified) flang/lib/Lower/OpenMP/Atomic.cpp (+1-1)
  • (modified) flang/lib/Lower/OpenMP/DataSharingProcessor.cpp (+19-20)
  • (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+14-35)
  • (modified) flang/lib/Parser/openmp-parsers.cpp (+100-92)
  • (modified) flang/lib/Parser/parse-tree.cpp (+3-3)
  • (modified) flang/lib/Parser/unparse.cpp (+20-80)
  • (modified) flang/lib/Semantics/check-omp-atomic.cpp (+3-5)
  • (modified) flang/lib/Semantics/check-omp-loop.cpp (+5-9)
  • (modified) flang/lib/Semantics/check-omp-structure.cpp (+50-65)
  • (modified) flang/lib/Semantics/check-omp-structure.h (+3-3)
  • (modified) flang/lib/Semantics/resolve-directives.cpp (+21-13)
  • (modified) flang/lib/Semantics/resolve-names.cpp (+5-7)
  • (modified) flang/test/Parser/OpenMP/affinity-clause.f90 (+6-6)
  • (modified) flang/test/Parser/OpenMP/allocators-unparse.f90 (+4-4)
  • (modified) flang/test/Parser/OpenMP/atomic-compare.f90 (+9-9)
  • (modified) flang/test/Parser/OpenMP/atomic-end.f90 (+4-4)
  • (modified) flang/test/Parser/OpenMP/block-construct.f90 (+10-10)
  • (modified) flang/test/Parser/OpenMP/construct-prefix-conflict.f90 (+20-20)
  • (modified) flang/test/Parser/OpenMP/defaultmap-clause.f90 (+12-12)
  • (modified) flang/test/Parser/OpenMP/defaultmap-unparse.f90 (+18-18)
  • (modified) flang/test/Parser/OpenMP/dispatch.f90 (+4-4)
  • (modified) flang/test/Parser/OpenMP/if-clause.f90 (+2-2)
  • (modified) flang/test/Parser/OpenMP/in-reduction-clause.f90 (+6-6)
  • (modified) flang/test/Parser/OpenMP/map-modifiers-v60.f90 (+10-10)
  • (modified) flang/test/Parser/OpenMP/map-modifiers.f90 (+23-23)
  • (modified) flang/test/Parser/OpenMP/masked-unparse.f90 (+6-6)
  • (modified) flang/test/Parser/OpenMP/master-unparse.f90 (+4-4)
  • (modified) flang/test/Parser/OpenMP/openmp6-directive-spellings.f90 (+6-6)
  • (modified) flang/test/Parser/OpenMP/proc-bind.f90 (+2-2)
  • (modified) flang/test/Parser/OpenMP/scope.f90 (+4-4)
  • (modified) flang/test/Parser/OpenMP/target_device_parse.f90 (+32-32)
  • (modified) flang/test/Parser/OpenMP/task-reduction-clause.f90 (+2-2)
  • (modified) flang/test/Parser/OpenMP/task.f90 (+1-1)
  • (modified) flang/test/Semantics/OpenMP/clause-validity01.f90 (+1-2)
  • (modified) flang/test/Semantics/OpenMP/symbol08.f90 (+1-2)
diff --git a/flang/include/flang/Parser/dump-parse-tree.h b/flang/include/flang/Parser/dump-parse-tree.h
index 23e35d106c077..8c9da5005948e 100644
--- a/flang/include/flang/Parser/dump-parse-tree.h
+++ b/flang/include/flang/Parser/dump-parse-tree.h
@@ -534,10 +534,8 @@ class ParseTreeDumper {
   NODE(parser, OmpAtClause)
   NODE_ENUM(OmpAtClause, ActionTime)
   NODE_ENUM(OmpSeverityClause, Severity)
-  NODE(parser, OmpBeginBlockDirective)
   NODE(parser, OmpBeginLoopDirective)
   NODE(parser, OmpBeginSectionsDirective)
-  NODE(parser, OmpBlockDirective)
   static std::string GetNodeName(const llvm::omp::Directive &x) {
     return llvm::Twine("llvm::omp::Directive = ",
         llvm::omp::getOpenMPDirectiveName(x, llvm::omp::FallbackVersion))
@@ -584,7 +582,6 @@ class ParseTreeDumper {
   NODE(parser, OmpDetachClause)
   NODE(parser, OmpDoacrossClause)
   NODE(parser, OmpDestroyClause)
-  NODE(parser, OmpEndBlockDirective)
   NODE(parser, OmpEndCriticalDirective)
   NODE(parser, OmpEndLoopDirective)
   NODE(parser, OmpEndSectionsDirective)
@@ -705,6 +702,8 @@ class ParseTreeDumper {
   NODE(parser, OpenMPDeclarativeAssumes)
   NODE(parser, OmpAssumeDirective)
   NODE(parser, OmpEndAssumeDirective)
+  NODE(parser, OmpBeginDirective)
+  NODE(parser, OmpEndDirective)
   NODE(parser, OpenMPAtomicConstruct)
   NODE(parser, OpenMPBlockConstruct)
   NODE(parser, OpenMPCancelConstruct)
diff --git a/flang/include/flang/Parser/openmp-utils.h b/flang/include/flang/Parser/openmp-utils.h
index 579ea7d74957f..c95c7be4888d3 100644
--- a/flang/include/flang/Parser/openmp-utils.h
+++ b/flang/include/flang/Parser/openmp-utils.h
@@ -68,11 +68,6 @@ struct DirectiveNameScope {
     return MakeName(x.source, llvm::omp::Directive::OMPD_nothing);
   }
 
-  static OmpDirectiveName GetOmpDirectiveName(const OmpBeginBlockDirective &x) {
-    auto &dir{std::get<OmpBlockDirective>(x.t)};
-    return MakeName(dir.source, dir.v);
-  }
-
   static OmpDirectiveName GetOmpDirectiveName(const OmpBeginLoopDirective &x) {
     auto &dir{std::get<OmpLoopDirective>(x.t)};
     return MakeName(dir.source, dir.v);
@@ -98,10 +93,8 @@ struct DirectiveNameScope {
         return GetOmpDirectiveName(x.v);
       }
     } else if constexpr (TupleTrait<T>) {
-      if constexpr (std::is_same_v<T, OpenMPAllocatorsConstruct> ||
-          std::is_same_v<T, OpenMPAtomicConstruct> ||
-          std::is_same_v<T, OpenMPDispatchConstruct>) {
-        return std::get<OmpDirectiveSpecification>(x.t).DirName();
+      if constexpr (std::is_base_of_v<OmpBlockConstruct, T>) {
+        return std::get<OmpBeginDirective>(x.t).DirName();
       } else if constexpr (std::is_same_v<T, OmpAssumeDirective> ||
           std::is_same_v<T, OmpCriticalDirective> ||
           std::is_same_v<T, OmpDeclareVariantDirective> ||
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index 3a28f6f9731c3..a8638c5dd3bbc 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -4701,6 +4701,13 @@ struct OmpClauseList {
 
 // --- Directives and constructs
 
+#define INHERITED_TUPLE_CLASS_BOILERPLATE(classname, basename) \
+  template <typename... Ts, typename = common::NoLvalue<Ts...>> \
+  classname(Ts &&...args) : basename(std::move(args)...) {} \
+  classname(basename &&b) : basename(std::move(b)) {} \
+  using TupleTrait = std::true_type; \
+  BOILERPLATE(classname)
+
 struct OmpDirectiveSpecification {
   ENUM_CLASS(Flags, None, DeprecatedSyntax);
   TUPLE_CLASS_BOILERPLATE(OmpDirectiveSpecification);
@@ -4719,6 +4726,33 @@ struct OmpDirectiveSpecification {
       t;
 };
 
+// OmpBeginDirective and OmpEndDirective are needed for semantic analysis,
+// where some checks are done specifically for either the begin or the end
+// directive. The structure of both is identical, but the diffent types
+// allow to distinguish them in the type-based parse-tree visitor.
+struct OmpBeginDirective : public OmpDirectiveSpecification {
+  INHERITED_TUPLE_CLASS_BOILERPLATE(
+      OmpBeginDirective, OmpDirectiveSpecification);
+};
+
+struct OmpEndDirective : public OmpDirectiveSpecification {
+  INHERITED_TUPLE_CLASS_BOILERPLATE(OmpEndDirective, OmpDirectiveSpecification);
+};
+
+// Common base class for block-associated constructs.
+struct OmpBlockConstruct {
+  TUPLE_CLASS_BOILERPLATE(OmpBlockConstruct);
+  const OmpBeginDirective &BeginDir() const {
+    return std::get<OmpBeginDirective>(t);
+  }
+  const std::optional<OmpEndDirective> &EndDir() const {
+    return std::get<std::optional<OmpEndDirective>>(t);
+  }
+
+  CharBlock source;
+  std::tuple<OmpBeginDirective, Block, std::optional<OmpEndDirective>> t;
+};
+
 struct OmpMetadirectiveDirective {
   TUPLE_CLASS_BOILERPLATE(OmpMetadirectiveDirective);
   std::tuple<Verbatim, OmpClauseList> t;
@@ -4824,12 +4858,6 @@ struct OpenMPSectionsConstruct {
       t;
 };
 
-// OpenMP directive beginning or ending a block
-struct OmpBlockDirective {
-  WRAPPER_CLASS_BOILERPLATE(OmpBlockDirective, llvm::omp::Directive);
-  CharBlock source;
-};
-
 struct OmpDeclareVariantDirective {
   TUPLE_CLASS_BOILERPLATE(OmpDeclareVariantDirective);
   CharBlock source;
@@ -4954,12 +4982,9 @@ struct OpenMPExecutableAllocate {
 //    ALLOCATORS [allocate-clause...]
 //    block
 //    [END ALLOCATORS]
-struct OpenMPAllocatorsConstruct {
-  TUPLE_CLASS_BOILERPLATE(OpenMPAllocatorsConstruct);
-  CharBlock source;
-  std::tuple<OmpDirectiveSpecification, Block,
-      std::optional<OmpDirectiveSpecification>>
-      t;
+struct OpenMPAllocatorsConstruct : public OmpBlockConstruct {
+  INHERITED_TUPLE_CLASS_BOILERPLATE(
+      OpenMPAllocatorsConstruct, OmpBlockConstruct);
 };
 
 // 2.17.7 Atomic construct/2.17.8 Flush construct [OpenMP 5.0]
@@ -4973,15 +4998,11 @@ struct OmpMemoryOrderClause {
   CharBlock source;
 };
 
-struct OpenMPAtomicConstruct {
+struct OpenMPAtomicConstruct : public OmpBlockConstruct {
   llvm::omp::Clause GetKind() const;
   bool IsCapture() const;
   bool IsCompare() const;
-  TUPLE_CLASS_BOILERPLATE(OpenMPAtomicConstruct);
-  CharBlock source;
-  std::tuple<OmpDirectiveSpecification, Block,
-      std::optional<OmpDirectiveSpecification>>
-      t;
+  INHERITED_TUPLE_CLASS_BOILERPLATE(OpenMPAtomicConstruct, OmpBlockConstruct);
 
   // Information filled out during semantic checks to avoid duplication
   // of analyses.
@@ -5045,12 +5066,8 @@ struct OpenMPDepobjConstruct {
 //                    nocontext-clause |
 //                    novariants-clause |
 //                    nowait-clause
-struct OpenMPDispatchConstruct {
-  TUPLE_CLASS_BOILERPLATE(OpenMPDispatchConstruct);
-  CharBlock source;
-  std::tuple<OmpDirectiveSpecification, Block,
-      std::optional<OmpDirectiveSpecification>>
-      t;
+struct OpenMPDispatchConstruct : public OmpBlockConstruct {
+  INHERITED_TUPLE_CLASS_BOILERPLATE(OpenMPDispatchConstruct, OmpBlockConstruct);
 };
 
 // [4.5:162-165], [5.0:242-246], [5.1:275-279], [5.2:315-316], [6.0:498-500]
@@ -5105,22 +5122,8 @@ struct OmpEndLoopDirective {
   CharBlock source;
 };
 
-struct OmpBeginBlockDirective {
-  TUPLE_CLASS_BOILERPLATE(OmpBeginBlockDirective);
-  std::tuple<OmpBlockDirective, OmpClauseList> t;
-  CharBlock source;
-};
-
-struct OmpEndBlockDirective {
-  TUPLE_CLASS_BOILERPLATE(OmpEndBlockDirective);
-  std::tuple<OmpBlockDirective, OmpClauseList> t;
-  CharBlock source;
-};
-
-struct OpenMPBlockConstruct {
-  TUPLE_CLASS_BOILERPLATE(OpenMPBlockConstruct);
-  std::tuple<OmpBeginBlockDirective, Block, std::optional<OmpEndBlockDirective>>
-      t;
+struct OpenMPBlockConstruct : public OmpBlockConstruct {
+  INHERITED_TUPLE_CLASS_BOILERPLATE(OpenMPBlockConstruct, OmpBlockConstruct);
 };
 
 // OpenMP directives enclosing do loop
diff --git a/flang/lib/Lower/OpenMP/Atomic.cpp b/flang/lib/Lower/OpenMP/Atomic.cpp
index d4f83f57bd5d4..e536fcb822880 100644
--- a/flang/lib/Lower/OpenMP/Atomic.cpp
+++ b/flang/lib/Lower/OpenMP/Atomic.cpp
@@ -706,7 +706,7 @@ void Fortran::lower::omp::lowerAtomic(
   };
 
   fir::FirOpBuilder &builder = converter.getFirOpBuilder();
-  auto &dirSpec = std::get<parser::OmpDirectiveSpecification>(construct.t);
+  const parser::OmpDirectiveSpecification &dirSpec = construct.BeginDir();
   omp::List<omp::Clause> clauses = makeClauses(dirSpec.Clauses(), semaCtx);
   lower::StatementContext stmtCtx;
 
diff --git a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
index 2ac4d9548b65b..7c92867f69a27 100644
--- a/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/DataSharingProcessor.cpp
@@ -395,26 +395,25 @@ getSource(const semantics::SemanticsContext &semaCtx,
   const parser::CharBlock *source = nullptr;
 
   auto ompConsVisit = [&](const parser::OpenMPConstruct &x) {
-    std::visit(
-        common::visitors{
-            [&](const parser::OpenMPSectionsConstruct &x) {
-              source = &std::get<0>(x.t).source;
-            },
-            [&](const parser::OpenMPLoopConstruct &x) {
-              source = &std::get<0>(x.t).source;
-            },
-            [&](const parser::OpenMPBlockConstruct &x) {
-              source = &std::get<0>(x.t).source;
-            },
-            [&](const parser::OpenMPCriticalConstruct &x) {
-              source = &std::get<0>(x.t).source;
-            },
-            [&](const parser::OpenMPAtomicConstruct &x) {
-              source = &std::get<parser::OmpDirectiveSpecification>(x.t).source;
-            },
-            [&](const auto &x) { source = &x.source; },
-        },
-        x.u);
+    std::visit(common::visitors{
+                   [&](const parser::OpenMPSectionsConstruct &x) {
+                     source = &std::get<0>(x.t).source;
+                   },
+                   [&](const parser::OpenMPLoopConstruct &x) {
+                     source = &std::get<0>(x.t).source;
+                   },
+                   [&](const parser::OpenMPBlockConstruct &x) {
+                     source = &std::get<0>(x.t).source;
+                   },
+                   [&](const parser::OpenMPCriticalConstruct &x) {
+                     source = &std::get<0>(x.t).source;
+                   },
+                   [&](const parser::OpenMPAtomicConstruct &x) {
+                     source = &x.BeginDir().source;
+                   },
+                   [&](const auto &x) { source = &x.source; },
+               },
+               x.u);
   };
 
   eval.visit(common::visitors{
diff --git a/flang/lib/Lower/OpenMP/OpenMP.cpp b/flang/lib/Lower/OpenMP/OpenMP.cpp
index 12089d6caa5fe..5942547ae9f96 100644
--- a/flang/lib/Lower/OpenMP/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP/OpenMP.cpp
@@ -407,16 +407,9 @@ static void processHostEvalClauses(lower::AbstractConverter &converter,
     common::visit(
         common::visitors{
             [&](const parser::OpenMPBlockConstruct &ompConstruct) {
-              const auto &beginDirective =
-                  std::get<parser::OmpBeginBlockDirective>(ompConstruct.t);
-              beginClauseList =
-                  &std::get<parser::OmpClauseList>(beginDirective.t);
-              if (auto &endDirective =
-                      std::get<std::optional<parser::OmpEndBlockDirective>>(
-                          ompConstruct.t)) {
-                endClauseList =
-                    &std::get<parser::OmpClauseList>(endDirective->t);
-              }
+              beginClauseList = &ompConstruct.BeginDir().Clauses();
+              if (auto &endSpec = ompConstruct.EndDir())
+                endClauseList = &endSpec->Clauses();
             },
             [&](const parser::OpenMPLoopConstruct &ompConstruct) {
               const auto &beginDirective =
@@ -3716,25 +3709,16 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
                    semantics::SemanticsContext &semaCtx,
                    lower::pft::Evaluation &eval,
                    const parser::OpenMPBlockConstruct &blockConstruct) {
-  const auto &beginBlockDirective =
-      std::get<parser::OmpBeginBlockDirective>(blockConstruct.t);
-  mlir::Location currentLocation =
-      converter.genLocation(beginBlockDirective.source);
-  const auto origDirective =
-      std::get<parser::OmpBlockDirective>(beginBlockDirective.t).v;
-  List<Clause> clauses = makeClauses(
-      std::get<parser::OmpClauseList>(beginBlockDirective.t), semaCtx);
-
-  if (const auto &endBlockDirective =
-          std::get<std::optional<parser::OmpEndBlockDirective>>(
-              blockConstruct.t)) {
-    clauses.append(makeClauses(
-        std::get<parser::OmpClauseList>(endBlockDirective->t), semaCtx));
-  }
-
-  assert(llvm::omp::blockConstructSet.test(origDirective) &&
+  const parser::OmpDirectiveSpecification &beginSpec =
+      blockConstruct.BeginDir();
+  List<Clause> clauses = makeClauses(beginSpec.Clauses(), semaCtx);
+  if (auto &endSpec = blockConstruct.EndDir())
+    clauses.append(makeClauses(endSpec->Clauses(), semaCtx));
+
+  llvm::omp::Directive directive = beginSpec.DirId();
+  assert(llvm::omp::blockConstructSet.test(directive) &&
          "Expected block construct");
-  (void)origDirective;
+  mlir::Location currentLocation = converter.genLocation(beginSpec.source);
 
   for (const Clause &clause : clauses) {
     mlir::Location clauseLocation = converter.genLocation(clause.source);
@@ -3777,13 +3761,9 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
     }
   }
 
-  llvm::omp::Directive directive =
-      std::get<parser::OmpBlockDirective>(beginBlockDirective.t).v;
-  const parser::CharBlock &source =
-      std::get<parser::OmpBlockDirective>(beginBlockDirective.t).source;
   ConstructQueue queue{
       buildConstructQueue(converter.getFirOpBuilder().getModule(), semaCtx,
-                          eval, source, directive, clauses)};
+                          eval, beginSpec.source, directive, clauses)};
   genOMPDispatch(converter, symTable, semaCtx, eval, currentLocation, queue,
                  queue.begin());
 }
@@ -4071,8 +4051,7 @@ bool Fortran::lower::isOpenMPTargetConstruct(
     const parser::OpenMPConstruct &omp) {
   llvm::omp::Directive dir = llvm::omp::Directive::OMPD_unknown;
   if (const auto *block = std::get_if<parser::OpenMPBlockConstruct>(&omp.u)) {
-    const auto &begin = std::get<parser::OmpBeginBlockDirective>(block->t);
-    dir = std::get<parser::OmpBlockDirective>(begin.t).v;
+    dir = block->BeginDir().DirId();
   } else if (const auto *loop =
                  std::get_if<parser::OpenMPLoopConstruct>(&omp.u)) {
     const auto &begin = std::get<parser::OmpBeginLoopDirective>(loop->t);
diff --git a/flang/lib/Parser/openmp-parsers.cpp b/flang/lib/Parser/openmp-parsers.cpp
index 25a692deba3cc..0d9a89f8faa4d 100644
--- a/flang/lib/Parser/openmp-parsers.cpp
+++ b/flang/lib/Parser/openmp-parsers.cpp
@@ -1328,16 +1328,41 @@ TYPE_PARSER(sourced(construct<OmpLoopDirective>(first(
 TYPE_PARSER(sourced(construct<OmpBeginLoopDirective>(
     sourced(Parser<OmpLoopDirective>{}), Parser<OmpClauseList>{})))
 
+static inline constexpr auto IsDirective(llvm::omp::Directive dir) {
+  return [dir](const OmpDirectiveName &name) -> bool { return dir == name.v; };
+}
+
+struct OmpBeginDirectiveParser {
+  using resultType = OmpDirectiveSpecification;
+
+  constexpr OmpBeginDirectiveParser(llvm::omp::Directive dir) : dir_(dir) {}
+
+  std::optional<resultType> Parse(ParseState &state) const {
+    auto &&p{predicated(Parser<OmpDirectiveName>{}, IsDirective(dir_)) >=
+        Parser<OmpDirectiveSpecification>{}};
+    return p.Parse(state);
+  }
+
+private:
+  llvm::omp::Directive dir_;
+};
+
 struct OmpEndDirectiveParser {
   using resultType = OmpDirectiveSpecification;
 
   constexpr OmpEndDirectiveParser(llvm::omp::Directive dir) : dir_(dir) {}
 
   std::optional<resultType> Parse(ParseState &state) const {
-    if ((startOmpLine >> "END"_sptok).Parse(state)) {
-      auto &&dirSpec{Parser<OmpDirectiveSpecification>{}.Parse(state)};
-      if (dirSpec && dirSpec->DirId() == dir_) {
-        return std::move(dirSpec);
+    if (startOmpLine.Parse(state)) {
+      if (auto endToken{verbatim("END"_sptok).Parse(state)}) {
+        if (auto &&dirSpec{OmpBeginDirectiveParser(dir_).Parse(state)}) {
+          // Extend the "source" on both the OmpDirectiveName and the
+          // OmpDirectiveNameSpecification.
+          CharBlock &nameSource{std::get<OmpDirectiveName>(dirSpec->t).source};
+          nameSource.ExtendToCover(endToken->source);
+          dirSpec->source.ExtendToCover(endToken->source);
+          return std::move(*dirSpec);
+        }
       }
     }
     return std::nullopt;
@@ -1347,57 +1372,67 @@ struct OmpEndDirectiveParser {
   llvm::omp::Directive dir_;
 };
 
-struct OmpAllocatorsConstructParser {
-  using resultType = OpenMPAllocatorsConstruct;
+struct OmpStatementConstructParser {
+  using resultType = OmpBlockConstruct;
+
+  constexpr OmpStatementConstructParser(llvm::omp::Directive dir) : dir_(dir) {}
 
   std::optional<resultType> Parse(ParseState &state) const {
-    auto dirSpec{Parser<OmpDirectiveSpecification>{}.Parse(state)};
-    if (!dirSpec || dirSpec->DirId() != llvm::omp::Directive::OMPD_allocators) {
-      return std::nullopt;
-    }
+    if (auto begin{OmpBeginDirectiveParser(dir_).Parse(state)}) {
+      Block body;
+      if (auto stmt{attempt(Parser<ExecutionPartConstruct>{}).Parse(state)}) {
+        body.emplace_back(std::move(*stmt));
+      }
+      // Allow empty block. Check for this in semantics.
 
-    // This should be an allocate-stmt. That will be checked in semantics.
-    Block block;
-    if (auto stmt{attempt(Parser<ExecutionPartConstruct>{}).Parse(state)}) {
-      block.emplace_back(std::move(*stmt));
+      auto end{maybe(OmpEndDirectiveParser{dir_}).Parse(state)};
+      return OmpBlockConstruct{OmpBeginDirective(std::move(*begin)),
+          std::move(body),
+          llvm::transformOptional(std::move(*end),
+              [](auto &&s) { return OmpEndDirective(std::move(s)); })};
     }
-    // Allow empty block. Check for this in semantics.
-
-    auto end{OmpEndDirectiveParser{llvm::omp::Directive::OMPD_allocators}};
-    return OpenMPAllocatorsConstruct{
-        std::move(*dirSpec), std::move(block), *maybe(end).Parse(state)};
+    return std::nullopt;
   }
+
+private:
+  llvm::omp::Directive dir_;
 };
 
-TYPE_PARSER(sourced( //
-    construct<OpenMPAllocatorsConstruct>(
-        "ALLOCATORS"_tok >= OmpAllocatorsConstructParser{})))
+struct OmpBlockConstructParser {
+  using resultType = OmpBlockConstruct;
 
-struct OmpDispatchConstructParser {
-  using resultType = OpenMPDispatchConstruct;
+  constexpr OmpBlockConstructParser(llvm::omp::Directive dir) : dir_(dir) {}
 
   std::optional<resultType> Parse(ParseState &state) const {
-    auto dirSpec{Parser<OmpDirectiveSpecification>{}.Parse(state)};
-    if (!dirSpec || dirSpec->DirId() != llvm::omp::Directive::OMPD_dispatch) {
-      return std::nullopt;
-    }
-
-    // This should be a function call. That will be checked in semantics.
-    Block block;
-    if (auto stmt{attempt(Parser<ExecutionPartConstruct>{}).Parse(state)}) {
-      block.emplace_back(std::move(*stmt));
+    if (auto &&begin{OmpBeginDirectiveParser(dir_).Parse(state)}) {
+      if (auto &&body{attempt(StrictlyStructuredBlockParser{}).Parse(state)}) {
+        // Try strictly-structured block with an optional end-directive
+        auto end{maybe(OmpEndDirectiveParser{dir_}).Parse(state)};
+        return OmpBlockConstruct{OmpBeginDirective(std::move(*begin)),
+            std::move(*body),
+            llvm::transformOptional(std::move(*end),
+                [](auto &&s) { return OmpEndDirective(std::move(s)); })};
+      } else if (auto &&body{
+                     attempt(LooselyStructuredBlockParser{}).Parse(state)}) {
+        // Try loosely-structured block with a mandatory end-directive
+        if (auto end{OmpEndDirectiveParser{dir_}.Parse(state)}) {
+          return OmpBlockConstruct{OmpBeginDirective(std::move(*begin)),
+              std::move(*body), OmpEndDirective{std::move(*end)}};
+        }
+      }
     }
-    // Allow empty block. Check for this in semantics.
-
-    auto end{OmpEndDirectiveParser{llvm::omp::Directive::OMPD_dispatch}};
-    return OpenMPDispatchConstruct{
-        std::move(*dirSpec), std::move(block), *maybe(end).Parse(state)};
+    return std::nullo...
[truncated]

@kparzysz
Copy link
Contributor Author

The last bullet should be

  • optional<OmpEndDirective> (aka optional<OmpDirectiveSpecification>)

but < and > in the commit message were consumed by github.

Copy link

github-actions bot commented Jul 24, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@kparzysz
Copy link
Contributor Author

Replaced with #150956 (part of a PR stack).

@kparzysz kparzysz closed this Jul 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants