[flang][OpenMP] Store directive information in OpenMPSectionConstruct (#150804)

The OpenMPSectionConstruct corresponds to the `!$omp section` directive,
but there is nothing in the AST node that stores the directive
information. Even though the only possibility (at the moment) is
"section" without any clauses, for improved generality it is helpful to
have that information anyway.

Author: kparzysz

flang/examples/FeatureList/FeatureList.cpp

@@ -529,7 +529,6 @@ struct NodeVisitor {
   READ_FEATURE(OmpChunkModifier::Value)
   READ_FEATURE(OmpOrderingModifier)
   READ_FEATURE(OmpOrderingModifier::Value)
-  READ_FEATURE(OmpSectionBlocks)
   READ_FEATURE(OmpSectionsDirective)
   READ_FEATURE(Only)
   READ_FEATURE(OpenACCAtomicConstruct)

flang/examples/FlangOmpReport/FlangOmpReportVisitor.cpp

@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "FlangOmpReportVisitor.h"
+#include "flang/Parser/openmp-utils.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Frontend/OpenMP/OMP.h"
 
@@ -118,60 +119,8 @@ std::string OpenMPCounterVisitor::getName(const OpenMPDeclarativeConstruct &c) {
       c.u);
 }
 std::string OpenMPCounterVisitor::getName(const OpenMPConstruct &c) {
-  return std::visit(
-      Fortran::common::visitors{
-          [&](const OpenMPStandaloneConstruct &c) -> std::string {
-            return common::visit(
-                common::visitors{
-                    [&](const OmpMetadirectiveDirective &d) {
-                      return normalize_construct_name(d.source.ToString());
-                    },
-                    [&](auto &&d) {
-                      const CharBlock &source{
-                          std::get<OmpDirectiveName>(d.v.t).source};
-                      return normalize_construct_name(source.ToString());
-                    },
-                },
-                c.u);
-          },
-          [&](const OpenMPExecutableAllocate &c) -> std::string {
-            const CharBlock &source{std::get<0>(c.t).source};
-            return normalize_construct_name(source.ToString());
-          },
-          [&](const OpenMPDeclarativeAllocate &c) -> std::string {
-            const CharBlock &source{std::get<0>(c.t).source};
-            return normalize_construct_name(source.ToString());
-          },
-          [&](const OpenMPAssumeConstruct &c) -> std::string {
-            const CharBlock &source{std::get<0>(c.t).source};
-            return normalize_construct_name(source.ToString());
-          },
-          [&](const OpenMPAllocatorsConstruct &c) -> std::string {
-            const CharBlock &source{std::get<0>(c.t).source};
-            return normalize_construct_name(source.ToString());
-          },
-          [&](const OpenMPAtomicConstruct &c) -> std::string {
-            auto &dirSpec = std::get<OmpDirectiveSpecification>(c.t);
-            auto &dirName = std::get<OmpDirectiveName>(dirSpec.t);
-            return normalize_construct_name(dirName.source.ToString());
-          },
-          [&](const OpenMPUtilityConstruct &c) -> std::string {
-            const CharBlock &source{c.source};
-            return normalize_construct_name(source.ToString());
-          },
-          [&](const OpenMPSectionConstruct &c) -> std::string {
-            return "section";
-          },
-          // OpenMPSectionsConstruct, OpenMPLoopConstruct,
-          // OpenMPBlockConstruct, OpenMPCriticalConstruct Get the source from
-          // the directive field of the begin directive or from the verbatim
-          // field of the begin directive in Critical
-          [&](const auto &c) -> std::string {
-            const CharBlock &source{std::get<0>(std::get<0>(c.t).t).source};
-            return normalize_construct_name(source.ToString());
-          },
-      },
-      c.u);
+  return normalize_construct_name(
+      omp::GetOmpDirectiveName(c).source.ToString());
 }
 
 bool OpenMPCounterVisitor::Pre(const OpenMPDeclarativeConstruct &c) {

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

@@ -681,7 +681,6 @@ class ParseTreeDumper {
   NODE_ENUM(OmpChunkModifier, Value)
   NODE(parser, OmpOrderingModifier)
   NODE_ENUM(OmpOrderingModifier, Value)
-  NODE(parser, OmpSectionBlocks)
   NODE(parser, OmpSectionsDirective)
   NODE(parser, OmpToClause)
   NODE(OmpToClause, Modifier)

flang/include/flang/Parser/openmp-utils.h

@@ -78,6 +78,14 @@ struct DirectiveNameScope {
     return MakeName(dir.source, dir.v);
   }
 
+  static OmpDirectiveName GetOmpDirectiveName(const OpenMPSectionConstruct &x) {
+    if (auto &spec{std::get<std::optional<OmpDirectiveSpecification>>(x.t)}) {
+      return spec->DirName();
+    } else {
+      return MakeName({}, llvm::omp::Directive::OMPD_section);
+    }
+  }
+
   static OmpDirectiveName GetOmpDirectiveName(
       const OmpBeginSectionsDirective &x) {
     auto &dir{std::get<OmpSectionsDirective>(x.t)};

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

@@ -4818,18 +4818,17 @@ struct OmpEndSectionsDirective {
 //    structured-block]
 // ...
 struct OpenMPSectionConstruct {
-  WRAPPER_CLASS_BOILERPLATE(OpenMPSectionConstruct, Block);
+  TUPLE_CLASS_BOILERPLATE(OpenMPSectionConstruct);
+  std::tuple<std::optional<OmpDirectiveSpecification>, Block> t;
   CharBlock source;
 };
 
-// `OmpSectionBlocks` is a list of section constructs. The parser guarentees
-// that the `OpenMPConstruct` here always encapsulates an
-// `OpenMPSectionConstruct` and not any other OpenMP construct.
-WRAPPER_CLASS(OmpSectionBlocks, std::list<OpenMPConstruct>);
-
 struct OpenMPSectionsConstruct {
   TUPLE_CLASS_BOILERPLATE(OpenMPSectionsConstruct);
-  std::tuple<OmpBeginSectionsDirective, OmpSectionBlocks,
+  CharBlock source;
+  // Each of the OpenMPConstructs in the list below contains an
+  // OpenMPSectionConstruct. This is guaranteed by the parser.
+  std::tuple<OmpBeginSectionsDirective, std::list<OpenMPConstruct>,
       OmpEndSectionsDirective>
       t;
 };

flang/lib/Lower/OpenMP/OpenMP.cpp

@@ -2332,7 +2332,7 @@ genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
   assert(sectionsConstruct && "Missing additional parsing information");
 
   const auto &sectionBlocks =
-      std::get<parser::OmpSectionBlocks>(sectionsConstruct->t);
+      std::get<std::list<parser::OpenMPConstruct>>(sectionsConstruct->t);
   mlir::omp::SectionsOperands clauseOps;
   llvm::SmallVector<const semantics::Symbol *> reductionSyms;
   genSectionsClauses(converter, semaCtx, item->clauses, loc, clauseOps,
@@ -2385,7 +2385,7 @@ genSectionsOp(lower::AbstractConverter &converter, lower::SymMap &symTable,
   // because we need to run genReductionVars on each omp.section so that the
   // reduction variable gets mapped to the private version
   for (auto [construct, nestedEval] :
-       llvm::zip(sectionBlocks.v, eval.getNestedEvaluations())) {
+       llvm::zip(sectionBlocks, eval.getNestedEvaluations())) {
     const auto *sectionConstruct =
         std::get_if<parser::OpenMPSectionConstruct>(&construct.u);
     if (!sectionConstruct) {

flang/lib/Parser/openmp-parsers.cpp

@@ -34,6 +34,39 @@ namespace Fortran::parser {
 constexpr auto startOmpLine = skipStuffBeforeStatement >> "!$OMP "_sptok;
 constexpr auto endOmpLine = space >> endOfLine;
 
+// Given a parser for a single element, and a parser for a list of elements
+// of the same type, create a parser that constructs the entire list by having
+// the single element be the head of the list, and the rest be the tail.
+template <typename ParserH, typename ParserT> struct ConsParser {
+  static_assert(std::is_same_v<std::list<typename ParserH::resultType>,
+      typename ParserT::resultType>);
+
+  using resultType = typename ParserT::resultType;
+  constexpr ConsParser(ParserH h, ParserT t) : head_(h), tail_(t) {}
+
+  std::optional<resultType> Parse(ParseState &state) const {
+    if (auto &&first{head_.Parse(state)}) {
+      if (auto rest{tail_.Parse(state)}) {
+        rest->push_front(std::move(*first));
+        return std::move(*rest);
+      }
+    }
+    return std::nullopt;
+  }
+
+private:
+  const ParserH head_;
+  const ParserT tail_;
+};
+
+template <typename ParserH, typename ParserT,
+    typename ValueH = typename ParserH::resultType,
+    typename ValueT = typename ParserT::resultType,
+    typename = std::enable_if_t<std::is_same_v<std::list<ValueH>, ValueT>>>
+constexpr auto cons(ParserH head, ParserT tail) {
+  return ConsParser<ParserH, ParserT>(head, tail);
+}
+
 // Given a parser P for a wrapper class, invoke P, and if it succeeds return
 // the wrapped object.
 template <typename Parser> struct UnwrapParser {
@@ -1831,19 +1864,20 @@ TYPE_PARSER(
                         sourced("END"_tok >> Parser<OmpSectionsDirective>{}),
                         Parser<OmpClauseList>{})))
 
-// OMP SECTION-BLOCK
-
-TYPE_PARSER(construct<OpenMPSectionConstruct>(block))
-
-TYPE_PARSER(maybe(startOmpLine >> "SECTION"_tok / endOmpLine) >>
-    construct<OmpSectionBlocks>(nonemptySeparated(
-        construct<OpenMPConstruct>(sourced(Parser<OpenMPSectionConstruct>{})),
-        startOmpLine >> "SECTION"_tok / endOmpLine)))
+static constexpr auto sectionDir{
+    startOmpLine >> (predicated(OmpDirectiveNameParser{},
+                         IsDirective(llvm::omp::Directive::OMPD_section)) >=
+                        Parser<OmpDirectiveSpecification>{})};
 
 // OMP SECTIONS (OpenMP 5.0 - 2.8.1), PARALLEL SECTIONS (OpenMP 5.0 - 2.13.3)
-TYPE_PARSER(construct<OpenMPSectionsConstruct>(
+TYPE_PARSER(sourced(construct<OpenMPSectionsConstruct>(
     Parser<OmpBeginSectionsDirective>{} / endOmpLine,
-    Parser<OmpSectionBlocks>{}, Parser<OmpEndSectionsDirective>{} / endOmpLine))
+    cons( //
+        construct<OpenMPConstruct>(sourced(
+            construct<OpenMPSectionConstruct>(maybe(sectionDir), block))),
+        many(construct<OpenMPConstruct>(
+            sourced(construct<OpenMPSectionConstruct>(sectionDir, block))))),
+    Parser<OmpEndSectionsDirective>{} / endOmpLine)))
 
 static bool IsExecutionPart(const OmpDirectiveName &name) {
   return name.IsExecutionPart();

flang/lib/Parser/unparse.cpp

@@ -2801,24 +2801,24 @@ class UnparseVisitor {
       break;
     }
   }
-  void Unparse(const OmpSectionBlocks &x) {
-    for (const auto &y : x.v) {
+  void Unparse(const OpenMPSectionConstruct &x) {
+    if (auto &&dirSpec{
+            std::get<std::optional<OmpDirectiveSpecification>>(x.t)}) {
       BeginOpenMP();
-      Word("!$OMP SECTION");
+      Word("!$OMP ");
+      Walk(*dirSpec);
       Put("\n");
       EndOpenMP();
-      // y.u is an OpenMPSectionConstruct
-      // (y.u).v is Block
-      Walk(std::get<OpenMPSectionConstruct>(y.u).v, "");
     }
+    Walk(std::get<Block>(x.t), "");
   }
   void Unparse(const OpenMPSectionsConstruct &x) {
     BeginOpenMP();
     Word("!$OMP ");
     Walk(std::get<OmpBeginSectionsDirective>(x.t));
     Put("\n");
     EndOpenMP();
-    Walk(std::get<OmpSectionBlocks>(x.t));
+    Walk(std::get<std::list<OpenMPConstruct>>(x.t), "");
     BeginOpenMP();
     Word("!$OMP END ");
     Walk(std::get<OmpEndSectionsDirective>(x.t));

flang/lib/Semantics/check-omp-structure.cpp

@@ -1057,10 +1057,11 @@ void OmpStructureChecker::Enter(const parser::OpenMPSectionsConstruct &x) {
   PushContextAndClauseSets(beginDir.source, beginDir.v);
   AddEndDirectiveClauses(std::get<parser::OmpClauseList>(endSectionsDir.t));
 
-  const auto &sectionBlocks{std::get<parser::OmpSectionBlocks>(x.t)};
-  for (const parser::OpenMPConstruct &block : sectionBlocks.v) {
-    CheckNoBranching(std::get<parser::OpenMPSectionConstruct>(block.u).v,
-        beginDir.v, beginDir.source);
+  const auto &sectionBlocks{std::get<std::list<parser::OpenMPConstruct>>(x.t)};
+  for (const parser::OpenMPConstruct &construct : sectionBlocks) {
+    auto &section{std::get<parser::OpenMPSectionConstruct>(construct.u)};
+    CheckNoBranching(
+        std::get<parser::Block>(section.t), beginDir.v, beginDir.source);
   }
   HasInvalidWorksharingNesting(
       beginDir.source, llvm::omp::nestedWorkshareErrSet);

flang/test/Examples/omp-sections.f90

@@ -13,11 +13,11 @@ subroutine omp_sections()
 end subroutine omp_sections
 
 !CHECK: - file:            {{.*}}
-!CHECK:   line:            9
+!CHECK:   line:            8
 !CHECK:   construct:       section
 !CHECK:   clauses:         []
 !CHECK: - file:            {{.*}}
-!CHECK:   line:            11
+!CHECK:   line:            10
 !CHECK:   construct:       section
 !CHECK:   clauses:         []
 !CHECK: - file:            {{.*}}