Skip to content

Commit c1730f4

Browse files
[mlir][SCF] Do not verify step size of scf.for (#78141)
An op verifier should verify only local properties. This commit removes the verification of `scf.for` step sizes. (Verifiers can check attributes but should not follow SSA values.) This verification could reject IR that is actually valid, e.g.: ```mlir scf.if %always_false { // Branch is never entered. scf.for ... step %c0 { ... } } ``` This commit fixes `for-loop-peeling.mlir` when running with `MLIR_ENABLE_EXPENSIVE_PATTERN_API_CHECKS`: ``` within split at llvm-project/mlir/test/Dialect/SCF/for-loop-peeling.mlir:293 offset :9:3: note: see current operation: "scf.for"(%0, %3, %2) ({ ^bb0(%arg1: index): %4 = "arith.index_cast"(%arg1) : (index) -> i64 "memref.store"(%4, %arg0) : (i64, memref<i64>) -> () "scf.yield"() : () -> () }) {__peeled_loop__} : (index, index, index) -> () LLVM ERROR: IR failed to verify after folding ``` Note: `%2` is `arith.constant 0 : index`.
1 parent fcfe1b6 commit c1730f4

File tree

3 files changed

+6
-18
lines changed

3 files changed

+6
-18
lines changed

mlir/lib/Dialect/SCF/IR/SCF.cpp

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -332,10 +332,6 @@ void ForOp::build(OpBuilder &builder, OperationState &result, Value lb,
332332
}
333333

334334
LogicalResult ForOp::verify() {
335-
IntegerAttr step;
336-
if (matchPattern(getStep(), m_Constant(&step)) && step.getInt() <= 0)
337-
return emitOpError("constant step operand must be positive");
338-
339335
// Check that the number of init args and op results is the same.
340336
if (getInitArgs().size() != getNumResults())
341337
return emitOpError(

mlir/test/Dialect/SCF/for-loop-peeling.mlir

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -292,15 +292,19 @@ func.func @regression(%arg0: memref<i64>, %arg1: index) {
292292

293293
// -----
294294

295-
// Check that this doesn't crash but trigger the verifier.
295+
// Regression test: Make sure that we do not crash.
296+
297+
// CHECK-LABEL: func @zero_step(
298+
// CHECK: scf.for
299+
// CHECK: scf.for
296300
func.func @zero_step(%arg0: memref<i64>) {
297301
%c0 = arith.constant 0 : index
298302
%c1 = arith.constant 1 : index
299303
%foldto0 = arith.subi %c1, %c1 : index
300-
// expected-error @+1 {{'scf.for' op constant step operand must be positive}}
301304
scf.for %arg2 = %c0 to %c1 step %foldto0 {
302305
%2 = arith.index_cast %arg2 : index to i64
303306
memref.store %2, %arg0[] : memref<i64>
304307
}
305308
return
306309
}
310+

mlir/test/Dialect/SCF/invalid.mlir

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -32,18 +32,6 @@ func.func @loop_for_mismatch(%arg0: i32, %arg1: index) {
3232

3333
// -----
3434

35-
func.func @loop_for_step_positive(%arg0: index) {
36-
// expected-error@+2 {{constant step operand must be positive}}
37-
%c0 = arith.constant 0 : index
38-
"scf.for"(%arg0, %arg0, %c0) ({
39-
^bb0(%arg1: index):
40-
scf.yield
41-
}) : (index, index, index) -> ()
42-
return
43-
}
44-
45-
// -----
46-
4735
func.func @loop_for_one_region(%arg0: index) {
4836
// expected-error@+1 {{requires one region}}
4937
"scf.for"(%arg0, %arg0, %arg0) (

0 commit comments

Comments
 (0)