[flang-commits] [flang] [Flang][bbc] Prevent bbc -emit-fir command invoking all passes twice (PR #80927)

[via flang-commits]

https://github.com/agozillon updated https://github.com/llvm/llvm-project/pull/80927

>From 3fd8e67690e3a10d6000b2a0480a0d62dcf275e9 Mon Sep 17 00:00:00 2001
From: Andrew Gozillon <Andrew.Gozillon at amd.com>
Date: Tue, 6 Feb 2024 20:42:32 -0600
Subject: [PATCH 1/3] [Flang][bbc] Prevent bbc -emit-fir command invoking all
 passes twice

Currently when the bbc tool is invoked with the emit-fir command the pass pipeline will be invoked twice causing passes added to the pass manager to
run twice on the input IR.

This change seeks to prevent that from occuring by only invoking the pass
managers run command once and situationally adding the HLFIR to FIR
pass pipeline in the same scenario as before.
---
 flang/tools/bbc/bbc.cpp | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/flang/tools/bbc/bbc.cpp b/flang/tools/bbc/bbc.cpp
index 98d9258e023e55..68445be28c656f 100644
--- a/flang/tools/bbc/bbc.cpp
+++ b/flang/tools/bbc/bbc.cpp
@@ -389,18 +389,14 @@ static mlir::LogicalResult convertFortranSourceToMLIR(
     // --emit-fir: Build the IR, verify it, and dump the IR if the IR passes
     // verification. Use --dump-module-on-failure to dump invalid IR.
     pm.addPass(std::make_unique<Fortran::lower::VerifierPass>());
-    if (mlir::failed(pm.run(mlirModule))) {
-      llvm::errs() << "FATAL: verification of lowering to FIR failed";
-      return mlir::failure();
-    }
 
-    if (emitFIR && useHLFIR) {
-      // lower HLFIR to FIR
+    // Add HLFIR to FIR lowering pass
+    if (emitFIR && useHLFIR)
       fir::createHLFIRToFIRPassPipeline(pm, llvm::OptimizationLevel::O2);
-      if (mlir::failed(pm.run(mlirModule))) {
-        llvm::errs() << "FATAL: lowering from HLFIR to FIR failed";
-        return mlir::failure();
-      }
+
+    if (mlir::failed(pm.run(mlirModule))) {
+      llvm::errs() << "FATAL: verification of FIR or HLFIR module failed";
+      return mlir::failure();
     }
 
     printModule(mlirModule, out);

>From a5194c82a48fbd03eb0d1122244f6aec636ed8e1 Mon Sep 17 00:00:00 2001
From: Andrew Gozillon <Andrew.Gozillon at amd.com>
Date: Wed, 7 Feb 2024 09:32:31 -0600
Subject: [PATCH 2/3] Revert "[Flang][bbc] Prevent bbc -emit-fir command
 invoking all passes twice"

This reverts commit 3fd8e67690e3a10d6000b2a0480a0d62dcf275e9.
---
 flang/tools/bbc/bbc.cpp | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/flang/tools/bbc/bbc.cpp b/flang/tools/bbc/bbc.cpp
index 68445be28c656f..98d9258e023e55 100644
--- a/flang/tools/bbc/bbc.cpp
+++ b/flang/tools/bbc/bbc.cpp
@@ -389,16 +389,20 @@ static mlir::LogicalResult convertFortranSourceToMLIR(
     // --emit-fir: Build the IR, verify it, and dump the IR if the IR passes
     // verification. Use --dump-module-on-failure to dump invalid IR.
     pm.addPass(std::make_unique<Fortran::lower::VerifierPass>());
-
-    // Add HLFIR to FIR lowering pass
-    if (emitFIR && useHLFIR)
-      fir::createHLFIRToFIRPassPipeline(pm, llvm::OptimizationLevel::O2);
-
     if (mlir::failed(pm.run(mlirModule))) {
-      llvm::errs() << "FATAL: verification of FIR or HLFIR module failed";
+      llvm::errs() << "FATAL: verification of lowering to FIR failed";
       return mlir::failure();
     }
 
+    if (emitFIR && useHLFIR) {
+      // lower HLFIR to FIR
+      fir::createHLFIRToFIRPassPipeline(pm, llvm::OptimizationLevel::O2);
+      if (mlir::failed(pm.run(mlirModule))) {
+        llvm::errs() << "FATAL: lowering from HLFIR to FIR failed";
+        return mlir::failure();
+      }
+    }
+
     printModule(mlirModule, out);
     return mlir::success();
   } else {

>From ee41dae609e752ad1131e210f8e320e74d2ae841 Mon Sep 17 00:00:00 2001
From: Andrew Gozillon <Andrew.Gozillon at amd.com>
Date: Wed, 7 Feb 2024 09:37:38 -0600
Subject: [PATCH 3/3] [Flang][bbc] Prevent double pass execution for OpenMP
 passes

This is done by invoking them in a seperate pass manager as soon as neccesary.
The alternative would be to place a clear in-between the two invocations of
run and re-add the verification pass, but this seems like a better way to
seperate concerns and keep future logic addiitons simpler.
---
 flang/tools/bbc/bbc.cpp | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/flang/tools/bbc/bbc.cpp b/flang/tools/bbc/bbc.cpp
index 98d9258e023e55..497cf1141c7e85 100644
--- a/flang/tools/bbc/bbc.cpp
+++ b/flang/tools/bbc/bbc.cpp
@@ -256,6 +256,22 @@ createTargetMachine(llvm::StringRef targetTriple, std::string &error) {
                                      /*Reloc::Model=*/std::nullopt)};
 }
 
+/// Build and execute the OpenMPFIRPassPipeline with its own instance
+/// of the pass manager, allowing it to be invoked as soon as it's
+/// required without impacting the main pass pipeline that may be invoked
+/// more than once for verification.
+static mlir::LogicalResult runOpenMPPasses(mlir::ModuleOp mlirModule) {
+  mlir::PassManager pm(mlirModule->getName(),
+                       mlir::OpPassManager::Nesting::Implicit);
+  fir::createOpenMPFIRPassPipeline(pm, enableOpenMPDevice);
+  (void)mlir::applyPassManagerCLOptions(pm);
+  if (mlir::failed(pm.run(mlirModule))) {
+    llvm::errs() << "FATAL: failed to correctly apply OpenMP pass pipeline";
+    return mlir::failure();
+  }
+  return mlir::success();
+}
+
 //===----------------------------------------------------------------------===//
 // Translate Fortran input to FIR, a dialect of MLIR.
 //===----------------------------------------------------------------------===//
@@ -368,14 +384,16 @@ static mlir::LogicalResult convertFortranSourceToMLIR(
                            "could not open output file ")
            << outputName;
 
+  // WARNING: This pipeline must be run immediately after the lowering to
+  // ensure that the FIR is correct with respect to OpenMP operations/
+  // attributes.
+  if (enableOpenMP)
+    if (mlir::failed(runOpenMPPasses(mlirModule)))
+      return mlir::failure();
+
   // Otherwise run the default passes.
   mlir::PassManager pm(mlirModule->getName(),
                        mlir::OpPassManager::Nesting::Implicit);
-  if (enableOpenMP)
-    // WARNING: This pipeline must be run immediately after the lowering to
-    // ensure that the FIR is correct with respect to OpenMP operations/
-    // attributes.
-    fir::createOpenMPFIRPassPipeline(pm, enableOpenMPDevice);
   pm.enableVerifier(/*verifyPasses=*/true);
   (void)mlir::applyPassManagerCLOptions(pm);
   if (passPipeline.hasAnyOccurrences()) {