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

This reverts commit 6533ad0.

Author: ronlieb

flang/examples/FeatureList/FeatureList.cpp

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

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

@@ -534,8 +534,10 @@ 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,6 +586,7 @@ class ParseTreeDumper {
   NODE(parser, OmpDetachClause)
   NODE(parser, OmpDoacrossClause)
   NODE(parser, OmpDestroyClause)
+  NODE(parser, OmpEndBlockDirective)
   NODE(parser, OmpEndCriticalDirective)
   NODE(parser, OmpEndLoopDirective)
   NODE(parser, OmpEndSectionsDirective)
@@ -705,8 +708,6 @@ 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,6 +68,11 @@ 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);
@@ -101,8 +106,10 @@ struct DirectiveNameScope {
         return GetOmpDirectiveName(x.v);
       }
     } else if constexpr (TupleTrait<T>) {
-      if constexpr (std::is_base_of_v<OmpBlockConstruct, T>) {
-        return std::get<OmpBeginDirective>(x.t).DirName();
+      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();
       } 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,12 +3469,6 @@ 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; \
@@ -4756,33 +4750,6 @@ 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;
@@ -4887,6 +4854,12 @@ 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;
@@ -5011,9 +4984,12 @@ struct OpenMPExecutableAllocate {
 //    ALLOCATORS [allocate-clause...]
 //    block
 //    [END ALLOCATORS]
-struct OpenMPAllocatorsConstruct : public OmpBlockConstruct {
-  INHERITED_TUPLE_CLASS_BOILERPLATE(
-      OpenMPAllocatorsConstruct, OmpBlockConstruct);
+struct OpenMPAllocatorsConstruct {
+  TUPLE_CLASS_BOILERPLATE(OpenMPAllocatorsConstruct);
+  CharBlock source;
+  std::tuple<OmpDirectiveSpecification, Block,
+      std::optional<OmpDirectiveSpecification>>
+      t;
 };
 
 // 2.17.7 Atomic construct/2.17.8 Flush construct [OpenMP 5.0]
@@ -5027,11 +5003,15 @@ struct OmpMemoryOrderClause {
   CharBlock source;
 };
 
-struct OpenMPAtomicConstruct : public OmpBlockConstruct {
+struct OpenMPAtomicConstruct {
   llvm::omp::Clause GetKind() const;
   bool IsCapture() const;
   bool IsCompare() const;
-  INHERITED_TUPLE_CLASS_BOILERPLATE(OpenMPAtomicConstruct, OmpBlockConstruct);
+  TUPLE_CLASS_BOILERPLATE(OpenMPAtomicConstruct);
+  CharBlock source;
+  std::tuple<OmpDirectiveSpecification, Block,
+      std::optional<OmpDirectiveSpecification>>
+      t;
 
   // Information filled out during semantic checks to avoid duplication
   // of analyses.
@@ -5095,8 +5075,12 @@ struct OpenMPDepobjConstruct {
 //                    nocontext-clause |
 //                    novariants-clause |
 //                    nowait-clause
-struct OpenMPDispatchConstruct : public OmpBlockConstruct {
-  INHERITED_TUPLE_CLASS_BOILERPLATE(OpenMPDispatchConstruct, OmpBlockConstruct);
+struct OpenMPDispatchConstruct {
+  TUPLE_CLASS_BOILERPLATE(OpenMPDispatchConstruct);
+  CharBlock source;
+  std::tuple<OmpDirectiveSpecification, Block,
+      std::optional<OmpDirectiveSpecification>>
+      t;
 };
 
 // [4.5:162-165], [5.0:242-246], [5.1:275-279], [5.2:315-316], [6.0:498-500]
@@ -5151,8 +5135,22 @@ struct OmpEndLoopDirective {
   CharBlock source;
 };
 
-struct OpenMPBlockConstruct : public OmpBlockConstruct {
-  INHERITED_TUPLE_CLASS_BOILERPLATE(OpenMPBlockConstruct, OmpBlockConstruct);
+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;
 };
 
 // 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();
-  const parser::OmpDirectiveSpecification &dirSpec = construct.BeginDir();
+  auto &dirSpec = std::get<parser::OmpDirectiveSpecification>(construct.t);
   omp::List<omp::Clause> clauses = makeClauses(dirSpec.Clauses(), semaCtx);
   lower::StatementContext stmtCtx;
 

flang/lib/Lower/OpenMP/OpenMP.cpp

@@ -407,9 +407,16 @@ static void processHostEvalClauses(lower::AbstractConverter &converter,
     common::visit(
         common::visitors{
             [&](const parser::OpenMPBlockConstruct &ompConstruct) {
-              beginClauseList = &ompConstruct.BeginDir().Clauses();
-              if (auto &endSpec = ompConstruct.EndDir())
-                endClauseList = &endSpec->Clauses();
+              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);
+              }
             },
             [&](const parser::OpenMPLoopConstruct &ompConstruct) {
               const auto &beginDirective =
@@ -3660,16 +3667,25 @@ static void genOMP(lower::AbstractConverter &converter, lower::SymMap &symTable,
                    semantics::SemanticsContext &semaCtx,
                    lower::pft::Evaluation &eval,
                    const parser::OpenMPBlockConstruct &blockConstruct) {
-  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) &&
+  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) &&
          "Expected block construct");
-  mlir::Location currentLocation = converter.genLocation(beginSpec.source);
+  (void)origDirective;
 
   for (const Clause &clause : clauses) {
     mlir::Location clauseLocation = converter.genLocation(clause.source);
@@ -3712,9 +3728,13 @@ 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, beginSpec.source, directive, clauses)};
+                          eval, source, directive, clauses)};
   genOMPDispatch(converter, symTable, semaCtx, eval, currentLocation, queue,
                  queue.begin());
 }
@@ -4002,7 +4022,8 @@ 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)) {
-    dir = block->BeginDir().DirId();
+    const auto &begin = std::get<parser::OmpBeginBlockDirective>(block->t);
+    dir = std::get<parser::OmpBlockDirective>(begin.t).v;
   } else if (const auto *loop =
                  std::get_if<parser::OpenMPLoopConstruct>(&omp.u)) {
     const auto &begin = std::get<parser::OmpBeginLoopDirective>(loop->t);