[Mlir-commits] [mlir] [mlir][Transform] Fix crash in SequenceOp::getEffects when body region is empty (PR #185063)

[llvmlistbot at llvm.org]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Mehdi Amini (joker-eph)

<details>
<summary>Changes</summary>

When walking operations post-order and erasing blocks, the inner body block of a nested transform.sequence can be erased while the outer op is still alive. If printAsOperand is called on the outer block at that point, it triggers verification, which calls SequenceOp::getEffects -> getPotentialTopLevelEffects -> getBodyBlock() -> Region::front() on an empty region, causing an assertion failure in ilist_iterator ('\!NodePtr->isKnownSentinel()').

Fix by checking that the body region is non-empty before passing its front block to detail::getPotentialTopLevelEffects in the PossibleTopLevelTransformOpTrait.

Fixes #<!-- -->60213

Assisted-by: Claude Code

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


2 Files Affected:

- (modified) mlir/include/mlir/Dialect/Transform/Interfaces/TransformInterfaces.h (+4-1) 
- (modified) mlir/test/IR/visitors.mlir (+16) 


``````````diff
diff --git a/mlir/include/mlir/Dialect/Transform/Interfaces/TransformInterfaces.h b/mlir/include/mlir/Dialect/Transform/Interfaces/TransformInterfaces.h
index 8b0517a84a675..4a07dc9e7457b 100644
--- a/mlir/include/mlir/Dialect/Transform/Interfaces/TransformInterfaces.h
+++ b/mlir/include/mlir/Dialect/Transform/Interfaces/TransformInterfaces.h
@@ -1169,9 +1169,12 @@ class PossibleTopLevelTransformOpTrait
   /// Populates `effects` with side effects implied by this trait.
   void getPotentialTopLevelEffects(
       SmallVectorImpl<MemoryEffects::EffectInstance> &effects) {
+    Region &region = this->getOperation()->getRegion(0);
+    if (region.empty())
+      return;
     detail::getPotentialTopLevelEffects(
         this->getOperation(), cast<OpTy>(this->getOperation()).getRoot(),
-        *getBodyBlock(), effects);
+        region.front(), effects);
   }
 
   /// Sets up the mapping between the entry block of the given region of this op
diff --git a/mlir/test/IR/visitors.mlir b/mlir/test/IR/visitors.mlir
index 24929f9fb6cf1..85c3d52802c33 100644
--- a/mlir/test/IR/visitors.mlir
+++ b/mlir/test/IR/visitors.mlir
@@ -422,3 +422,19 @@ func.func @test_no_skip_block_erasure_block_args(%arg0: i32, %arg1: i32) -> i32
 module {}
 // CHECK-LABEL: Block post-order erasures (no skip)
 // CHECK-NEXT:  Erasing block ^bb0 from region 0 from operation 'builtin.module'
+
+// -----
+
+// Regression test for https://github.com/llvm/llvm-project/issues/60213:
+// testNoSkipErasureCallbacks should not crash when getEffects is called on a
+// transform.sequence op whose body block has already been erased. Previously
+// printAsOperand triggered verification which called SequenceOp::getEffects ->
+// getBodyBlock() -> Region::front() on an empty region, causing an assertion.
+// CHECK-LABEL: Block post-order erasures (no skip)
+// CHECK:  Erasing block {{.*}} from region 0 from operation 'transform.sequence'
+transform.sequence failures(propagate) {
+^bb0(%arg0: !pdl.operation):
+  sequence %arg0 : !pdl.operation failures(propagate) {
+  ^bb0(%arg1: !pdl.operation):
+  }
+}

``````````

</details>


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