Skip to content

[mlir][SCF] Fix invalid IR in ParallelOpSingleOrZeroIterationDimsFolder pattern #74552

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

matthias-springer
Copy link
Member

ParallelOpSingleOrZeroIterationDimsFolder used to produce invalid IR:

within split at mlir/test/Dialect/SCF/canonicalize.mlir:1 offset :11:3: error: 'scf.parallel' op expects region #0 to have 0 or 1 blocks
  scf.parallel (%i0, %i1, %i2) = (%c0, %c3, %c7) to (%c1, %c6, %c10) step (%c1, %c2, %c3) {
  ^
within split at mlir/test/Dialect/SCF/canonicalize.mlir:1 offset :11:3: note: see current operation:
"scf.parallel"(%4, %5, %3) <{operandSegmentSizes = array<i32: 1, 1, 1, 0>}> ({
^bb0(%arg1: index):
  "memref.store"(%0, %arg0, %1, %arg1, %6) : (i32, memref<?x?x?xi32>, index, index, index) -> ()
  "scf.yield"() : () -> ()
^bb1(%8: index):  // no predecessors
  "scf.yield"() : () -> ()
}) : (index, index, index) -> ()

Together with #74551, this commit fixes mlir/test/Dialect/SCF/canonicalize.mlir when verifying the IR after each pattern application (#74270).

…der` pattern

`ParallelOpSingleOrZeroIterationDimsFolder` used to produce invalid IR:
```
within split at mlir/test/Dialect/SCF/canonicalize.mlir:1 offset :11:3: error: 'scf.parallel' op expects region #0 to have 0 or 1 blocks
  scf.parallel (%i0, %i1, %i2) = (%c0, %c3, %c7) to (%c1, %c6, %c10) step (%c1, %c2, %c3) {
  ^
within split at mlir/test/Dialect/SCF/canonicalize.mlir:1 offset :11:3: note: see current operation:
"scf.parallel"(%4, %5, %3) <{operandSegmentSizes = array<i32: 1, 1, 1, 0>}> ({
^bb0(%arg1: index):
  "memref.store"(%0, %arg0, %1, %arg1, %6) : (i32, memref<?x?x?xi32>, index, index, index) -> ()
  "scf.yield"() : () -> ()
^bb1(%8: index):  // no predecessors
  "scf.yield"() : () -> ()
}) : (index, index, index) -> ()
```

Together with llvm#74551, this commit fixes `mlir/test/Dialect/SCF/canonicalize.mlir` when verifying the IR after each pattern application (llvm#74270).
@llvmbot
Copy link
Member

llvmbot commented Dec 6, 2023

@llvm/pr-subscribers-mlir

Author: Matthias Springer (matthias-springer)

Changes

ParallelOpSingleOrZeroIterationDimsFolder used to produce invalid IR:

within split at mlir/test/Dialect/SCF/canonicalize.mlir:1 offset :11:3: error: 'scf.parallel' op expects region #<!-- -->0 to have 0 or 1 blocks
  scf.parallel (%i0, %i1, %i2) = (%c0, %c3, %c7) to (%c1, %c6, %c10) step (%c1, %c2, %c3) {
  ^
within split at mlir/test/Dialect/SCF/canonicalize.mlir:1 offset :11:3: note: see current operation:
"scf.parallel"(%4, %5, %3) &lt;{operandSegmentSizes = array&lt;i32: 1, 1, 1, 0&gt;}&gt; ({
^bb0(%arg1: index):
  "memref.store"(%0, %arg0, %1, %arg1, %6) : (i32, memref&lt;?x?x?xi32&gt;, index, index, index) -&gt; ()
  "scf.yield"() : () -&gt; ()
^bb1(%8: index):  // no predecessors
  "scf.yield"() : () -&gt; ()
}) : (index, index, index) -&gt; ()

Together with #74551, this commit fixes mlir/test/Dialect/SCF/canonicalize.mlir when verifying the IR after each pattern application (#74270).


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

1 Files Affected:

  • (modified) mlir/lib/Dialect/SCF/IR/SCF.cpp (+2)
diff --git a/mlir/lib/Dialect/SCF/IR/SCF.cpp b/mlir/lib/Dialect/SCF/IR/SCF.cpp
index 3b55704c4ea07..2e7382160f8eb 100644
--- a/mlir/lib/Dialect/SCF/IR/SCF.cpp
+++ b/mlir/lib/Dialect/SCF/IR/SCF.cpp
@@ -3040,6 +3040,8 @@ struct ParallelOpSingleOrZeroIterationDimsFolder
     auto newOp =
         rewriter.create<ParallelOp>(op.getLoc(), newLowerBounds, newUpperBounds,
                                     newSteps, op.getInitVals(), nullptr);
+    // Erase the empty block that was inserted by the builder.
+    rewriter.eraseBlock(newOp.getBody());
     // Clone the loop body and remap the block arguments of the collapsed loops
     // (inlining does not support a cancellable block argument mapping).
     rewriter.cloneRegionBefore(op.getRegion(), newOp.getRegion(),

@llvmbot
Copy link
Member

llvmbot commented Dec 6, 2023

@llvm/pr-subscribers-mlir-scf

Author: Matthias Springer (matthias-springer)

Changes

ParallelOpSingleOrZeroIterationDimsFolder used to produce invalid IR:

within split at mlir/test/Dialect/SCF/canonicalize.mlir:1 offset :11:3: error: 'scf.parallel' op expects region #<!-- -->0 to have 0 or 1 blocks
  scf.parallel (%i0, %i1, %i2) = (%c0, %c3, %c7) to (%c1, %c6, %c10) step (%c1, %c2, %c3) {
  ^
within split at mlir/test/Dialect/SCF/canonicalize.mlir:1 offset :11:3: note: see current operation:
"scf.parallel"(%4, %5, %3) &lt;{operandSegmentSizes = array&lt;i32: 1, 1, 1, 0&gt;}&gt; ({
^bb0(%arg1: index):
  "memref.store"(%0, %arg0, %1, %arg1, %6) : (i32, memref&lt;?x?x?xi32&gt;, index, index, index) -&gt; ()
  "scf.yield"() : () -&gt; ()
^bb1(%8: index):  // no predecessors
  "scf.yield"() : () -&gt; ()
}) : (index, index, index) -&gt; ()

Together with #74551, this commit fixes mlir/test/Dialect/SCF/canonicalize.mlir when verifying the IR after each pattern application (#74270).


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

1 Files Affected:

  • (modified) mlir/lib/Dialect/SCF/IR/SCF.cpp (+2)
diff --git a/mlir/lib/Dialect/SCF/IR/SCF.cpp b/mlir/lib/Dialect/SCF/IR/SCF.cpp
index 3b55704c4ea07..2e7382160f8eb 100644
--- a/mlir/lib/Dialect/SCF/IR/SCF.cpp
+++ b/mlir/lib/Dialect/SCF/IR/SCF.cpp
@@ -3040,6 +3040,8 @@ struct ParallelOpSingleOrZeroIterationDimsFolder
     auto newOp =
         rewriter.create<ParallelOp>(op.getLoc(), newLowerBounds, newUpperBounds,
                                     newSteps, op.getInitVals(), nullptr);
+    // Erase the empty block that was inserted by the builder.
+    rewriter.eraseBlock(newOp.getBody());
     // Clone the loop body and remap the block arguments of the collapsed loops
     // (inlining does not support a cancellable block argument mapping).
     rewriter.cloneRegionBefore(op.getRegion(), newOp.getRegion(),

@matthias-springer matthias-springer merged commit df7545e into llvm:main Dec 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants