[flang] run CFG conversion on omp reduction declare ops (#84953)

Most FIR passes only look for FIR operations inside of functions (either
because they run only on func.func or they run on the module but iterate
over functions internally). But there can also be FIR operations inside
of fir.global, some OpenMP and OpenACC container operations.

This has worked so far for fir.global and OpenMP reductions because they
only contained very simple FIR code which doesn't need most passes to be
lowered into LLVM IR. I am not sure how OpenACC works.

In the long run, I hope to see a more systematic approach to making sure
that every pass runs on all of these container operations. I will write
an RFC for this soon.

In the meantime, this pass duplicates the CFG conversion pass to also
run on omp reduction operations. This is similar to how the
AbstractResult pass is already duplicated for fir.global operations.

OpenMP array reductions 2/6
Previous PR: #84952
Next PR: #84954

---------

Co-authored-by: Mats Petersson <[email protected]>

Author: tblah

flang/include/flang/Optimizer/Transforms/Passes.h

@@ -11,6 +11,7 @@
 
 #include "flang/Optimizer/Dialect/FIROps.h"
 #include "mlir/Dialect/LLVMIR/LLVMAttrs.h"
+#include "mlir/Dialect/OpenMP/OpenMPDialect.h"
 #include "mlir/Pass/Pass.h"
 #include "mlir/Pass/PassRegistry.h"
 #include <memory>
@@ -37,7 +38,8 @@ namespace fir {
 #define GEN_PASS_DECL_ANNOTATECONSTANTOPERANDS
 #define GEN_PASS_DECL_ARRAYVALUECOPY
 #define GEN_PASS_DECL_CHARACTERCONVERSION
-#define GEN_PASS_DECL_CFGCONVERSION
+#define GEN_PASS_DECL_CFGCONVERSIONONFUNC
+#define GEN_PASS_DECL_CFGCONVERSIONONREDUCTION
 #define GEN_PASS_DECL_EXTERNALNAMECONVERSION
 #define GEN_PASS_DECL_MEMREFDATAFLOWOPT
 #define GEN_PASS_DECL_SIMPLIFYINTRINSICS
@@ -53,7 +55,8 @@ std::unique_ptr<mlir::Pass> createAbstractResultOnGlobalOptPass();
 std::unique_ptr<mlir::Pass> createAffineDemotionPass();
 std::unique_ptr<mlir::Pass>
 createArrayValueCopyPass(fir::ArrayValueCopyOptions options = {});
-std::unique_ptr<mlir::Pass> createFirToCfgPass();
+std::unique_ptr<mlir::Pass> createFirToCfgOnFuncPass();
+std::unique_ptr<mlir::Pass> createFirToCfgOnReductionPass();
 std::unique_ptr<mlir::Pass> createCharacterConversionPass();
 std::unique_ptr<mlir::Pass> createExternalNameConversionPass();
 std::unique_ptr<mlir::Pass>
@@ -96,6 +99,9 @@ createFunctionAttrPass(FunctionAttrTypes &functionAttr, bool noInfsFPMath,
                        bool noNaNsFPMath, bool approxFuncFPMath,
                        bool noSignedZerosFPMath, bool unsafeFPMath);
 
+void populateCfgConversionRewrites(mlir::RewritePatternSet &patterns,
+                                   bool forceLoopToExecuteOnce = false);
+
 // declarative passes
 #define GEN_PASS_REGISTRATION
 #include "flang/Optimizer/Transforms/Passes.h.inc"

flang/include/flang/Optimizer/Transforms/Passes.td

@@ -145,7 +145,8 @@ def CharacterConversion : Pass<"character-conversion"> {
   ];
 }
 
-def CFGConversion : Pass<"cfg-conversion", "::mlir::func::FuncOp"> {
+class CFGConversionBase<string optExt, string operation>
+  : Pass<"cfg-conversion-on-" # optExt # "-opt", operation> {
   let summary = "Convert FIR structured control flow ops to CFG ops.";
   let description = [{
     Transform the `fir.do_loop`, `fir.if`, `fir.iterate_while` and
@@ -154,7 +155,6 @@ def CFGConversion : Pass<"cfg-conversion", "::mlir::func::FuncOp"> {
 
     This pass is required before code gen to the LLVM IR dialect.
   }];
-  let constructor = "::fir::createFirToCfgPass()";
   let dependentDialects = [
     "fir::FIROpsDialect", "mlir::func::FuncDialect"
   ];
@@ -165,6 +165,14 @@ def CFGConversion : Pass<"cfg-conversion", "::mlir::func::FuncOp"> {
   ];
 }
 
+def CFGConversionOnFunc : CFGConversionBase<"func", "mlir::func::FuncOp"> {
+  let constructor = "::fir::createFirToCfgOnFuncPass()";
+}
+
+def CFGConversionOnReduction : CFGConversionBase<"reduce", "mlir::omp::ReductionDeclareOp"> {
+  let constructor = "::fir::createFirToCfgOnReductionPass()";
+}
+
 def ExternalNameConversion : Pass<"external-name-interop", "mlir::ModuleOp"> {
   let summary = "Convert name for external interoperability";
   let description = [{

flang/include/flang/Tools/CLOptions.inc

@@ -123,7 +123,9 @@ static void addCanonicalizerPassWithoutRegionSimplification(
 
 inline void addCfgConversionPass(mlir::PassManager &pm) {
   addNestedPassConditionally<mlir::func::FuncOp>(
-      pm, disableCfgConversion, fir::createFirToCfgPass);
+      pm, disableCfgConversion, fir::createFirToCfgOnFuncPass);
+  addNestedPassConditionally<mlir::omp::ReductionDeclareOp>(
+      pm, disableCfgConversion, fir::createFirToCfgOnReductionPass);
 }
 
 inline void addAVC(

flang/lib/Optimizer/Transforms/ControlFlowConverter.cpp

@@ -24,7 +24,8 @@
 #include "llvm/Support/CommandLine.h"
 
 namespace fir {
-#define GEN_PASS_DEF_CFGCONVERSION
+#define GEN_PASS_DEF_CFGCONVERSIONONFUNC
+#define GEN_PASS_DEF_CFGCONVERSIONONREDUCTION
 #include "flang/Optimizer/Transforms/Passes.h.inc"
 } // namespace fir
 
@@ -308,13 +309,13 @@ class CfgIterWhileConv : public mlir::OpRewritePattern<fir::IterWhileOp> {
 };
 
 /// Convert FIR structured control flow ops to CFG ops.
-class CfgConversion : public fir::impl::CFGConversionBase<CfgConversion> {
+template <typename Pass, template <typename> class PassBase>
+class CfgConversionTemplate : public PassBase<Pass> {
 public:
   void runOnOperation() override {
-    auto *context = &getContext();
+    auto *context = &this->getContext();
     mlir::RewritePatternSet patterns(context);
-    patterns.insert<CfgLoopConv, CfgIfConv, CfgIterWhileConv>(
-        context, forceLoopToExecuteOnce);
+    fir::populateCfgConversionRewrites(patterns, this->forceLoopToExecuteOnce);
     mlir::ConversionTarget target(*context);
     target.addLegalDialect<mlir::affine::AffineDialect,
                            mlir::cf::ControlFlowDialect, FIROpsDialect,
@@ -323,19 +324,38 @@ class CfgConversion : public fir::impl::CFGConversionBase<CfgConversion> {
     // apply the patterns
     target.addIllegalOp<ResultOp, DoLoopOp, IfOp, IterWhileOp>();
     target.markUnknownOpDynamicallyLegal([](Operation *) { return true; });
-    if (mlir::failed(mlir::applyPartialConversion(getOperation(), target,
+    if (mlir::failed(mlir::applyPartialConversion(this->getOperation(), target,
                                                   std::move(patterns)))) {
       mlir::emitError(mlir::UnknownLoc::get(context),
                       "error in converting to CFG\n");
-      signalPassFailure();
+      this->signalPassFailure();
     }
   }
 };
+
+class CfgConversionOnFunc
+    : public CfgConversionTemplate<CfgConversionOnFunc,
+                                   ::fir::impl::CFGConversionOnFuncBase> {};
+
+class CfgConversionOnReduction
+    : public CfgConversionTemplate<CfgConversionOnReduction,
+                                   ::fir::impl::CFGConversionOnReductionBase> {
+};
 } // namespace
 
+/// Expose conversion rewriters to other passes
+void fir::populateCfgConversionRewrites(mlir::RewritePatternSet &patterns,
+                                        bool forceLoopToExecuteOnce) {
+  patterns.insert<CfgLoopConv, CfgIfConv, CfgIterWhileConv>(
+      patterns.getContext(), forceLoopToExecuteOnce);
+}
+
 /// Convert FIR's structured control flow ops to CFG ops.  This
 /// conversion enables the `createLowerToCFGPass` to transform these to CFG
 /// form.
-std::unique_ptr<mlir::Pass> fir::createFirToCfgPass() {
-  return std::make_unique<CfgConversion>();
+std::unique_ptr<mlir::Pass> fir::createFirToCfgOnFuncPass() {
+  return std::make_unique<CfgConversionOnFunc>();
+}
+std::unique_ptr<mlir::Pass> fir::createFirToCfgOnReductionPass() {
+  return std::make_unique<CfgConversionOnReduction>();
 }

flang/test/Driver/bbc-mlir-pass-pipeline.f90

@@ -38,9 +38,12 @@
 ! CHECK-NEXT:   (S) 0 num-cse'd - Number of operations CSE'd
 ! CHECK-NEXT:   (S) 0 num-dce'd - Number of operations DCE'd
 
+! CHECK-NEXT: Pipeline Collection : ['func.func', 'omp.reduction.declare']
 ! CHECK-NEXT: 'func.func' Pipeline
 ! CHECK-NEXT:   PolymorphicOpConversion
-! CHECK-NEXT:   CFGConversion
+! CHECK-NEXT:   CFGConversionOnFunc
+! CHECK-NEXT: 'omp.reduction.declare' Pipeline
+! CHECK-NEXT:   CFGConversionOnReduction
 
 ! CHECK-NEXT: SCFToControlFlow
 ! CHECK-NEXT: Canonicalizer

flang/test/Driver/mlir-debug-pass-pipeline.f90

@@ -58,10 +58,12 @@
 ! ALL-NEXT:   (S) 0 num-cse'd - Number of operations CSE'd
 ! ALL-NEXT:   (S) 0 num-dce'd - Number of operations DCE'd
 
-! ALL-NEXT: 'func.func' Pipeline
-! ALL-NEXT:   PolymorphicOpConversion
-! ALL-NEXT:   CFGConversion
-
+! ALL-NEXT: Pipeline Collection : ['func.func', 'omp.reduction.declare']
+! ALL-NEXT:   'func.func' Pipeline
+! ALL-NEXT:     PolymorphicOpConversion
+! ALL-NEXT:     CFGConversionOnFunc
+! ALL-NEXT:   'omp.reduction.declare' Pipeline
+! ALL-NEXT:     CFGConversionOnReduction
 ! ALL-NEXT: SCFToControlFlow
 ! ALL-NEXT: Canonicalizer
 ! ALL-NEXT: SimplifyRegionLite
@@ -72,9 +74,9 @@
 
 ! ALL-NEXT: Pipeline Collection : ['fir.global', 'func.func']
 ! ALL-NEXT:   'fir.global' Pipeline
-! ALL-NEXT:    AbstractResultOnGlobalOpt
-! ALL-NEXT:  'func.func' Pipeline
-! ALL-NEXT:    AbstractResultOnFuncOpt
+! ALL-NEXT:   AbstractResultOnGlobalOpt
+! ALL-NEXT:   'func.func' Pipeline
+! ALL-NEXT:   AbstractResultOnFuncOpt
 
 ! ALL-NEXT: CodeGenRewrite
 ! ALL-NEXT:   (S) 0 num-dce'd - Number of operations eliminated

flang/test/Driver/mlir-pass-pipeline.f90

@@ -1,8 +1,8 @@
 ! Test the MLIR pass pipeline
 
-! RUN: %flang_fc1 -S -mmlir --mlir-pass-statistics -mmlir --mlir-pass-statistics-display=pipeline -o /dev/null %s 2>&1 | FileCheck --check-prefixes=ALL %s
+! RUN: %flang_fc1 -S -mmlir --mlir-pass-statistics -mmlir --mlir-pass-statistics-display=pipeline -o /dev/null %s 2>&1 | FileCheck --check-prefixes=ALL,NOTO2 %s
 ! -O0 is the default:
-! RUN: %flang_fc1 -S -mmlir --mlir-pass-statistics -mmlir --mlir-pass-statistics-display=pipeline %s -O0 -o /dev/null 2>&1 | FileCheck --check-prefixes=ALL %s
+! RUN: %flang_fc1 -S -mmlir --mlir-pass-statistics -mmlir --mlir-pass-statistics-display=pipeline %s -O0 -o /dev/null 2>&1 | FileCheck --check-prefixes=ALL,NOTO2 %s
 ! RUN: %flang_fc1 -S -mmlir --mlir-pass-statistics -mmlir --mlir-pass-statistics-display=pipeline %s -O2 -o /dev/null 2>&1 | FileCheck --check-prefixes=ALL,O2 %s
 
 ! REQUIRES: asserts
@@ -49,11 +49,15 @@
 ! ALL-NEXT:   (S) 0 num-cse'd - Number of operations CSE'd
 ! ALL-NEXT:   (S) 0 num-dce'd - Number of operations DCE'd
 
-! ALL-NEXT: 'func.func' Pipeline
-! ALL-NEXT:   PolymorphicOpConversion
+! O2-NEXT:    'func.func' Pipeline
+! O2-NEXT:      PolymorphicOpConversion
 ! O2-NEXT:  AddAliasTags
-! O2-NEXT:  'func.func' Pipeline
-! ALL-NEXT:   CFGConversion
+! ALL-NEXT: Pipeline Collection : ['func.func', 'omp.reduction.declare']
+! ALL-NEXT:    'func.func' Pipeline
+! NOTO2-NEXT:      PolymorphicOpConversion
+! ALL-NEXT:      CFGConversionOnFunc
+! ALL-NEXT:   'omp.reduction.declare' Pipeline
+! ALL-NEXT:      CFGConversionOnReduction
 
 ! ALL-NEXT: SCFToControlFlow
 ! ALL-NEXT: Canonicalizer

flang/test/Fir/array-value-copy-2.fir

@@ -1,5 +1,5 @@
-// RUN: fir-opt --array-value-copy --cfg-conversion %s | FileCheck %s
-// RUN: fir-opt --array-value-copy="optimize-conflicts=true" --cfg-conversion %s | FileCheck %s
+// RUN: fir-opt --array-value-copy --cfg-conversion-on-func-opt %s | FileCheck %s
+// RUN: fir-opt --array-value-copy="optimize-conflicts=true" --cfg-conversion-on-func-opt %s | FileCheck %s
 
 // CHECK-LABEL: func @_QPslice1(
 // CHECK-NOT: fir.allocmem

flang/test/Fir/basic-program.fir

@@ -60,8 +60,11 @@ func.func @_QQmain() {
 
 // PASSES-NEXT: AddAliasTags
 
+// PASSES-NEXT: Pipeline Collection : ['func.func', 'omp.reduction.declare']
 // PASSES-NEXT: 'func.func' Pipeline
-// PASSES-NEXT:   CFGConversion
+// PASSES-NEXT:   CFGConversionOnFunc
+// PASSES-NEXT: 'omp.reduction.declare' Pipeline
+// PASSES-NEXT:   CFGConversionOnReduction
 
 // PASSES-NEXT: SCFToControlFlow
 // PASSES-NEXT: Canonicalizer

flang/test/Fir/convert-to-llvm-openmp-and-fir.fir

@@ -1,4 +1,4 @@
-// RUN: fir-opt --split-input-file --cfg-conversion --fir-to-llvm-ir="target=aarch64-unknown-linux-gnu" %s | FileCheck %s
+// RUN: fir-opt --split-input-file --cfg-conversion-on-func-opt --fir-to-llvm-ir="target=aarch64-unknown-linux-gnu" %s | FileCheck %s
 
 func.func @_QPsb1(%arg0: !fir.ref<i32> {fir.bindc_name = "n"}, %arg1: !fir.ref<!fir.array<?xi32>> {fir.bindc_name = "arr"}) {
   %c1_i64 = arith.constant 1 : i64

flang/test/Fir/loop01.fir

@@ -1,4 +1,4 @@
-// RUN: fir-opt --split-input-file --cfg-conversion %s | FileCheck %s
+// RUN: fir-opt --split-input-file --cfg-conversion-on-func-opt %s | FileCheck %s
 
 func.func @x(%lb : index, %ub : index, %step : index, %b : i1, %addr : !fir.ref<index>) {
   fir.do_loop %iv = %lb to %ub step %step unordered {

flang/test/Fir/loop02.fir

@@ -1,5 +1,5 @@
-// RUN: fir-opt --cfg-conversion="always-execute-loop-body=true" %s | FileCheck %s
-// RUN: fir-opt --cfg-conversion %s | FileCheck %s --check-prefix=NOOPT
+// RUN: fir-opt --cfg-conversion-on-func-opt="always-execute-loop-body=true" %s | FileCheck %s
+// RUN: fir-opt --cfg-conversion-on-func-opt %s | FileCheck %s --check-prefix=NOOPT
 
 func.func @x(%addr : !fir.ref<index>) {
   %bound = arith.constant 452 : index

flang/test/Lower/OpenMP/FIR/flush.f90

@@ -1,7 +1,7 @@
 ! This test checks lowering of OpenMP Flush Directive.
 
 !RUN: %flang_fc1 -emit-fir -flang-deprecated-no-hlfir -fopenmp %s -o - | FileCheck %s --check-prefixes="FIRDialect,OMPDialect"
-!RUN: %flang_fc1 -emit-fir -flang-deprecated-no-hlfir -fopenmp %s -o - | fir-opt --cfg-conversion | fir-opt --fir-to-llvm-ir | FileCheck %s --check-prefixes="LLVMIRDialect,OMPDialect"
+!RUN: %flang_fc1 -emit-fir -flang-deprecated-no-hlfir -fopenmp %s -o - | fir-opt --cfg-conversion-on-func-opt | fir-opt --fir-to-llvm-ir | FileCheck %s --check-prefixes="LLVMIRDialect,OMPDialect"
 
 subroutine flush_standalone(a, b, c)
     integer, intent(inout) :: a, b, c

flang/test/Lower/OpenMP/FIR/master.f90

@@ -1,5 +1,5 @@
 !RUN: %flang_fc1 -emit-fir -flang-deprecated-no-hlfir -fopenmp %s -o - | FileCheck %s --check-prefixes="FIRDialect,OMPDialect"
-!RUN: %flang_fc1 -emit-fir -flang-deprecated-no-hlfir -fopenmp %s -o - | fir-opt --cfg-conversion | fir-opt --fir-to-llvm-ir | FileCheck %s --check-prefixes="OMPDialect"
+!RUN: %flang_fc1 -emit-fir -flang-deprecated-no-hlfir -fopenmp %s -o - | fir-opt --cfg-conversion-on-func-opt | fir-opt --fir-to-llvm-ir | FileCheck %s --check-prefixes="OMPDialect"
 
 !===============================================================================
 ! parallel construct with function call which has master construct internally

flang/test/Lower/OpenMP/FIR/parallel-sections.f90

@@ -1,5 +1,5 @@
 !RUN: %flang_fc1 -emit-fir -flang-deprecated-no-hlfir -fopenmp %s -o - | FileCheck %s --check-prefixes="FIRDialect,OMPDialect"
-!RUN: %flang_fc1 -emit-fir -flang-deprecated-no-hlfir -fopenmp %s -o - | fir-opt --cfg-conversion | fir-opt --fir-to-llvm-ir | FileCheck %s --check-prefixes="OMPDialect,LLVMDialect"
+!RUN: %flang_fc1 -emit-fir -flang-deprecated-no-hlfir -fopenmp %s -o - | fir-opt --cfg-conversion-on-func-opt | fir-opt --fir-to-llvm-ir | FileCheck %s --check-prefixes="OMPDialect,LLVMDialect"
 
 !===============================================================================
 ! Parallel sections construct

flang/test/Transforms/omp-reduction-cfg-conversion.fir

@@ -0,0 +1,57 @@
+// RUN: fir-opt --cfg-conversion-on-reduce-opt %s | FileCheck %s
+
+omp.reduction.declare @add_reduction_i_32_box_3_byref : !fir.ref<!fir.box<!fir.array<3xi32>>> init {
+^bb0(%arg0: !fir.ref<!fir.box<!fir.array<3xi32>>>):
+  %c4_i32 = arith.constant 4 : i32
+  %c0_i32 = arith.constant 0 : i32
+  %c3 = arith.constant 3 : index
+  %0 = fir.alloca !fir.box<!fir.array<3xi32>>
+  %1 = fir.alloca !fir.array<3xi32> {bindc_name = "omp.reduction.array.init"}
+  %2 = fir.shape %c3 : (index) -> !fir.shape<1>
+  %3 = fir.declare %1(%2) {uniq_name = "omp.reduction.array.init"} : (!fir.ref<!fir.array<3xi32>>, !fir.shape<1>) -> !fir.ref<!fir.array<3xi32>>
+  %4 = fir.embox %3(%2) : (!fir.ref<!fir.array<3xi32>>, !fir.shape<1>) -> !fir.box<!fir.array<3xi32>>
+  %5 = fir.alloca i32
+  fir.store %c0_i32 to %5 : !fir.ref<i32>
+  %6 = fir.embox %5 : (!fir.ref<i32>) -> !fir.box<i32>
+  fir.store %4 to %0 : !fir.ref<!fir.box<!fir.array<3xi32>>>
+  %7 = fir.address_of(@_QQclX9a9fdf8c5fd329fbbf2b0c08e2ca9a1e) : !fir.ref<!fir.char<1,40>>
+  %8 = fir.convert %0 : (!fir.ref<!fir.box<!fir.array<3xi32>>>) -> !fir.ref<!fir.box<none>>
+  %9 = fir.convert %6 : (!fir.box<i32>) -> !fir.box<none>
+  %10 = fir.convert %7 : (!fir.ref<!fir.char<1,40>>) -> !fir.ref<i8>
+  %11 = fir.call @_FortranAAssign(%8, %9, %10, %c4_i32) : (!fir.ref<!fir.box<none>>, !fir.box<none>, !fir.ref<i8>, i32) -> none
+  %12 = fir.alloca !fir.box<!fir.array<3xi32>>
+  fir.store %4 to %12 : !fir.ref<!fir.box<!fir.array<3xi32>>>
+  omp.yield(%12 : !fir.ref<!fir.box<!fir.array<3xi32>>>)
+} combiner {
+^bb0(%arg0: !fir.ref<!fir.box<!fir.array<3xi32>>>, %arg1: !fir.ref<!fir.box<!fir.array<3xi32>>>):
+  %c1 = arith.constant 1 : index
+  %c0 = arith.constant 0 : index
+  %0 = fir.load %arg0 : !fir.ref<!fir.box<!fir.array<3xi32>>>
+  %1 = fir.load %arg1 : !fir.ref<!fir.box<!fir.array<3xi32>>>
+  %2:3 = fir.box_dims %0, %c0 : (!fir.box<!fir.array<3xi32>>, index) -> (index, index, index)
+  %3 = fir.shape_shift %2#0, %2#1 : (index, index) -> !fir.shapeshift<1>
+  fir.do_loop %arg2 = %c1 to %2#1 step %c1 unordered {
+    %4 = fir.array_coor %0(%3) %arg2 : (!fir.box<!fir.array<3xi32>>, !fir.shapeshift<1>, index) -> !fir.ref<i32>
+    %5 = fir.array_coor %1(%3) %arg2 : (!fir.box<!fir.array<3xi32>>, !fir.shapeshift<1>, index) -> !fir.ref<i32>
+    %6 = fir.load %4 : !fir.ref<i32>
+    %7 = fir.load %5 : !fir.ref<i32>
+    %8 = arith.addi %6, %7 : i32
+    fir.store %8 to %4 : !fir.ref<i32>
+  }
+  omp.yield(%arg0 : !fir.ref<!fir.box<!fir.array<3xi32>>>)
+}
+
+// ensure cfg conversion has run on the do loop
+// CHECK: combiner {
+// CHECK-NOT: fir.do_loop
+// CHECK: ^bb0({{.*}}):
+// ...
+// CHECK:   cf.br ^bb1
+// CHECK: ^bb1({{.*}}):
+// ...
+// CHECK:   cf.cond_br %{{.*}} ^bb2, ^bb3
+// CHECK: ^bb2:
+// ...
+// CHECK:   cf.br ^bb1
+// CHECK: ^bb3:
+