Skip to content

Commit 87e6fd1

Browse files
authored
[MLIR] Erase unreachable blocks before applying patterns in the greedy rewriter (#153957)
Operations like: %add = arith.addi %add, %add : i64 are legal in unreachable code. Unfortunately many patterns would be unsafe to apply on such IR and can lead to crashes or infinite loops. To avoid this we can remove unreachable blocks before attempting to apply patterns. We may have to do this also whenever the CFG is changed by a pattern, it is left up for future work right now. Fixes #153732
1 parent 7ee6cf0 commit 87e6fd1

File tree

3 files changed

+34
-10
lines changed

3 files changed

+34
-10
lines changed

mlir/lib/Transforms/Utils/GreedyPatternRewriteDriver.cpp

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -871,7 +871,18 @@ LogicalResult RegionPatternRewriteDriver::simplify(bool *changed) && {
871871

872872
ctx->executeAction<GreedyPatternRewriteIteration>(
873873
[&] {
874-
continueRewrites = processWorklist();
874+
continueRewrites = false;
875+
876+
// Erase unreachable blocks
877+
// Operations like:
878+
// %add = arith.addi %add, %add : i64
879+
// are legal in unreachable code. Unfortunately many patterns would be
880+
// unsafe to apply on such IR and can lead to crashes or infinite
881+
// loops.
882+
continueRewrites |=
883+
succeeded(eraseUnreachableBlocks(rewriter, region));
884+
885+
continueRewrites |= processWorklist();
875886

876887
// After applying patterns, make sure that the CFG of each of the
877888
// regions is kept up to date.

mlir/test/Dialect/Arith/canonicalize.mlir

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3363,3 +3363,18 @@ func.func @bf16_fma(%arg0: vector<32x32x32xbf16>, %arg1: vector<32x32x32xbf16>,
33633363
}
33643364
}
33653365
#-}
3366+
3367+
// CHECK-LABEL: func @unreachable()
3368+
// CHECK-NEXT: return
3369+
// CHECK-NOT: arith
3370+
func.func @unreachable() {
3371+
return
3372+
^unreachable:
3373+
%c1_i64 = arith.constant 1 : i64
3374+
// This self referencing operation is legal in an unreachable block.
3375+
// Many patterns are unsafe with respect to this kind of situation,
3376+
// check that we don't infinite loop here.
3377+
%add = arith.addi %add, %c1_i64 : i64
3378+
cf.br ^unreachable
3379+
}
3380+

mlir/test/Transforms/test-canonicalize.mlir

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: mlir-opt %s -pass-pipeline='builtin.module(func.func(canonicalize))' | FileCheck %s
1+
// RUN: mlir-opt %s -pass-pipeline='builtin.module(func.func(canonicalize))' | FileCheck %s --check-prefixes=CHECK,RS
22
// RUN: mlir-opt %s -pass-pipeline='builtin.module(func.func(canonicalize{region-simplify=disabled}))' | FileCheck %s --check-prefixes=CHECK,NO-RS
33

44
// CHECK-LABEL: func @remove_op_with_inner_ops_pattern
@@ -80,12 +80,10 @@ func.func @test_dialect_canonicalizer() -> (i32) {
8080

8181
// Check that the option to control region simplification actually works
8282
// CHECK-LABEL: test_region_simplify
83-
func.func @test_region_simplify() {
84-
// CHECK-NEXT: return
85-
// NO-RS-NEXT: ^bb1
86-
// NO-RS-NEXT: return
87-
// CHECK-NEXT: }
88-
return
89-
^bb1:
90-
return
83+
func.func @test_region_simplify(%input1 : i32, %cond : i1) -> i32 {
84+
// RS-NEXT: "test.br"(%arg0)[^bb1] : (i32) -> ()
85+
// NO-RS-NEXT: "test.br"(%arg0, %arg0)[^bb1] : (i32, i32) -> ()
86+
"test.br"(%input1, %input1)[^bb1] : (i32, i32) -> ()
87+
^bb1(%used_arg : i32, %unused_arg : i32):
88+
return %used_arg : i32
9189
}

0 commit comments

Comments
 (0)