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

[Federico Bruzzone]

================
@@ -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
----------------
FedericoBruzzone wrote:

You might find interesting the MLIR [Testing Guide](https://mlir.llvm.org/getting_started/TestingGuide/). 

In particular, the [Test Formatting Best Practices](https://mlir.llvm.org/getting_started/TestingGuide/#test-formatting-best-practices) explicitly say: 
> This applies when modifying existing test files that follow a particular convention, as it likely fits the context.

In particular, genuinely speaking:
1. I don't see a motivation to touch the pre-existing RUN lines,
2. I'd remove the `RUN: split-file %s %t`,
3. The invalid case should not be included in this file (and I don't think we really need them)


Are you able to align the last commit with these suggestions? 🫶

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