diff options
author | Matthias Springer <me@m-sp.org> | 2024-01-15 17:46:54 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2024-01-15 17:46:54 +0100 |
commit | c1730f42219365f5105148870422592c25083104 (patch) | |
tree | e43aa403215a08550bbbd3dcfb2d99dd127a1cec | |
parent | fcfe1b648219f40514b8934bc32543b8d739509d (diff) | |
download | llvm-c1730f42219365f5105148870422592c25083104.zip llvm-c1730f42219365f5105148870422592c25083104.tar.gz llvm-c1730f42219365f5105148870422592c25083104.tar.bz2 |
[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`.
-rw-r--r-- | mlir/lib/Dialect/SCF/IR/SCF.cpp | 4 | ||||
-rw-r--r-- | mlir/test/Dialect/SCF/for-loop-peeling.mlir | 8 | ||||
-rw-r--r-- | mlir/test/Dialect/SCF/invalid.mlir | 12 |
3 files changed, 6 insertions, 18 deletions
diff --git a/mlir/lib/Dialect/SCF/IR/SCF.cpp b/mlir/lib/Dialect/SCF/IR/SCF.cpp index 5570c2e..cdc0b6f 100644 --- a/mlir/lib/Dialect/SCF/IR/SCF.cpp +++ b/mlir/lib/Dialect/SCF/IR/SCF.cpp @@ -332,10 +332,6 @@ void ForOp::build(OpBuilder &builder, OperationState &result, Value lb, } LogicalResult ForOp::verify() { - IntegerAttr step; - if (matchPattern(getStep(), m_Constant(&step)) && step.getInt() <= 0) - return emitOpError("constant step operand must be positive"); - // Check that the number of init args and op results is the same. if (getInitArgs().size() != getNumResults()) return emitOpError( diff --git a/mlir/test/Dialect/SCF/for-loop-peeling.mlir b/mlir/test/Dialect/SCF/for-loop-peeling.mlir index aa8fd7e..f59b796 100644 --- a/mlir/test/Dialect/SCF/for-loop-peeling.mlir +++ b/mlir/test/Dialect/SCF/for-loop-peeling.mlir @@ -292,15 +292,19 @@ func.func @regression(%arg0: memref<i64>, %arg1: index) { // ----- -// Check that this doesn't crash but trigger the verifier. +// Regression test: Make sure that we do not crash. + +// CHECK-LABEL: func @zero_step( +// CHECK: scf.for +// CHECK: scf.for func.func @zero_step(%arg0: memref<i64>) { %c0 = arith.constant 0 : index %c1 = arith.constant 1 : index %foldto0 = arith.subi %c1, %c1 : index - // expected-error @+1 {{'scf.for' op constant step operand must be positive}} scf.for %arg2 = %c0 to %c1 step %foldto0 { %2 = arith.index_cast %arg2 : index to i64 memref.store %2, %arg0[] : memref<i64> } return } + diff --git a/mlir/test/Dialect/SCF/invalid.mlir b/mlir/test/Dialect/SCF/invalid.mlir index fac9d82..337eb9e 100644 --- a/mlir/test/Dialect/SCF/invalid.mlir +++ b/mlir/test/Dialect/SCF/invalid.mlir @@ -32,18 +32,6 @@ func.func @loop_for_mismatch(%arg0: i32, %arg1: index) { // ----- -func.func @loop_for_step_positive(%arg0: index) { - // expected-error@+2 {{constant step operand must be positive}} - %c0 = arith.constant 0 : index - "scf.for"(%arg0, %arg0, %c0) ({ - ^bb0(%arg1: index): - scf.yield - }) : (index, index, index) -> () - return -} - -// ----- - func.func @loop_for_one_region(%arg0: index) { // expected-error@+1 {{requires one region}} "scf.for"(%arg0, %arg0, %arg0) ( |