[flang][OpenMP] Move nested eval conversion to OpenMP.cpp, NFC

[kparzysz]

This is the first step towards exploiting genEval functionality from inside of OpenMP-generating functions.

This follows discourse discussion:
https://discourse.llvm.org/t/openmp-lowering-from-pft-to-fir/75263

[llvmbot]

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

@llvm/pr-subscribers-flang-openmp

Author: Krzysztof Parzyszek (kparzysz)

Changes

This is the first step towards exploiting genEval functionality from inside of OpenMP-generating functions.

---

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

3 Files Affected:

• (modified) flang/include/flang/Lower/OpenMP.h (+4-2)
• (modified) flang/lib/Lower/Bridge.cpp (+2-44)
• (modified) flang/lib/Lower/OpenMP.cpp (+84-25)

diff --git a/flang/include/flang/Lower/OpenMP.h b/flang/include/flang/Lower/OpenMP.h
index c9162761a08d54..b65fc4134c6e13 100644
--- a/flang/include/flang/Lower/OpenMP.h
+++ b/flang/include/flang/Lower/OpenMP.h
@@ -42,6 +42,7 @@ class SemanticsContext;
 namespace lower {
 
 class AbstractConverter;
+class SymMap;
 
 namespace pft {
 struct Evaluation;
@@ -52,8 +53,9 @@ struct Variable;
 void genOpenMPTerminator(fir::FirOpBuilder &, mlir::Operation *,
                          mlir::Location);
 
-void genOpenMPConstruct(AbstractConverter &, semantics::SemanticsContext &,
-                        pft::Evaluation &, const parser::OpenMPConstruct &);
+void genOpenMPConstruct(AbstractConverter &, Fortran::lower::SymMap &,
+                        semantics::SemanticsContext &, pft::Evaluation &,
+                        const parser::OpenMPConstruct &);
 void genOpenMPDeclarativeConstruct(AbstractConverter &, pft::Evaluation &,
                                    const parser::OpenMPDeclarativeConstruct &);
 int64_t getCollapseValue(const Fortran::parser::OmpClauseList &clauseList);
diff --git a/flang/lib/Lower/Bridge.cpp b/flang/lib/Lower/Bridge.cpp
index 6ca910d2696742..f3cb5158a9de3f 100644
--- a/flang/lib/Lower/Bridge.cpp
+++ b/flang/lib/Lower/Bridge.cpp
@@ -2421,48 +2421,8 @@ class FirConverter : public Fortran::lower::AbstractConverter {
 
   void genFIR(const Fortran::parser::OpenMPConstruct &omp) {
     mlir::OpBuilder::InsertPoint insertPt = builder->saveInsertionPoint();
-    localSymbols.pushScope();
-    genOpenMPConstruct(*this, bridge.getSemanticsContext(), getEval(), omp);
-
-    const Fortran::parser::OpenMPLoopConstruct *ompLoop =
-        std::get_if<Fortran::parser::OpenMPLoopConstruct>(&omp.u);
-    const Fortran::parser::OpenMPBlockConstruct *ompBlock =
-        std::get_if<Fortran::parser::OpenMPBlockConstruct>(&omp.u);
-
-    // If loop is part of an OpenMP Construct then the OpenMP dialect
-    // workshare loop operation has already been created. Only the
-    // body needs to be created here and the do_loop can be skipped.
-    // Skip the number of collapsed loops, which is 1 when there is a
-    // no collapse requested.
-
-    Fortran::lower::pft::Evaluation *curEval = &getEval();
-    const Fortran::parser::OmpClauseList *loopOpClauseList = nullptr;
-    if (ompLoop) {
-      loopOpClauseList = &std::get<Fortran::parser::OmpClauseList>(
-          std::get<Fortran::parser::OmpBeginLoopDirective>(ompLoop->t).t);
-      int64_t collapseValue =
-          Fortran::lower::getCollapseValue(*loopOpClauseList);
-
-      curEval = &curEval->getFirstNestedEvaluation();
-      for (int64_t i = 1; i < collapseValue; i++) {
-        curEval = &*std::next(curEval->getNestedEvaluations().begin());
-      }
-    }
-
-    for (Fortran::lower::pft::Evaluation &e : curEval->getNestedEvaluations())
-      genFIR(e);
-
-    if (ompLoop) {
-      genOpenMPReduction(*this, *loopOpClauseList);
-    } else if (ompBlock) {
-      const auto &blockStart =
-          std::get<Fortran::parser::OmpBeginBlockDirective>(ompBlock->t);
-      const auto &blockClauses =
-          std::get<Fortran::parser::OmpClauseList>(blockStart.t);
-      genOpenMPReduction(*this, blockClauses);
-    }
-
-    localSymbols.popScope();
+    genOpenMPConstruct(*this, localSymbols, bridge.getSemanticsContext(),
+                       getEval(), omp);
     builder->restoreInsertionPoint(insertPt);
 
     // Register if a target region was found
@@ -2478,8 +2438,6 @@ class FirConverter : public Fortran::lower::AbstractConverter {
         ompDeviceCodeFound ||
         Fortran::lower::isOpenMPDeviceDeclareTarget(*this, getEval(), ompDecl);
     genOpenMPDeclarativeConstruct(*this, getEval(), ompDecl);
-    for (Fortran::lower::pft::Evaluation &e : getEval().getNestedEvaluations())
-      genFIR(e);
     builder->restoreInsertionPoint(insertPt);
   }
 
diff --git a/flang/lib/Lower/OpenMP.cpp b/flang/lib/Lower/OpenMP.cpp
index 12b8ea82884d9d..2c51faf3302d0d 100644
--- a/flang/lib/Lower/OpenMP.cpp
+++ b/flang/lib/Lower/OpenMP.cpp
@@ -17,6 +17,7 @@
 #include "flang/Lower/ConvertExpr.h"
 #include "flang/Lower/ConvertVariable.h"
 #include "flang/Lower/PFTBuilder.h"
+#include "flang/Lower/SymbolMap.h"
 #include "flang/Lower/StatementContext.h"
 #include "flang/Optimizer/Builder/BoxValue.h"
 #include "flang/Optimizer/Builder/FIRBuilder.h"
@@ -3384,27 +3385,12 @@ static void genOMP(Fortran::lower::AbstractConverter &converter,
   }
 }
 
-//===----------------------------------------------------------------------===//
-// Public functions
-//===----------------------------------------------------------------------===//
-
-void Fortran::lower::genOpenMPTerminator(fir::FirOpBuilder &builder,
-                                         mlir::Operation *op,
-                                         mlir::Location loc) {
-  if (mlir::isa<mlir::omp::WsLoopOp, mlir::omp::ReductionDeclareOp,
-                mlir::omp::AtomicUpdateOp, mlir::omp::SimdLoopOp>(op))
-    builder.create<mlir::omp::YieldOp>(loc);
-  else
-    builder.create<mlir::omp::TerminatorOp>(loc);
-}
-
-void Fortran::lower::genOpenMPConstruct(
-    Fortran::lower::AbstractConverter &converter,
-    Fortran::semantics::SemanticsContext &semanticsContext,
-    Fortran::lower::pft::Evaluation &eval,
-    const Fortran::parser::OpenMPConstruct &ompConstruct) {
+static void genOMP(Fortran::lower::AbstractConverter &converter,
+                   Fortran::semantics::SemanticsContext &semanticsContext,
+                   Fortran::lower::pft::Evaluation &eval,
+                   const Fortran::parser::OpenMPConstruct &ompConstruct) {
   std::visit(
-      common::visitors{
+      Fortran::common::visitors{
           [&](const Fortran::parser::OpenMPStandaloneConstruct
                   &standaloneConstruct) {
             genOMP(converter, eval, semanticsContext, standaloneConstruct);
@@ -3445,12 +3431,12 @@ void Fortran::lower::genOpenMPConstruct(
       ompConstruct.u);
 }
 
-void Fortran::lower::genOpenMPDeclarativeConstruct(
-    Fortran::lower::AbstractConverter &converter,
-    Fortran::lower::pft::Evaluation &eval,
-    const Fortran::parser::OpenMPDeclarativeConstruct &ompDeclConstruct) {
+static void
+genOMP(Fortran::lower::AbstractConverter &converter,
+       Fortran::lower::pft::Evaluation &eval,
+       const Fortran::parser::OpenMPDeclarativeConstruct &ompDeclConstruct) {
   std::visit(
-      common::visitors{
+      Fortran::common::visitors{
           [&](const Fortran::parser::OpenMPDeclarativeAllocate
                   &declarativeAllocate) {
             TODO(converter.getCurrentLocation(), "OpenMPDeclarativeAllocate");
@@ -3483,6 +3469,79 @@ void Fortran::lower::genOpenMPDeclarativeConstruct(
       ompDeclConstruct.u);
 }
 
+//===----------------------------------------------------------------------===//
+// Public functions
+//===----------------------------------------------------------------------===//
+
+void Fortran::lower::genOpenMPTerminator(fir::FirOpBuilder &builder,
+                                         mlir::Operation *op,
+                                         mlir::Location loc) {
+  if (mlir::isa<mlir::omp::WsLoopOp, mlir::omp::ReductionDeclareOp,
+                mlir::omp::AtomicUpdateOp, mlir::omp::SimdLoopOp>(op))
+    builder.create<mlir::omp::YieldOp>(loc);
+  else
+    builder.create<mlir::omp::TerminatorOp>(loc);
+}
+
+void Fortran::lower::genOpenMPConstruct(
+    Fortran::lower::AbstractConverter &converter,
+    Fortran::lower::SymMap &symTable,
+    Fortran::semantics::SemanticsContext &semanticsContext,
+    Fortran::lower::pft::Evaluation &eval,
+    const Fortran::parser::OpenMPConstruct &omp) {
+
+  symTable.pushScope();
+  genOMP(converter, semanticsContext, eval, omp);
+
+  const Fortran::parser::OpenMPLoopConstruct *ompLoop =
+      std::get_if<Fortran::parser::OpenMPLoopConstruct>(&omp.u);
+  const Fortran::parser::OpenMPBlockConstruct *ompBlock =
+      std::get_if<Fortran::parser::OpenMPBlockConstruct>(&omp.u);
+
+  // If loop is part of an OpenMP Construct then the OpenMP dialect
+  // workshare loop operation has already been created. Only the
+  // body needs to be created here and the do_loop can be skipped.
+  // Skip the number of collapsed loops, which is 1 when there is a
+  // no collapse requested.
+
+  Fortran::lower::pft::Evaluation *curEval = &eval;
+  const Fortran::parser::OmpClauseList *loopOpClauseList = nullptr;
+  if (ompLoop) {
+    loopOpClauseList = &std::get<Fortran::parser::OmpClauseList>(
+        std::get<Fortran::parser::OmpBeginLoopDirective>(ompLoop->t).t);
+    int64_t collapseValue = Fortran::lower::getCollapseValue(*loopOpClauseList);
+
+    curEval = &curEval->getFirstNestedEvaluation();
+    for (int64_t i = 1; i < collapseValue; i++) {
+      curEval = &*std::next(curEval->getNestedEvaluations().begin());
+    }
+  }
+
+  for (Fortran::lower::pft::Evaluation &e : curEval->getNestedEvaluations())
+    converter.genEval(e);
+
+  if (ompLoop) {
+    genOpenMPReduction(converter, *loopOpClauseList);
+  } else if (ompBlock) {
+    const auto &blockStart =
+        std::get<Fortran::parser::OmpBeginBlockDirective>(ompBlock->t);
+    const auto &blockClauses =
+        std::get<Fortran::parser::OmpClauseList>(blockStart.t);
+    genOpenMPReduction(converter, blockClauses);
+  }
+
+  symTable.popScope();
+}
+
+void Fortran::lower::genOpenMPDeclarativeConstruct(
+    Fortran::lower::AbstractConverter &converter,
+    Fortran::lower::pft::Evaluation &eval,
+    const Fortran::parser::OpenMPDeclarativeConstruct &omp) {
+  genOMP(converter, eval, omp);
+  for (Fortran::lower::pft::Evaluation &e : eval.getNestedEvaluations())
+    converter.genEval(e);
+}
+
 int64_t Fortran::lower::getCollapseValue(
     const Fortran::parser::OmpClauseList &clauseList) {
   for (const Fortran::parser::OmpClause &clause : clauseList.v) {

[github-actions]

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

[kparzysz]

The change here is from

// --- Bridge.cpp
genFIR(OpenMPConstruct &omp) {
  pushScope();
  genOpenMPConstruct(omp);

  genFIR(nested_evals);
  popScope();
}

// --- OpenMP.cpp
genOpenMPConstruct(OpenMPConstruct &omp) {
  visit(omp);
}

to

// --- Bridge.cpp
genFIR(OpenMPConstruct &omp) {
  genOpenMPConstruct(omp);
}

// --- OpenMP.cpp
genOMP(OpenMPConstruct &omp) {
  visit(omp);
}

genOpenMPConstruct(OpenMPConstruct &omp) {
  pushScope();              // Former genFIR code
  genOMP(omp);              // .

  genEval(nested_evals);    // .
  popScope();               // .
}

[kiranchandramohan]

LG.