[Flang][OpenMP] Handle SECTION construct from within SECTIONS

[kparzysz]

Introduce createSectionOp, invoke it from the SECTIONS construct for each nested SECTION construct. This makes it unnecessary to embed OpenMPSectionConstruct inside of OpenMPSectionConstruct anymore.

Recursive lowering [3/5]

[llvmbot]

@llvm/pr-subscribers-flang-openmp

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

Author: Krzysztof Parzyszek (kparzysz)

Changes

Introduce createSectionOp, invoke it from the SECTIONS construct for each nested SECTION construct. This makes it unnecessary to embed OpenMPSectionConstruct inside of OpenMPSectionConstruct anymore.

Recursive lowering [3/5]

---

Full diff: https://github.com/llvm/llvm-project/pull/77759.diff

1 Files Affected:

• (modified) flang/lib/Lower/OpenMP.cpp (+26-31)

diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index 496b4ba27a0533..aa315d7ab280e2 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -2379,6 +2379,18 @@ genParallelOp(Fortran::lower::AbstractConverter &converter,
       procBindKindAttr);
 }
 
+static mlir::omp::SectionOp
+genSectionOp(Fortran::lower::AbstractConverter &converter,
+             Fortran::lower::pft::Evaluation &eval,
+             mlir::Location currentLocation,
+             const Fortran::parser::OmpClauseList &sectionsClauseList) {
+  // Currently only private/firstprivate clause is handled, and
+  // all privatization is done within `omp.section` operations.
+  return genOpWithBody<mlir::omp::SectionOp>(converter, eval, currentLocation,
+                                             /*outerCombined=*/false,
+                                             &sectionsClauseList);
+}
+
 static mlir::omp::SingleOp
 genSingleOp(Fortran::lower::AbstractConverter &converter,
             Fortran::lower::pft::Evaluation &eval,
@@ -3375,35 +3387,6 @@ genOMP(Fortran::lower::AbstractConverter &converter,
   genNestedEvaluations(converter, eval);
 }
 
-static void
-genOMP(Fortran::lower::AbstractConverter &converter,
-       Fortran::lower::SymMap &symTable, Fortran::lower::pft::Evaluation &eval,
-       const Fortran::parser::OpenMPSectionConstruct &sectionConstruct) {
-  mlir::Location currentLocation = converter.getCurrentLocation();
-  const Fortran::parser::OpenMPConstruct *parentOmpConstruct =
-      eval.parentConstruct->getIf<Fortran::parser::OpenMPConstruct>();
-  assert(parentOmpConstruct &&
-         "No enclosing parent OpenMPConstruct on SECTION construct");
-  const Fortran::parser::OpenMPSectionsConstruct *sectionsConstruct =
-      std::get_if<Fortran::parser::OpenMPSectionsConstruct>(
-          &parentOmpConstruct->u);
-  assert(sectionsConstruct && "SECTION construct must have parent"
-                              "SECTIONS construct");
-  const Fortran::parser::OmpClauseList &sectionsClauseList =
-      std::get<Fortran::parser::OmpClauseList>(
-          std::get<Fortran::parser::OmpBeginSectionsDirective>(
-              sectionsConstruct->t)
-              .t);
-  // Currently only private/firstprivate clause is handled, and
-  // all privatization is done within `omp.section` operations.
-  symTable.pushScope();
-  genOpWithBody<mlir::omp::SectionOp>(converter, eval, currentLocation,
-                                      /*outerCombined=*/false,
-                                      &sectionsClauseList);
-  genNestedEvaluations(converter, eval);
-  symTable.popScope();
-}
-
 static void
 genOMP(Fortran::lower::AbstractConverter &converter,
        Fortran::lower::SymMap &symTable, Fortran::lower::pft::Evaluation &eval,
@@ -3447,7 +3430,18 @@ genOMP(Fortran::lower::AbstractConverter &converter,
                                        /*reductions=*/nullptr, allocateOperands,
                                        allocatorOperands, nowaitClauseOperand);
 
-  genNestedEvaluations(converter, eval);
+  const auto &sectionBlocks =
+      std::get<Fortran::parser::OmpSectionBlocks>(sectionsConstruct.t);
+  auto &firOpBuilder = converter.getFirOpBuilder();
+  auto ip = firOpBuilder.saveInsertionPoint();
+  for (const auto &[nblock, neval] :
+       llvm::zip(sectionBlocks.v, eval.getNestedEvaluations())) {
+    symTable.pushScope();
+    genSectionOp(converter, neval, currentLocation, sectionsClauseList);
+    genNestedEvaluations(converter, neval);
+    symTable.popScope();
+    firOpBuilder.restoreInsertionPoint(ip);
+  }
 }
 
 static void
@@ -3562,7 +3556,8 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
             genOMP(converter, symTable, eval, sectionsConstruct);
           },
           [&](const Fortran::parser::OpenMPSectionConstruct &sectionConstruct) {
-            genOMP(converter, symTable, eval, sectionConstruct);
+            // SECTION constructs are handled as a part of SECTIONS.
+            llvm_unreachable("Unexpected standalone OMP SECTION");
           },
           [&](const Fortran::parser::OpenMPLoopConstruct &loopConstruct) {
             genOMP(converter, symTable, eval, semanticsContext, loopConstruct);

[skatrak]

Thanks, LGTM!

[kiranchandramohan]

This makes it unnecessary to embed OpenMPSectionConstruct inside of OpenMPSectionConstruct anymore.

What do you mean by this? Is there a typo?

Would we be able to remove OpenMPSectionConstruct from struct OpenMPConstruct after this change?

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

Line 3990 in 2f2217a

OpenMPSectionConstruct, OpenMPLoopConstruct, OpenMPBlockConstruct,

[kparzysz]

This makes it unnecessary to embed OpenMPSectionConstruct inside of OpenMPSectionConstruct anymore.

What do you mean by this? Is there a typo?

Yes, fixed. I meant embedding it inside of OpenMPConstruct. Thanks for the catch.

Would we be able to remove OpenMPSectionConstruct from struct OpenMPConstruct after this change?

Yes.

[kiranchandramohan]

Introduce createSectionOp

genSectionOp?

[kiranchandramohan]

LG.

[kparzysz]

Introduce createSectionOp

genSectionOp?

Yes---fixed the commit message.