[Flang][OpenMP] Run Flang-specific OpenMP MLIR passes in bbc #66633

skatrak · 2023-09-18T11:39:59Z

This patch moves the group of OpenMP MLIR passes ran after lowering of Fortran to MLIR into a pipeline to be shared by flang-new and bbc. Currently, the bbc tool does not produce the expected FIR for offloading- enabled OpenMP codes due to not running these passes.

Unit tests exercising these passes are updated to check bbc output as well.

llvmbot · 2023-09-18T11:41:35Z

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

@llvm/pr-subscribers-flang-driver

Changes

This patch moves the group of OpenMP MLIR passes ran after lowering of Fortran to MLIR into a pipeline to be shared by flang-new and bbc. Currently, the bbc tool does not produce the expected FIR for offloading- enabled OpenMP codes due to not running these passes.

Unit tests exercising these passes are updated to check `bbc` output as well.

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

9 Files Affected:

(modified) flang/include/flang/Tools/CLOptions.inc (+18)
(modified) flang/lib/Frontend/FrontendActions.cpp (+3-8)
(modified) flang/test/Driver/omp-cse-region-boundary.f90 (+3-1)
(modified) flang/test/Lower/OpenMP/declare-target-implicit-func-and-subr-cap.f90 (+2)
(modified) flang/test/Lower/OpenMP/declare-target-implicit-tarop-cap.f90 (+2)
(modified) flang/test/Lower/OpenMP/function-filtering-2.f90 (+2)
(modified) flang/test/Lower/OpenMP/function-filtering.f90 (+2)
(modified) flang/test/Lower/OpenMP/omp-target-early-outlining.f90 (+2)
(modified) flang/tools/bbc/bbc.cpp (+2)

diff --git a/flang/include/flang/Tools/CLOptions.inc b/flang/include/flang/Tools/CLOptions.inc
index e2a071d75169790..616d9ddc066a75d 100644
--- a/flang/include/flang/Tools/CLOptions.inc
+++ b/flang/include/flang/Tools/CLOptions.inc
@@ -254,6 +254,24 @@ inline void createHLFIRToFIRPassPipeline(
   pm.addPass(hlfir::createConvertHLFIRtoFIRPass());
 }
 
+/// Create a pass pipeline for handling certain OpenMP transformations needed
+/// prior to FIR lowering.
+///
+/// WARNING: These passes must be run immediately after the lowering to ensure
+/// that the FIR is correct with respect to OpenMP operations/attributes.
+///
+/// \param pm - MLIR pass manager that will hold the pipeline definition.
+/// \param isTargetDevice - Whether code is being generated for a target device
+/// rather than the host device.
+inline void createOpenMPFIRPassPipeline(
+    mlir::PassManager &pm, bool isTargetDevice) {
+  pm.addPass(fir::createOMPMarkDeclareTargetPass());
+  if (isTargetDevice) {
+    pm.addPass(fir::createOMPEarlyOutliningPass());
+    pm.addPass(fir::createOMPFunctionFilteringPass());
+  }
+}
+
 #if !defined(FLANG_EXCLUDE_CODEGEN)
 inline void createDebugPasses(
     mlir::PassManager &pm, llvm::codegenoptions::DebugInfoKind debugLevel) {
diff --git a/flang/lib/Frontend/FrontendActions.cpp b/flang/lib/Frontend/FrontendActions.cpp
index 1cb253a9c04165c..486abc9b08320d1 100644
--- a/flang/lib/Frontend/FrontendActions.cpp
+++ b/flang/lib/Frontend/FrontendActions.cpp
@@ -70,6 +70,8 @@
 #include <memory>
 #include <system_error>
 
+#include "flang/Tools/CLOptions.inc"
+
 using namespace Fortran::frontend;
 
 // Declare plugin extension function declarations.
@@ -313,12 +315,7 @@ bool CodeGenAction::beginSourceFileAction() {
     if (auto offloadMod = llvm::dyn_cast<mlir::omp::OffloadModuleInterface>(
             mlirModule->getOperation()))
       isDevice = offloadMod.getIsTargetDevice();
-
-    pm.addPass(fir::createOMPMarkDeclareTargetPass());
-    if (isDevice) {
-      pm.addPass(fir::createOMPEarlyOutliningPass());
-      pm.addPass(fir::createOMPFunctionFilteringPass());
-    }
+    fir::createOpenMPFIRPassPipeline(pm, isDevice);
   }
 
   pm.enableVerifier(/*verifyPasses=*/true);
@@ -651,8 +648,6 @@ void GetSymbolsSourcesAction::executeAction() {
 
 CodeGenAction::~CodeGenAction() = default;
 
-#include "flang/Tools/CLOptions.inc"
-
 static llvm::OptimizationLevel
 mapToLevel(const Fortran::frontend::CodeGenOptions &opts) {
   switch (opts.OptimizationLevel) {
diff --git a/flang/test/Driver/omp-cse-region-boundary.f90 b/flang/test/Driver/omp-cse-region-boundary.f90
index 99388da1b143241..4858676770b3d76 100644
--- a/flang/test/Driver/omp-cse-region-boundary.f90
+++ b/flang/test/Driver/omp-cse-region-boundary.f90
@@ -3,6 +3,8 @@
 !compiling for the host CSE is done.
 !RUN: %flang_fc1 -fopenmp-is-device -emit-mlir -fopenmp %s -o - | fir-opt -cse | FileCheck %s -check-prefix=CHECK-DEVICE
 !RUN: %flang_fc1 -emit-mlir -fopenmp %s -o - | fir-opt -cse | FileCheck %s -check-prefix=CHECK-HOST
+!RUN: bbc -fopenmp-is-target-device -emit-fir -fopenmp %s -o - | fir-opt -cse | FileCheck %s -check-prefix=CHECK-DEVICE
+!RUN: bbc -emit-fir -fopenmp %s -o - | fir-opt -cse | FileCheck %s -check-prefix=CHECK-HOST
 
 !Constant should be present inside target region.
 !CHECK-DEVICE: omp.target
@@ -19,7 +21,7 @@ subroutine writeIndex(sum)
         integer :: myconst1
         integer :: myconst2
         myconst1 = 10
-!$omp target map(from:new_len)
+!$omp target map(from:myconst2)
         myconst2 = 10
 !$omp end target
         sum = myconst2 + myconst2
diff --git a/flang/test/Lower/OpenMP/declare-target-implicit-func-and-subr-cap.f90 b/flang/test/Lower/OpenMP/declare-target-implicit-func-and-subr-cap.f90
index a9f0c8883751d5c..4d621f9977944c2 100644
--- a/flang/test/Lower/OpenMP/declare-target-implicit-func-and-subr-cap.f90
+++ b/flang/test/Lower/OpenMP/declare-target-implicit-func-and-subr-cap.f90
@@ -1,5 +1,7 @@
 !RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
 !RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-device %s -o - | FileCheck %s  --check-prefix=DEVICE
+!RUN: bbc -emit-fir -fopenmp %s -o - | FileCheck %s
+!RUN: bbc -emit-fir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s --check-prefix=DEVICE
 
 ! CHECK-LABEL: func.func @_QPimplicitly_captured
 ! CHECK-SAME: {{.*}}attributes {omp.declare_target = #omp.declaretarget<device_type = (any), capture_clause = (to)>{{.*}}}
diff --git a/flang/test/Lower/OpenMP/declare-target-implicit-tarop-cap.f90 b/flang/test/Lower/OpenMP/declare-target-implicit-tarop-cap.f90
index b597513b611056e..cee24883f47151b 100644
--- a/flang/test/Lower/OpenMP/declare-target-implicit-tarop-cap.f90
+++ b/flang/test/Lower/OpenMP/declare-target-implicit-tarop-cap.f90
@@ -1,5 +1,7 @@
 !RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s
 !RUN: %flang_fc1 -emit-fir -fopenmp -fopenmp-is-device %s -o - | FileCheck %s  --check-prefix=DEVICE
+!RUN: bbc -emit-fir -fopenmp %s -o - | FileCheck %s
+!RUN: bbc -emit-fir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s --check-prefix=DEVICE
 
 ! DEVICE-LABEL: func.func @_QPimplicit_capture
 ! DEVICE-SAME: {{.*}}attributes {omp.declare_target = #omp.declaretarget<device_type = (nohost), capture_clause = (to)>{{.*}}}
diff --git a/flang/test/Lower/OpenMP/function-filtering-2.f90 b/flang/test/Lower/OpenMP/function-filtering-2.f90
index a12b88d21f8785c..8065467504dd27d 100644
--- a/flang/test/Lower/OpenMP/function-filtering-2.f90
+++ b/flang/test/Lower/OpenMP/function-filtering-2.f90
@@ -2,6 +2,8 @@
 ! RUN: %flang_fc1 -fopenmp -emit-mlir %s -o - | FileCheck --check-prefix=MLIR %s
 ! RUN: %flang_fc1 -fopenmp -fopenmp-is-target-device -emit-llvm %s -o - | FileCheck --check-prefix=LLVM %s
 ! RUN: %flang_fc1 -fopenmp -fopenmp-is-target-device -emit-mlir %s -o - | FileCheck --check-prefix=MLIR %s
+! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck --check-prefixes=MLIR-HOST,MLIR-ALL %s
+! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-fir %s -o - | FileCheck --check-prefixes=MLIR-DEVICE,MLIR-ALL %s
 
 ! MLIR: func.func @{{.*}}implicit_invocation() attributes {omp.declare_target = #omp.declaretarget<device_type = (nohost), capture_clause = (to)>}
 ! MLIR: return
diff --git a/flang/test/Lower/OpenMP/function-filtering.f90 b/flang/test/Lower/OpenMP/function-filtering.f90
index 83302205943f332..c0ca351b9b335f0 100644
--- a/flang/test/Lower/OpenMP/function-filtering.f90
+++ b/flang/test/Lower/OpenMP/function-filtering.f90
@@ -2,6 +2,8 @@
 ! RUN: %flang_fc1 -fopenmp -emit-mlir %s -o - | FileCheck --check-prefixes=MLIR-HOST,MLIR-ALL %s
 ! RUN: %flang_fc1 -fopenmp -fopenmp-is-target-device -emit-llvm %s -o - | FileCheck --check-prefixes=LLVM-DEVICE,LLVM-ALL %s
 ! RUN: %flang_fc1 -fopenmp -fopenmp-is-target-device -emit-mlir %s -o - | FileCheck --check-prefixes=MLIR-DEVICE,MLIR-ALL %s
+! RUN: bbc -fopenmp -emit-fir %s -o - | FileCheck --check-prefixes=MLIR-HOST,MLIR-ALL %s
+! RUN: bbc -fopenmp -fopenmp-is-target-device -emit-fir %s -o - | FileCheck --check-prefixes=MLIR-DEVICE,MLIR-ALL %s
 
 ! Check that the correct LLVM IR functions are kept for the host and device
 ! after running the whole set of translation and transformation passes from
diff --git a/flang/test/Lower/OpenMP/omp-target-early-outlining.f90 b/flang/test/Lower/OpenMP/omp-target-early-outlining.f90
index 724f363c013cc6e..5385c075743a339 100644
--- a/flang/test/Lower/OpenMP/omp-target-early-outlining.f90
+++ b/flang/test/Lower/OpenMP/omp-target-early-outlining.f90
@@ -2,6 +2,8 @@
 
 !RUN: %flang_fc1 -triple amdgcn-amd-amdhsa -emit-fir -fopenmp -fopenmp-is-device %s -o - | FileCheck %s
 !RUN: %flang_fc1 -triple x86_64-unknown-linux-gnu -emit-fir -fopenmp -fopenmp-is-device %s -o - | FileCheck %s
+!RUN: bbc -emit-fir -fopenmp -fopenmp-is-target-device %s -o - | FileCheck %s 
+!RUN: bbc -emit-fir -fopenmp -fopenmp-is-gpu -fopenmp-is-target-device %s -o - | FileCheck %s 
 
 !CHECK: func.func @_QPtarget_function
 
diff --git a/flang/tools/bbc/bbc.cpp b/flang/tools/bbc/bbc.cpp
index f3783ea6f1ce6aa..3f753753bd30d47 100644
--- a/flang/tools/bbc/bbc.cpp
+++ b/flang/tools/bbc/bbc.cpp
@@ -329,6 +329,8 @@ static mlir::LogicalResult convertFortranSourceToMLIR(
   // Otherwise run the default passes.
   mlir::PassManager pm(mlirModule->getName(),
                        mlir::OpPassManager::Nesting::Implicit);
+  if (enableOpenMP)
+    fir::createOpenMPFIRPassPipeline(pm, enableOpenMPDevice);
   pm.enableVerifier(/*verifyPasses=*/true);
   (void)mlir::applyPassManagerCLOptions(pm);
   if (passPipeline.hasAnyOccurrences()) {

kiranchandramohan

LG.

Please wait for @jsjodin or @agozillon.

flang/include/flang/Tools/CLOptions.inc

agozillon · 2023-09-18T12:06:22Z

LGTM.

And nice catch with the test where we've specified the incorrect/non-existent variable name in omp-cse-region-boundary.f90, I spotted it as well it seems we need to add a semantic check for it at some point in the near future, it might be worth opening an issue if there isn't one that exists already.

This patch moves the group of OpenMP MLIR passes using after lowering of Fortran to MLIR into a pipeline to be shared by `flang-new` and `bbc`. Currently, the `bbc` tool does not produce the expected FIR for offloading- enabled OpenMP codes due to not running these passes. Unit tests exercising these passes are updated to check `bbc` output as well.

skatrak · 2023-09-18T13:16:43Z

I just squashed and merged this manually in a way that GitHub didn't seem to pick up: fb4bdf3. I suppose I should close this PR now and just click the button next time. Is any other way to proceed preferred? Sorry for the inconvenience!

kiranchandramohan · 2023-09-18T14:07:36Z

Yes, you can close.

skatrak requested review from jsjodin, kiranchandramohan, raghavendhra, agozillon, DominikAdamski, shraiysh and TIFitis September 18, 2023 11:39

llvmbot added flang:driver flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp labels Sep 18, 2023

kiranchandramohan approved these changes Sep 18, 2023

View reviewed changes

flang/include/flang/Tools/CLOptions.inc Show resolved Hide resolved

skatrak force-pushed the bbc-omp-passes branch from c877a04 to 44927ad Compare September 18, 2023 13:12

skatrak closed this Sep 18, 2023

skatrak deleted the bbc-omp-passes branch September 18, 2023 14:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Flang][OpenMP] Run Flang-specific OpenMP MLIR passes in bbc #66633

[Flang][OpenMP] Run Flang-specific OpenMP MLIR passes in bbc #66633

skatrak commented Sep 18, 2023

llvmbot commented Sep 18, 2023 •

edited

Unit tests exercising these passes are updated to check `bbc` output as well.

kiranchandramohan left a comment

agozillon commented Sep 18, 2023

skatrak commented Sep 18, 2023

kiranchandramohan commented Sep 18, 2023

[Flang][OpenMP] Run Flang-specific OpenMP MLIR passes in bbc #66633

[Flang][OpenMP] Run Flang-specific OpenMP MLIR passes in bbc #66633

Conversation

skatrak commented Sep 18, 2023

llvmbot commented Sep 18, 2023 • edited

Unit tests exercising these passes are updated to check bbc output as well.

kiranchandramohan left a comment

Choose a reason for hiding this comment

agozillon commented Sep 18, 2023

skatrak commented Sep 18, 2023

kiranchandramohan commented Sep 18, 2023

llvmbot commented Sep 18, 2023 •

edited

Unit tests exercising these passes are updated to check `bbc` output as well.