aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorMatthias Springer <me@m-sp.org>2024-01-15 17:46:54 +0100
committerGitHub <noreply@github.com>2024-01-15 17:46:54 +0100
commitc1730f42219365f5105148870422592c25083104 (patch)
treee43aa403215a08550bbbd3dcfb2d99dd127a1cec
parentfcfe1b648219f40514b8934bc32543b8d739509d (diff)
downloadllvm-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.cpp4
-rw-r--r--mlir/test/Dialect/SCF/for-loop-peeling.mlir8
-rw-r--r--mlir/test/Dialect/SCF/invalid.mlir12
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) (