[Mlir-commits] [mlir] a506279 - [mlir] Do not merge blocks during canonicalization by default (#95057)
llvmlistbot at llvm.org
llvmlistbot at llvm.org
Fri Jun 14 13:39:01 PDT 2024
Author: Mehdi Amini
Date: 2024-06-14T22:38:56+02:00
New Revision: a506279e5c5d5668e66b0749c26a93d8d373931a
URL: https://github.com/llvm/llvm-project/commit/a506279e5c5d5668e66b0749c26a93d8d373931a
DIFF: https://github.com/llvm/llvm-project/commit/a506279e5c5d5668e66b0749c26a93d8d373931a.diff
LOG: [mlir] Do not merge blocks during canonicalization by default (#95057)
This is a heavy process, and it can trigger a massive explosion in
adding block arguments. While potentially reducing the code size, the
resulting merged blocks with arguments are hiding some of the def-use
chain and can even hinder some further analyses/optimizations: a merge
block does not have it's own path-sensitive context, instead the context
is merged from all the predecessors.
Previous behavior can be restored by passing:
{test-convergence region-simplify=aggressive}
to the canonicalize pass.
Added:
Modified:
flang/include/flang/Tools/CLOptions.inc
flang/lib/Optimizer/HLFIR/Transforms/InlineElementals.cpp
flang/lib/Optimizer/HLFIR/Transforms/LowerHLFIRIntrinsics.cpp
flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp
flang/lib/Optimizer/Transforms/AssumedRankOpConversion.cpp
flang/lib/Optimizer/Transforms/StackArrays.cpp
mlir/include/mlir/Transforms/GreedyPatternRewriteDriver.h
mlir/include/mlir/Transforms/Passes.h
mlir/include/mlir/Transforms/Passes.td
mlir/include/mlir/Transforms/RegionUtils.h
mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp
mlir/lib/Transforms/Utils/RegionUtils.cpp
mlir/test/Dialect/SPIRV/Transforms/canonicalize.mlir
mlir/test/Pass/run-reproducer.mlir
mlir/test/Transforms/canonicalize-block-merge.mlir
mlir/test/Transforms/canonicalize-dce.mlir
mlir/test/Transforms/canonicalize.mlir
mlir/test/Transforms/test-canonicalize.mlir
Removed:
################################################################################
diff --git a/flang/include/flang/Tools/CLOptions.inc b/flang/include/flang/Tools/CLOptions.inc
index df396e04b2a76..bb4347d0e82c8 100644
--- a/flang/include/flang/Tools/CLOptions.inc
+++ b/flang/include/flang/Tools/CLOptions.inc
@@ -144,7 +144,7 @@ namespace fir {
static void addCanonicalizerPassWithoutRegionSimplification(
mlir::OpPassManager &pm) {
mlir::GreedyRewriteConfig config;
- config.enableRegionSimplification = false;
+ config.enableRegionSimplification = mlir::GreedySimplifyRegionLevel::Disabled;
pm.addPass(mlir::createCanonicalizerPass(config));
}
@@ -260,7 +260,7 @@ inline void createDefaultFIROptimizerPassPipeline(
// simplify the IR
mlir::GreedyRewriteConfig config;
- config.enableRegionSimplification = false;
+ config.enableRegionSimplification = mlir::GreedySimplifyRegionLevel::Disabled;
pm.addPass(mlir::createCSEPass());
fir::addAVC(pm, pc.OptLevel);
addNestedPassToAllTopLevelOperations(pm, fir::createCharacterConversion);
diff --git a/flang/lib/Optimizer/HLFIR/Transforms/InlineElementals.cpp b/flang/lib/Optimizer/HLFIR/Transforms/InlineElementals.cpp
index 6c8e3e1193747..4b0b8594c3ccf 100644
--- a/flang/lib/Optimizer/HLFIR/Transforms/InlineElementals.cpp
+++ b/flang/lib/Optimizer/HLFIR/Transforms/InlineElementals.cpp
@@ -119,7 +119,8 @@ class InlineElementalsPass
mlir::GreedyRewriteConfig config;
// Prevent the pattern driver from merging blocks.
- config.enableRegionSimplification = false;
+ config.enableRegionSimplification =
+ mlir::GreedySimplifyRegionLevel::Disabled;
mlir::RewritePatternSet patterns(context);
patterns.insert<InlineElementalConversion>(context);
diff --git a/flang/lib/Optimizer/HLFIR/Transforms/LowerHLFIRIntrinsics.cpp b/flang/lib/Optimizer/HLFIR/Transforms/LowerHLFIRIntrinsics.cpp
index 707c0feffbb36..0347bec3a9a2a 100644
--- a/flang/lib/Optimizer/HLFIR/Transforms/LowerHLFIRIntrinsics.cpp
+++ b/flang/lib/Optimizer/HLFIR/Transforms/LowerHLFIRIntrinsics.cpp
@@ -486,7 +486,8 @@ class LowerHLFIRIntrinsics
// Pattern rewriting only requires that the resulting IR is still valid
mlir::GreedyRewriteConfig config;
// Prevent the pattern driver from merging blocks
- config.enableRegionSimplification = false;
+ config.enableRegionSimplification =
+ mlir::GreedySimplifyRegionLevel::Disabled;
if (mlir::failed(mlir::applyPatternsAndFoldGreedily(
module, std::move(patterns), config))) {
diff --git a/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp b/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp
index 3c8424ca564e2..1dfa2f9d093bb 100644
--- a/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp
+++ b/flang/lib/Optimizer/HLFIR/Transforms/OptimizedBufferization.cpp
@@ -1042,7 +1042,8 @@ class OptimizedBufferizationPass
mlir::GreedyRewriteConfig config;
// Prevent the pattern driver from merging blocks
- config.enableRegionSimplification = false;
+ config.enableRegionSimplification =
+ mlir::GreedySimplifyRegionLevel::Disabled;
mlir::RewritePatternSet patterns(context);
// TODO: right now the patterns are non-conflicting,
diff --git a/flang/lib/Optimizer/Transforms/AssumedRankOpConversion.cpp b/flang/lib/Optimizer/Transforms/AssumedRankOpConversion.cpp
index cb6d6a73eace2..5dfc5009d2355 100644
--- a/flang/lib/Optimizer/Transforms/AssumedRankOpConversion.cpp
+++ b/flang/lib/Optimizer/Transforms/AssumedRankOpConversion.cpp
@@ -152,7 +152,8 @@ class AssumedRankOpConversion
patterns.insert<ReboxAssumedRankConv>(context, &symbolTable, kindMap);
patterns.insert<IsAssumedSizeConv>(context, &symbolTable, kindMap);
mlir::GreedyRewriteConfig config;
- config.enableRegionSimplification = false;
+ config.enableRegionSimplification =
+ mlir::GreedySimplifyRegionLevel::Disabled;
(void)applyPatternsAndFoldGreedily(mod, std::move(patterns), config);
}
};
diff --git a/flang/lib/Optimizer/Transforms/StackArrays.cpp b/flang/lib/Optimizer/Transforms/StackArrays.cpp
index 16bbb1c356460..7157b55b47c93 100644
--- a/flang/lib/Optimizer/Transforms/StackArrays.cpp
+++ b/flang/lib/Optimizer/Transforms/StackArrays.cpp
@@ -767,7 +767,7 @@ void StackArraysPass::runOnFunc(mlir::Operation *func) {
mlir::RewritePatternSet patterns(&context);
mlir::GreedyRewriteConfig config;
// prevent the pattern driver form merging blocks
- config.enableRegionSimplification = false;
+ config.enableRegionSimplification = mlir::GreedySimplifyRegionLevel::Disabled;
patterns.insert<AllocMemConversion>(&context, *candidateOps);
if (mlir::failed(mlir::applyOpPatternsAndFold(opsToConvert,
diff --git a/mlir/include/mlir/Transforms/GreedyPatternRewriteDriver.h b/mlir/include/mlir/Transforms/GreedyPatternRewriteDriver.h
index 763146aac15b9..eaff85804f6b3 100644
--- a/mlir/include/mlir/Transforms/GreedyPatternRewriteDriver.h
+++ b/mlir/include/mlir/Transforms/GreedyPatternRewriteDriver.h
@@ -29,6 +29,16 @@ enum class GreedyRewriteStrictness {
ExistingOps
};
+enum class GreedySimplifyRegionLevel {
+ /// Disable region control-flow simplification.
+ Disabled,
+ /// Run the normal simplification (e.g. dead args elimination).
+ Normal,
+ /// Run extra simplificiations (e.g. block merging), these can be
+ /// more costly or have some tradeoffs associated.
+ Aggressive
+};
+
/// This class allows control over how the GreedyPatternRewriteDriver works.
class GreedyRewriteConfig {
public:
@@ -45,7 +55,8 @@ class GreedyRewriteConfig {
/// patterns.
///
/// Note: Only applicable when simplifying entire regions.
- bool enableRegionSimplification = true;
+ GreedySimplifyRegionLevel enableRegionSimplification =
+ GreedySimplifyRegionLevel::Aggressive;
/// This specifies the maximum number of times the rewriter will iterate
/// between applying patterns and simplifying regions. Use `kNoLimit` to
diff --git a/mlir/include/mlir/Transforms/Passes.h b/mlir/include/mlir/Transforms/Passes.h
index 58bd61b2ae8b8..8e4a43c3f2458 100644
--- a/mlir/include/mlir/Transforms/Passes.h
+++ b/mlir/include/mlir/Transforms/Passes.h
@@ -15,6 +15,7 @@
#define MLIR_TRANSFORMS_PASSES_H
#include "mlir/Pass/Pass.h"
+#include "mlir/Transforms/GreedyPatternRewriteDriver.h"
#include "mlir/Transforms/LocationSnapshot.h"
#include "mlir/Transforms/ViewOpGraph.h"
#include "llvm/Support/Debug.h"
diff --git a/mlir/include/mlir/Transforms/Passes.td b/mlir/include/mlir/Transforms/Passes.td
index 1b40a87c63f27..000d9f697618e 100644
--- a/mlir/include/mlir/Transforms/Passes.td
+++ b/mlir/include/mlir/Transforms/Passes.td
@@ -32,9 +32,17 @@ def Canonicalizer : Pass<"canonicalize"> {
Option<"topDownProcessingEnabled", "top-down", "bool",
/*default=*/"true",
"Seed the worklist in general top-down order">,
- Option<"enableRegionSimplification", "region-simplify", "bool",
- /*default=*/"true",
- "Perform control flow optimizations to the region tree">,
+ Option<"enableRegionSimplification", "region-simplify", "mlir::GreedySimplifyRegionLevel",
+ /*default=*/"mlir::GreedySimplifyRegionLevel::Normal",
+ "Perform control flow optimizations to the region tree",
+ [{::llvm::cl::values(
+ clEnumValN(mlir::GreedySimplifyRegionLevel::Disabled, "disabled",
+ "Don't run any control-flow simplification."),
+ clEnumValN(mlir::GreedySimplifyRegionLevel::Normal, "normal",
+ "Perform simple control-flow simplifications (e.g. dead args elimination)."),
+ clEnumValN(mlir::GreedySimplifyRegionLevel::Aggressive, "aggressive",
+ "Perform aggressive control-flow simplification (e.g. block merging).")
+ )}]>,
Option<"maxIterations", "max-iterations", "int64_t",
/*default=*/"10",
"Max. iterations between applying patterns / simplifying regions">,
diff --git a/mlir/include/mlir/Transforms/RegionUtils.h b/mlir/include/mlir/Transforms/RegionUtils.h
index 06eebff201d1b..5c57dd5b7532a 100644
--- a/mlir/include/mlir/Transforms/RegionUtils.h
+++ b/mlir/include/mlir/Transforms/RegionUtils.h
@@ -74,8 +74,11 @@ SmallVector<Value> makeRegionIsolatedFromAbove(
/// elimination, as well as some other DCE. This function returns success if any
/// of the regions were simplified, failure otherwise. The provided rewriter is
/// used to notify callers of operation and block deletion.
+/// Structurally similar blocks will be merged if the `mergeBlock` argument is
+/// true. Note this can lead to merged blocks with extra arguments.
LogicalResult simplifyRegions(RewriterBase &rewriter,
- MutableArrayRef<Region> regions);
+ MutableArrayRef<Region> regions,
+ bool mergeBlocks = true);
/// Erase the unreachable blocks within the provided regions. Returns success
/// if any blocks were erased, failure otherwise.
diff --git a/mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp b/mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp
index 597cb29ce911b..e0d0acd122e26 100644
--- a/mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp
+++ b/mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp
@@ -875,8 +875,13 @@ LogicalResult RegionPatternRewriteDriver::simplify(bool *changed) && {
// After applying patterns, make sure that the CFG of each of the
// regions is kept up to date.
- if (config.enableRegionSimplification)
- continueRewrites |= succeeded(simplifyRegions(rewriter, region));
+ if (config.enableRegionSimplification !=
+ GreedySimplifyRegionLevel::Disabled) {
+ continueRewrites |= succeeded(simplifyRegions(
+ rewriter, region,
+ /*mergeBlocks=*/config.enableRegionSimplification ==
+ GreedySimplifyRegionLevel::Aggressive));
+ }
},
{®ion}, iteration);
} while (continueRewrites);
diff --git a/mlir/lib/Transforms/Utils/RegionUtils.cpp b/mlir/lib/Transforms/Utils/RegionUtils.cpp
index b5e641d39fc0a..4c0f15bafbaba 100644
--- a/mlir/lib/Transforms/Utils/RegionUtils.cpp
+++ b/mlir/lib/Transforms/Utils/RegionUtils.cpp
@@ -827,11 +827,13 @@ static LogicalResult mergeIdenticalBlocks(RewriterBase &rewriter,
/// elimination, as well as some other DCE. This function returns success if any
/// of the regions were simplified, failure otherwise.
LogicalResult mlir::simplifyRegions(RewriterBase &rewriter,
- MutableArrayRef<Region> regions) {
+ MutableArrayRef<Region> regions,
+ bool mergeBlocks) {
bool eliminatedBlocks = succeeded(eraseUnreachableBlocks(rewriter, regions));
bool eliminatedOpsOrArgs = succeeded(runRegionDCE(rewriter, regions));
- bool mergedIdenticalBlocks =
- succeeded(mergeIdenticalBlocks(rewriter, regions));
+ bool mergedIdenticalBlocks = false;
+ if (mergeBlocks)
+ mergedIdenticalBlocks = succeeded(mergeIdenticalBlocks(rewriter, regions));
return success(eliminatedBlocks || eliminatedOpsOrArgs ||
mergedIdenticalBlocks);
}
diff --git a/mlir/test/Dialect/SPIRV/Transforms/canonicalize.mlir b/mlir/test/Dialect/SPIRV/Transforms/canonicalize.mlir
index 977d31a6bfe54..d07389d6822ce 100644
--- a/mlir/test/Dialect/SPIRV/Transforms/canonicalize.mlir
+++ b/mlir/test/Dialect/SPIRV/Transforms/canonicalize.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt %s -split-input-file -pass-pipeline='builtin.module(func.func(canonicalize{test-convergence}))' | FileCheck %s
+// RUN: mlir-opt %s -split-input-file -pass-pipeline='builtin.module(func.func(canonicalize{test-convergence region-simplify=aggressive}))' | FileCheck %s
//===----------------------------------------------------------------------===//
// spirv.AccessChain
diff --git a/mlir/test/Pass/run-reproducer.mlir b/mlir/test/Pass/run-reproducer.mlir
index 57a58dbaa5b96..bf3ab2dae2ff8 100644
--- a/mlir/test/Pass/run-reproducer.mlir
+++ b/mlir/test/Pass/run-reproducer.mlir
@@ -14,8 +14,8 @@ func.func @bar() {
external_resources: {
mlir_reproducer: {
verify_each: true,
- // CHECK: builtin.module(func.func(cse,canonicalize{ max-iterations=1 max-num-rewrites=-1 region-simplify=false test-convergence=false top-down=false}))
- pipeline: "builtin.module(func.func(cse,canonicalize{max-iterations=1 max-num-rewrites=-1 region-simplify=false top-down=false}))",
+ // CHECK: builtin.module(func.func(cse,canonicalize{ max-iterations=1 max-num-rewrites=-1 region-simplify=normal test-convergence=false top-down=false}))
+ pipeline: "builtin.module(func.func(cse,canonicalize{max-iterations=1 max-num-rewrites=-1 region-simplify=normal top-down=false}))",
disable_threading: true
}
}
diff --git a/mlir/test/Transforms/canonicalize-block-merge.mlir b/mlir/test/Transforms/canonicalize-block-merge.mlir
index bf44973ab646c..3b8b1fce0575a 100644
--- a/mlir/test/Transforms/canonicalize-block-merge.mlir
+++ b/mlir/test/Transforms/canonicalize-block-merge.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt -allow-unregistered-dialect %s -pass-pipeline='builtin.module(func.func(canonicalize))' -split-input-file | FileCheck %s
+// RUN: mlir-opt -allow-unregistered-dialect %s -pass-pipeline='builtin.module(func.func(canonicalize{region-simplify=aggressive}))' -split-input-file | FileCheck %s
// Check the simple case of single operation blocks with a return.
@@ -275,3 +275,18 @@ func.func @mismatch_dominance() -> i32 {
^bb4(%3: i32):
return %3 : i32
}
+
+// CHECK-LABEL: func @dead_dealloc_fold_multi_use
+func.func @dead_dealloc_fold_multi_use(%cond : i1) {
+ // CHECK-NEXT: return
+ %a = memref.alloc() : memref<4xf32>
+ cf.cond_br %cond, ^bb1, ^bb2
+
+^bb1:
+ memref.dealloc %a: memref<4xf32>
+ return
+
+^bb2:
+ memref.dealloc %a: memref<4xf32>
+ return
+}
diff --git a/mlir/test/Transforms/canonicalize-dce.mlir b/mlir/test/Transforms/canonicalize-dce.mlir
index 3048a7fed636b..ac034d567a26a 100644
--- a/mlir/test/Transforms/canonicalize-dce.mlir
+++ b/mlir/test/Transforms/canonicalize-dce.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt -allow-unregistered-dialect %s -split-input-file -pass-pipeline='builtin.module(func.func(canonicalize))' | FileCheck %s
+// RUN: mlir-opt -allow-unregistered-dialect %s -split-input-file -pass-pipeline='builtin.module(func.func(canonicalize{region-simplify=aggressive}))' | FileCheck %s
// Test case: Simple case of deleting a dead pure op.
diff --git a/mlir/test/Transforms/canonicalize.mlir b/mlir/test/Transforms/canonicalize.mlir
index b34b87610a0e4..5e0d78fafb25c 100644
--- a/mlir/test/Transforms/canonicalize.mlir
+++ b/mlir/test/Transforms/canonicalize.mlir
@@ -387,16 +387,21 @@ func.func @dead_dealloc_fold() {
// CHECK-LABEL: func @dead_dealloc_fold_multi_use
func.func @dead_dealloc_fold_multi_use(%cond : i1) {
- // CHECK-NEXT: return
+ // CHECK-NOT: alloc
%a = memref.alloc() : memref<4xf32>
+ // CHECK: cond_br
cf.cond_br %cond, ^bb1, ^bb2
^bb1:
+ // CHECK-NOT: alloc
memref.dealloc %a: memref<4xf32>
+ // CHECK: return
return
^bb2:
+ // CHECK-NOT: alloc
memref.dealloc %a: memref<4xf32>
+ // CHECK: return
return
}
diff --git a/mlir/test/Transforms/test-canonicalize.mlir b/mlir/test/Transforms/test-canonicalize.mlir
index 4f0095ed7e8cf..0fc822b0a23ae 100644
--- a/mlir/test/Transforms/test-canonicalize.mlir
+++ b/mlir/test/Transforms/test-canonicalize.mlir
@@ -1,5 +1,5 @@
// RUN: mlir-opt %s -pass-pipeline='builtin.module(func.func(canonicalize))' | FileCheck %s
-// RUN: mlir-opt %s -pass-pipeline='builtin.module(func.func(canonicalize{region-simplify=false}))' | FileCheck %s --check-prefixes=CHECK,NO-RS
+// RUN: mlir-opt %s -pass-pipeline='builtin.module(func.func(canonicalize{region-simplify=disabled}))' | FileCheck %s --check-prefixes=CHECK,NO-RS
// CHECK-LABEL: func @remove_op_with_inner_ops_pattern
func.func @remove_op_with_inner_ops_pattern() {
More information about the Mlir-commits
mailing list