[flang] Runs FIR SimplifyRegionLitePass pass after calling the inliner

[jeanPerier]

Summary

In flang pipeline, the inliner calls createCanonicalizerPass with the region
simplification disabled. The inliner pass does canonicalization even if
no inlining happens. After canonicalization, FIR lite region simplification
must be called to get rid of unreachable regions.
This code exposes the need to run SimplifyRegionLitePass after the inliner is
called with FIR pipeline.

[awarzynski]

In flang pipeline, the inliner calls createCanonicalizerPass with the region simplification disabled.

Why not enable region simplification instead? Just curious.

flang/test/Fir/simplify-region-lite-after-inliner.fir

1

[clementval]

LGTM

[PeteSteinfeld]

All builds and tests correctly and looks good.

[kiranchandramohan]

Just a drive-through comment. Feel free to ignore it.

We have a call to simplifyregions in the lowering code with a comment to move this to a pass. With this change can we remove that call in Bridge.cpp? Or alternatively call the simplifyregion pass early in the flow?
https://github.com/llvm/llvm-project/blob/883b0d5b7f873a7d6f8c8ee13c7f6174a2a79a50/flang/lib/Lower/Bridge.cpp#L2830

[jeanPerier]

In D130484#3676207, @awarzynski wrote:

In flang pipeline, the inliner calls createCanonicalizerPass with the region simplification disabled.

Why not enable region simplification instead? Just curious.

The issue with the MLIR regions simplification is that it always does block merging, and block merging breaks some properties of high level FIR where some operand are expected to be compile time constants/parent operation are expected to be visible: see https://reviews.llvm.org/D109579#3052374 for a better explanation.

In D130484#3676302, @kiranchandramohan wrote:

Just a drive-through comment. Feel free to ignore it.

We have a call to simplifyregions in the lowering code with a comment to move this to a pass. With this change can we remove that call in Bridge.cpp? Or alternatively call the simplify region pass early in the flow?
https://github.com/llvm/llvm-project/blob/883b0d5b7f873a7d6f8c8ee13c7f6174a2a79a50/flang/lib/Lower/Bridge.cpp#L2830

Thanks for the suggestion, the call to simplify region is required to produce valid CFGs after lowering (because we allow ourselves to produce unreachable blocks in lowering to simplify things), so this call does not allow removing it since many passes are called in between (in fact some regions createSimplifyRegionLitePass is already added a few line above in createDefaultFIROptimizerPassPipeline), it needs to be called every type a pass may create dead code.

flang/include/flang/Tools/CLOptions.inc

inline void createDefaultFIROptimizerPassPipeline(
   pm.addPass(mlir::createCSEPass());
   fir::addMemoryAllocationOpt(pm);
 
   // The default inliner pass adds the canonicalizer pass with the default
   // configuration. Create the inliner pass with tco config.
   llvm::StringMap<mlir::OpPassManager> pipelines;
   pm.addPass(
       mlir::createInlinerPass(pipelines, defaultFlangInlinerOptPipeline));
+  pm.addPass(fir::createSimplifyRegionLitePass());
   pm.addPass(mlir::createCSEPass());
 
   // convert control flow to CFG form
   fir::addCfgConversionPass(pm);
   pm.addPass(mlir::createConvertSCFToCFPass());
 
   pm.addPass(mlir::createCanonicalizerPass(config));
   pm.addPass(fir::createSimplifyRegionLitePass());

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

 ! CHECK-NEXT: CSE
 ! 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: 'func.func' Pipeline
 ! CHECK-NEXT:   MemoryAllocationOpt
 
 ! CHECK-NEXT: Inliner
+! CHECK-NEXT: SimplifyRegionLite
 ! CHECK-NEXT: CSE
 ! 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: 'func.func' Pipeline
 ! CHECK-NEXT:   CFGConversion
 
 ! CHECK-NEXT: SCFToControlFlow
 ! CHECK-NEXT: Canonicalizer
 ! CHECK-NEXT: SimplifyRegionLite
 ! CHECK-NEXT: CSE
 ! 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-NOT: LLVMIRLoweringPass

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

 ! ALL-NEXT: CSE
 ! 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:   MemoryAllocationOpt
 
 ! ALL-NEXT: Inliner
+! ALL-NEXT: SimplifyRegionLite
 ! ALL-NEXT: CSE
 ! 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:   CFGConversion
 
 ! ALL-NEXT: SCFToControlFlow

flang/test/Fir/basic-program.fir

 // PASSES-NEXT: CSE
 // PASSES-NEXT:   (S) 0 num-cse'd - Number of operations CSE'd
 // PASSES-NEXT:   (S) 0 num-dce'd - Number of operations DCE'd
 
 // PASSES-NEXT: 'func.func' Pipeline
 // PASSES-NEXT:   MemoryAllocationOpt
 
 // PASSES-NEXT: Inliner
+// PASSES-NEXT: SimplifyRegionLite
 // PASSES-NEXT: CSE
 // PASSES-NEXT:   (S) 0 num-cse'd - Number of operations CSE'd
 // PASSES-NEXT:   (S) 0 num-dce'd - Number of operations DCE'd
 
 // PASSES-NEXT: 'func.func' Pipeline
 // PASSES-NEXT:   CFGConversion
 
 // PASSES-NEXT: SCFToControlFlow

flang/test/Fir/simplify-region-lite-after-inliner.fir

+// RUN: tco %s | FileCheck %s
+
+
+// In flang pipeline, the inliner calls createCanonicalizerPass with the region
+// simplification disabled. The inliner pass does canonicalization even if
+// no inlining happens. After canonicalization, FIR lite region simplification
+// must be called to get rid of unreachable regions.
+// This code exposes the need to run SimplifyRegionLitePass after the inliner is
+// called with FIR pipeline.
+
+
+func.func @repro(%arg0: i8, %arg1: i8) {
+  %c34_i8 = arith.constant 34 : i8
+  %c-1_i8 = arith.constant -1 : i8
+  %2 = arith.xori %c34_i8, %c-1_i8 : i8
+  %3 = arith.andi %arg0, %c34_i8 : i8
+  %4 = arith.andi %arg1, %2 : i8
+  %5 = arith.ori %3, %4 : i8
+  %c34_i8_0 = arith.constant 34 : i8
+  %7 = arith.andi %arg0, %c34_i8_0 : i8
+  %c-35_i8 = arith.constant -35 : i8
+  %9 = arith.andi %arg1, %c-35_i8 : i8
+  %10 = arith.ori %7, %9 : i8
+  %11 = arith.cmpi ne, %5, %10 : i8
+  cf.cond_br %11, ^bb1, ^bb2
+^bb1:  // pred: ^bb0
+  %13 = func.call @foo() : () -> none
+  cf.br ^bb2
+^bb2:  // pred: ^bb0, ^bb2
+  return
+}
+func.func private @foo() -> none
+
+
+// CHECK: define void @repro(i8 %0, i8 %1)
+//   CHECK-NEXT  ret void