[Mlir-commits] [mlir] [MLIR][SCF] Verify number of operands in scf.parallel reduce terminator (PR #171450)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-scf

Author: Men-cotton (Men-cotton)

<details>
<summary>Changes</summary>

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
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 at +1 {{expects parent op 'scf.parallel'}}
   scf.reduce(%arg0 : f32) {

``````````

</details>


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