[Mlir-commits] [mlir] [mlir][SCF] Avoid crash in parallel loop unrolling test pass (PR #222644)

[purnima shrivastava]

================
@@ -1,8 +1,19 @@
-// RUN: not mlir-opt %s -test-parallel-loop-unrolling -split-input-file 2>&1 | FileCheck %s --check-prefix=CHECK-MISSING-FACTORS
-// RUN: not mlir-opt %s -test-parallel-loop-unrolling='unroll-factors=1,0' -split-input-file 2>&1 | FileCheck %s --check-prefix=CHECK-ZERO-FACTOR
-// RUN: mlir-opt %s -test-parallel-loop-unrolling='unroll-factors=1,2' -split-input-file | FileCheck %s
-// RUN: mlir-opt %s -test-parallel-loop-unrolling='unroll-factors=1,2 loop-depth=1' -split-input-file | FileCheck %s --check-prefix CHECK-UNROLL-INNER
-// RUN: mlir-opt %s -test-parallel-loop-unrolling='unroll-factors=3,1' -split-input-file | FileCheck %s --check-prefix CHECK-UNROLL-BY-3
+// RUN: split-file %s %t
+// RUN: mlir-opt %t/missing-factors.mlir -test-parallel-loop-unrolling -verify-diagnostics
+// RUN: mlir-opt %t/zero-factor.mlir -test-parallel-loop-unrolling='unroll-factors=1,0' -verify-diagnostics
+// RUN: mlir-opt %t/valid.mlir -test-parallel-loop-unrolling='unroll-factors=1,2' -split-input-file | FileCheck %s
+// RUN: mlir-opt %t/valid.mlir -test-parallel-loop-unrolling='unroll-factors=1,2 loop-depth=1' -split-input-file | FileCheck %s --check-prefix CHECK-UNROLL-INNER
+// RUN: mlir-opt %t/valid.mlir -test-parallel-loop-unrolling='unroll-factors=3,1' -split-input-file | FileCheck %s --check-prefix CHECK-UNROLL-BY-3
+
+//--- missing-factors.mlir
+// expected-error at unknown {{missing `unroll-factors` pass option}}
+module {}
+
+//--- zero-factor.mlir
+// expected-error at unknown {{unroll factors must be non-zero}}
+module {}
+
+//--- valid.mlir
----------------
purnima-nlp wrote:

@FedericoBruzzone I checked a few existing MLIR tests, including `transform-ops-invalid.mlir`, `one-shot-bufferize-invalid.mlir`, `tosa-to-scf-invalid.mlir`, and `properties-invalid.mlir`, and I see that keeping invalid cases in a separate test file is already a common pattern.

So your suggestion definitely makes sense. Thanks for pointing me in the right direction, and sorry I didn’t think of checking the existing test structure first. I’ve aligned the changes accordingly. 🫶
please check @Men-cotton @matthias-springer 

https://github.com/llvm/llvm-project/pull/222644