[Mlir-commits] [mlir] [mlir][transform] Fix crash on empty type converter region (PR #237717)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir

Author: Yi-Ping Pan (TheCloudlet)

<details>
<summary>Changes</summary>

The default type converter region of transform.apply_conversion_patterns is
declared MaxSizedRegion<1>, so it may have no blocks. The verifier called
front() on it unconditionally, which hits an assertion on an empty region.

Check for an empty region first and report the existing "expected exactly one op
in default type converter region" error. Add tests for both an empty block
(custom syntax) and a region without blocks, which is only expressible in the
generic form.

Fixes #<!-- -->234636

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


2 Files Affected:

- (modified) mlir/lib/Dialect/Transform/IR/TransformOps.cpp (+2-1) 
- (modified) mlir/test/Dialect/Transform/ops-invalid.mlir (+28) 


``````````diff
diff --git a/mlir/lib/Dialect/Transform/IR/TransformOps.cpp b/mlir/lib/Dialect/Transform/IR/TransformOps.cpp
index e40f3b7885c273..1c6c5590a43e1e 100644
--- a/mlir/lib/Dialect/Transform/IR/TransformOps.cpp
+++ b/mlir/lib/Dialect/Transform/IR/TransformOps.cpp
@@ -613,7 +613,8 @@ LogicalResult transform::ApplyConversionPatternsOp::verify() {
   }
   if (getNumRegions() == 2) {
     Region &typeConverterRegion = getRegion(1);
-    if (!llvm::hasSingleElement(typeConverterRegion.front()))
+    if (typeConverterRegion.empty() ||
+        !llvm::hasSingleElement(typeConverterRegion.front()))
       return emitOpError()
              << "expected exactly one op in default type converter region";
     Operation *maybeTypeConverter = &typeConverterRegion.front().front();
diff --git a/mlir/test/Dialect/Transform/ops-invalid.mlir b/mlir/test/Dialect/Transform/ops-invalid.mlir
index 585736d30269e7..a5449580f0feb4 100644
--- a/mlir/test/Dialect/Transform/ops-invalid.mlir
+++ b/mlir/test/Dialect/Transform/ops-invalid.mlir
@@ -996,3 +996,31 @@ module attributes {transform.with_named_sequence} {
     transform.yield
   }
 }
+
+// -----
+
+module attributes {transform.with_named_sequence} {
+  transform.named_sequence @__transform_main(%arg0: !transform.any_op) {
+    // The custom syntax creates an empty block for the type converter region.
+    // expected-error @below {{expected exactly one op in default type converter region}}
+    transform.apply_conversion_patterns to %arg0 {
+    } with type_converter {
+    } : !transform.any_op
+    transform.yield
+  }
+}
+
+// -----
+
+// The type converter region may have no blocks at all, which is only
+// expressible in the generic form. This used to crash the verifier.
+
+module attributes {transform.with_named_sequence} {
+  transform.named_sequence @__transform_main(%arg0: !transform.any_op) {
+    // expected-error @below {{expected exactly one op in default type converter region}}
+    "transform.apply_conversion_patterns"(%arg0) ({
+    }, {
+    }) : (!transform.any_op) -> ()
+    transform.yield
+  }
+}

``````````

</details>


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