[MLIR][SCF] Verify number of regions in scf.reduce

[Men-cotton]

This patch adds ReduceOp::verifyRegions to ensure that the number of reduction regions equals the number of operands (getReductions().size() == getOperands().size()).

Additionally, ParallelOp::verify is updated to gracefully handle cases where the number of reduce operands differs from the initial values, preventing verification logic crashes and relying on ReduceOp to report structural inconsistencies.

Fixes: #118768

[llvmbot]

@llvm/pr-subscribers-mlir

@llvm/pr-subscribers-mlir-scf

Author: Men-cotton (Men-cotton)

Changes

The scf.parallel operation expects the number of operands in its scf.reduce terminator to match the number of initial values provided to the loop. Previously, this mismatch was not verified, leading to assertion failures in downstream passes like convert-scf-to-openmp.

This patch adds a verification check to ParallelOp::verify to ensure that reduceOp.getNumOperands() equals initValsSize, raising a descriptive error if they differ.

Fixes: #118768

---

Full diff: https://github.com/llvm/llvm-project/pull/171450.diff

2 Files Affected:

• (modified) mlir/lib/Dialect/SCF/IR/SCF.cpp (+5)
• (modified) mlir/test/Dialect/SCF/invalid.mlir (+14)

diff --git a/mlir/lib/Dialect/SCF/IR/SCF.cpp b/mlir/lib/Dialect/SCF/IR/SCF.cpp
index c75528a76c999..bb18b2a1e5abc 100644
--- a/mlir/lib/Dialect/SCF/IR/SCF.cpp
+++ b/mlir/lib/Dialect/SCF/IR/SCF.cpp
@@ -3152,6 +3152,11 @@ LogicalResult ParallelOp::verify() {
     return emitOpError() << "expects number of results: " << resultsSize
                          << " to be the same as number of initial values: "
                          << initValsSize;
+  if (reduceOp.getNumOperands() != initValsSize)
+    return emitOpError() << "expects number of operands in the terminator: "
+                         << reduceOp.getNumOperands()
+                         << " to be the same as number of initial values: "
+                         << initValsSize;
 
   // Check that the types of the results and reductions are the same.
   for (int64_t i = 0; i < static_cast<int64_t>(reductionsSize); ++i) {
diff --git a/mlir/test/Dialect/SCF/invalid.mlir b/mlir/test/Dialect/SCF/invalid.mlir
index 3f481ad5dbba7..e2edeb6805864 100644
--- a/mlir/test/Dialect/SCF/invalid.mlir
+++ b/mlir/test/Dialect/SCF/invalid.mlir
@@ -274,6 +274,20 @@ func.func @parallel_different_types_of_results_and_reduces(
 
 // -----
 
+// The scf.parallel operation requires the number of operands in the terminator
+// (scf.reduce) to match the number of initial values provided to the loop.
+func.func @invalid_parallel_reduce_operand_count() {
+  %c0 = arith.constant 0 : index
+  %c1 = arith.constant 1 : index
+  // expected-error @+1 {{expects number of operands in the terminator: 1 to be the same as number of initial values: 0}}
+  scf.parallel (%arg1) = (%c0) to (%c1) step (%c1) {
+    scf.reduce(%c1 : index)
+  }
+  return
+}
+
+// -----
+
 func.func @top_level_reduce(%arg0 : f32) {
   // expected-error@+1 {{expects parent op 'scf.parallel'}}
   scf.reduce(%arg0 : f32) {

[joker-eph]

Do we already verify that the scf.reduce has the right number of regions? If not, can you also add this?

[Men-cotton]

@joker-eph
Thanks for the review! Could you merge this?