From e1f17d587d0802a9f870b6d33dcb65a462606508 Mon Sep 17 00:00:00 2001 From: Kai Sasaki Date: Mon, 2 Dec 2024 14:38:57 +0900 Subject: [PATCH 1/4] [mlir][transform] Guard parametric loop tiling pass from no option --- .../Transforms/invalid-outer-loop-size.mlir | 19 +++++++++++++++++++ .../Dialect/SCF/TestLoopParametricTiling.cpp | 3 +++ 2 files changed, 22 insertions(+) create mode 100644 mlir/test/Transforms/invalid-outer-loop-size.mlir diff --git a/mlir/test/Transforms/invalid-outer-loop-size.mlir b/mlir/test/Transforms/invalid-outer-loop-size.mlir new file mode 100644 index 0000000000000..42e5848b9704e --- /dev/null +++ b/mlir/test/Transforms/invalid-outer-loop-size.mlir @@ -0,0 +1,19 @@ +// RUN: mlir-opt -test-extract-fixed-outer-loops %s | FileCheck %s + +// COMMON-LABEL: @no_crash +func.func @no_crash(%arg0: memref) { + // CHECK: %[[c2:.*]] = arith.constant 2 : index + %c2 = arith.constant 2 : index + // CHECK: %[[c44:.*]] = arith.constant 44 : index + %c44 = arith.constant 44 : index + // CHECK: %[[c1:.*]] = arith.constant 1 : index + %c1 = arith.constant 1 : index + // CHECK: scf.for %[[i:.*]] = %[[c2]] to %[[c44]] step %[[c1]] + scf.for %i = %c2 to %c44 step %c1 { + // CHECK: scf.for %[[j:.*]] = %[[c1]] to %[[c44]] step %[[c2]] + scf.for %j = %c1 to %c44 step %c2 { + memref.load %arg0[%i, %j]: memref + } + } + return +} diff --git a/mlir/test/lib/Dialect/SCF/TestLoopParametricTiling.cpp b/mlir/test/lib/Dialect/SCF/TestLoopParametricTiling.cpp index 1f33a04b0668a..f90371a27e739 100644 --- a/mlir/test/lib/Dialect/SCF/TestLoopParametricTiling.cpp +++ b/mlir/test/lib/Dialect/SCF/TestLoopParametricTiling.cpp @@ -40,6 +40,9 @@ class SimpleParametricLoopTilingPass } void runOnOperation() override { + // If no outer loop iteration is given, ignore the pass. + if (sizes.empty()) + return; getOperation()->walk([this](scf::ForOp op) { // Ignore nested loops. if (op->getParentRegion()->getParentOfType()) From a5ec8af4b52a325c17e627c44e0a2cceeefdb807 Mon Sep 17 00:00:00 2001 From: Kai Sasaki Date: Wed, 4 Dec 2024 09:35:44 +0900 Subject: [PATCH 2/4] Throw the error in case of empty outer loop sizes --- mlir/test/Transforms/invalid-outer-loop-size.mlir | 10 +++------- mlir/test/lib/Dialect/SCF/TestLoopParametricTiling.cpp | 6 ++++-- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/mlir/test/Transforms/invalid-outer-loop-size.mlir b/mlir/test/Transforms/invalid-outer-loop-size.mlir index 42e5848b9704e..77e3df838588f 100644 --- a/mlir/test/Transforms/invalid-outer-loop-size.mlir +++ b/mlir/test/Transforms/invalid-outer-loop-size.mlir @@ -1,16 +1,12 @@ -// RUN: mlir-opt -test-extract-fixed-outer-loops %s | FileCheck %s +// RUN: mlir-opt -test-extract-fixed-outer-loops %s -verify-diagnostics -// COMMON-LABEL: @no_crash +// CHECK-LABEL: @no_crash func.func @no_crash(%arg0: memref) { - // CHECK: %[[c2:.*]] = arith.constant 2 : index + // expected-error@-5 {{expect non-empty outer loop sizes}} %c2 = arith.constant 2 : index - // CHECK: %[[c44:.*]] = arith.constant 44 : index %c44 = arith.constant 44 : index - // CHECK: %[[c1:.*]] = arith.constant 1 : index %c1 = arith.constant 1 : index - // CHECK: scf.for %[[i:.*]] = %[[c2]] to %[[c44]] step %[[c1]] scf.for %i = %c2 to %c44 step %c1 { - // CHECK: scf.for %[[j:.*]] = %[[c1]] to %[[c44]] step %[[c2]] scf.for %j = %c1 to %c44 step %c2 { memref.load %arg0[%i, %j]: memref } diff --git a/mlir/test/lib/Dialect/SCF/TestLoopParametricTiling.cpp b/mlir/test/lib/Dialect/SCF/TestLoopParametricTiling.cpp index f90371a27e739..01617ef7f8252 100644 --- a/mlir/test/lib/Dialect/SCF/TestLoopParametricTiling.cpp +++ b/mlir/test/lib/Dialect/SCF/TestLoopParametricTiling.cpp @@ -40,9 +40,11 @@ class SimpleParametricLoopTilingPass } void runOnOperation() override { - // If no outer loop iteration is given, ignore the pass. - if (sizes.empty()) + if (sizes.empty()) { + getOperation()->emitError("expect non-empty outer loop sizes"); + signalPassFailure(); return; + } getOperation()->walk([this](scf::ForOp op) { // Ignore nested loops. if (op->getParentRegion()->getParentOfType()) From 13e6eb69785b2923bc66a8e244c9438929040aa3 Mon Sep 17 00:00:00 2001 From: Kai Sasaki Date: Wed, 4 Dec 2024 11:58:15 +0900 Subject: [PATCH 3/4] Update mlir/test/lib/Dialect/SCF/TestLoopParametricTiling.cpp Co-authored-by: Mehdi Amini --- mlir/test/lib/Dialect/SCF/TestLoopParametricTiling.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mlir/test/lib/Dialect/SCF/TestLoopParametricTiling.cpp b/mlir/test/lib/Dialect/SCF/TestLoopParametricTiling.cpp index 01617ef7f8252..510fcf7c925c4 100644 --- a/mlir/test/lib/Dialect/SCF/TestLoopParametricTiling.cpp +++ b/mlir/test/lib/Dialect/SCF/TestLoopParametricTiling.cpp @@ -41,7 +41,7 @@ class SimpleParametricLoopTilingPass void runOnOperation() override { if (sizes.empty()) { - getOperation()->emitError("expect non-empty outer loop sizes"); + emitError(UnknownLoc::get(getContext()), "missing `" + getArgument() + "` pass-option for outer loop sizes"); signalPassFailure(); return; } From 5cc6df08756b8bb5703c44e6df140b35d63d843e Mon Sep 17 00:00:00 2001 From: Kai Sasaki Date: Wed, 4 Dec 2024 17:00:24 +0900 Subject: [PATCH 4/4] Refine error message --- mlir/test/Transforms/invalid-outer-loop-size.mlir | 13 ++----------- .../lib/Dialect/SCF/TestLoopParametricTiling.cpp | 4 +++- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/mlir/test/Transforms/invalid-outer-loop-size.mlir b/mlir/test/Transforms/invalid-outer-loop-size.mlir index 77e3df838588f..7c1e92a4b7ba7 100644 --- a/mlir/test/Transforms/invalid-outer-loop-size.mlir +++ b/mlir/test/Transforms/invalid-outer-loop-size.mlir @@ -1,15 +1,6 @@ -// RUN: mlir-opt -test-extract-fixed-outer-loops %s -verify-diagnostics +// RUN: not mlir-opt -test-extract-fixed-outer-loops %s 2>&1 | FileCheck %s -// CHECK-LABEL: @no_crash func.func @no_crash(%arg0: memref) { - // expected-error@-5 {{expect non-empty outer loop sizes}} - %c2 = arith.constant 2 : index - %c44 = arith.constant 44 : index - %c1 = arith.constant 1 : index - scf.for %i = %c2 to %c44 step %c1 { - scf.for %j = %c1 to %c44 step %c2 { - memref.load %arg0[%i, %j]: memref - } - } + // CHECK: error: missing `test-outer-loop-sizes` pass-option for outer loop sizes return } diff --git a/mlir/test/lib/Dialect/SCF/TestLoopParametricTiling.cpp b/mlir/test/lib/Dialect/SCF/TestLoopParametricTiling.cpp index 510fcf7c925c4..e7e2f70ba1197 100644 --- a/mlir/test/lib/Dialect/SCF/TestLoopParametricTiling.cpp +++ b/mlir/test/lib/Dialect/SCF/TestLoopParametricTiling.cpp @@ -41,7 +41,9 @@ class SimpleParametricLoopTilingPass void runOnOperation() override { if (sizes.empty()) { - emitError(UnknownLoc::get(getContext()), "missing `" + getArgument() + "` pass-option for outer loop sizes"); + emitError( + UnknownLoc::get(&getContext()), + "missing `test-outer-loop-sizes` pass-option for outer loop sizes"); signalPassFailure(); return; }