Skip to content

Commit 041b0a8

Browse files
wang-y-zisaacw
andauthored
[MLIR][Operation] Fix isBeforeInBlock crash bug mentioned in #60909 (#101172)
# summary This MR fix `isBeforeInBlock` crash bug mentioned in #60909. Fixes #60909. # Trigger condition 1. A block only have one operation. 2. `block->isOpOrderValid()` is true, but `op->hasValidOrder()` is false. 3. call: `op->isBeforeInBlock(op)`, compared with op itself. Will crash on `assert(blockFront != blockBack && "expected more than one operation");` # Case study Simplified repro case in `mlir/test/Pass/scf2cf-print-liveness-crash.mlir` When put `-convert-scf-to-cf -test-print-liveness` together in one cmd line, the first pass will work normally and crash on the second pass. Details please refer #60909 # Solutions option1. in `isBeforeInBlock`, check if block only have one operation before step into `updateOrderIfNecessary`, if have only one, it must return false option2. in `isBeforeInBlock`, check if `this == other`, if true return false option3. fix `addNodeToList` logic I prefer option3: When a block contains only one operation and the user calls op->isBeforeInBlock(op), if block->isOpOrderValid() returns true, updateOrderIfNecessary is called. If op->hasValidOrder() is false, it will crash at the assertion assert(blockFront != blockBack && "expected more than one operation");. This behavior is abnormal and needs fixing. I discovered that after the first pass of `-convert-scf-to-cf`, there is a block with only one operation where the block order is valid but the operation order is invalid, leading to a crash when `-test-print-liveness` pass runs. --------- Co-authored-by: isaacw <[email protected]>
1 parent 8f31ee9 commit 041b0a8

File tree

2 files changed

+21
-1
lines changed

2 files changed

+21
-1
lines changed

mlir/lib/IR/Operation.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -406,7 +406,7 @@ void Operation::updateOrderIfNecessary() {
406406
assert(block && "expected valid parent");
407407

408408
// If the order is valid for this operation there is nothing to do.
409-
if (hasValidOrder())
409+
if (hasValidOrder() || llvm::hasSingleElement(*block))
410410
return;
411411
Operation *blockFront = &block->front();
412412
Operation *blockBack = &block->back();
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// RUN: mlir-opt %s -pass-pipeline="builtin.module(func.func(convert-scf-to-cf), func.func(test-print-liveness))"
2+
3+
module {
4+
func.func @for_if_for(%arg0: index, %arg1: index, %arg2: index, %arg3: i1) {
5+
%cst = arith.constant dense<0.000000e+00> : tensor<128x32xf16>
6+
%0 = scf.for %arg4 = %arg0 to %arg1 step %arg2 iter_args(%arg5 = %cst) -> (tensor<128x32xf16>) {
7+
%1 = scf.if %arg3 -> (tensor<128x32xf16>) {
8+
scf.yield %arg5 : tensor<128x32xf16>
9+
} else {
10+
%2 = scf.for %arg6 = %arg0 to %arg1 step %arg2 iter_args(%arg7 = %arg5) -> (tensor<128x32xf16>) {
11+
scf.yield %arg7 : tensor<128x32xf16>
12+
}
13+
scf.yield %2 : tensor<128x32xf16>
14+
}
15+
scf.yield %1 : tensor<128x32xf16>
16+
}
17+
18+
return
19+
}
20+
}

0 commit comments

Comments
 (0)