[flang][OpenMP] Make all block constructs share the same structure (#150956)

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.

Author: kparzysz

flang/examples/FeatureList/FeatureList.cpp

@@ -445,10 +445,9 @@ struct NodeVisitor {
   READ_FEATURE(ObjectDecl)
   READ_FEATURE(OldParameterStmt)
   READ_FEATURE(OmpAlignedClause)
-  READ_FEATURE(OmpBeginBlockDirective)
+  READ_FEATURE(OmpBeginDirective)
   READ_FEATURE(OmpBeginLoopDirective)
   READ_FEATURE(OmpBeginSectionsDirective)
-  READ_FEATURE(OmpBlockDirective)
   READ_FEATURE(OmpClause)
   READ_FEATURE(OmpClauseList)
   READ_FEATURE(OmpCriticalDirective)
@@ -472,7 +471,7 @@ struct NodeVisitor {
   READ_FEATURE(OmpIteration)
   READ_FEATURE(OmpIterationOffset)
   READ_FEATURE(OmpIterationVector)
-  READ_FEATURE(OmpEndBlockDirective)
+  READ_FEATURE(OmpEndDirective)
   READ_FEATURE(OmpEndCriticalDirective)
   READ_FEATURE(OmpEndLoopDirective)
   READ_FEATURE(OmpEndSectionsDirective)

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))
@@ -586,7 +584,6 @@ class ParseTreeDumper {
   NODE(parser, OmpDetachClause)
   NODE(parser, OmpDoacrossClause)
   NODE(parser, OmpDestroyClause)
-  NODE(parser, OmpEndBlockDirective)
   NODE(parser, OmpEndCriticalDirective)
   NODE(parser, OmpEndLoopDirective)
   NODE(parser, OmpEndSectionsDirective)
@@ -708,6 +705,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)

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);
@@ -106,10 +101,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> ||

flang/include/flang/Parser/parse-tree.h

@@ -3469,6 +3469,12 @@ WRAPPER_CLASS(PauseStmt, std::optional<StopCode>);
 
 // --- Common definitions
 
+#define INHERITED_TUPLE_CLASS_BOILERPLATE(classname, basename) \
+  using basename::basename; \
+  classname(basename &&b) : basename(std::move(b)) {} \
+  using TupleTrait = std::true_type; \
+  BOILERPLATE(classname)
+
 #define INHERITED_WRAPPER_CLASS_BOILERPLATE(classname, basename) \
   BOILERPLATE(classname); \
   using basename::basename; \
@@ -4750,6 +4756,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;
@@ -4854,12 +4887,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;
@@ -4984,12 +5011,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]
@@ -5003,15 +5027,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.
@@ -5075,12 +5095,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]
@@ -5135,22 +5151,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

flang/lib/Lower/OpenMP/Atomic.cpp

@@ -707,7 +707,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;
 

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 =
@@ -3733,25 +3726,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);
@@ -3794,13 +3778,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());
 }
@@ -4088,8 +4068,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);