[Mlir-commits] [mlir] [mlir][memref] Reject memref.reshape with static shape and unranked result (PR #223496)

[llvmlistbot at llvm.org]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-mlir-memref

Author: Stanislav Bardyuk (kodlan)

<details>
<summary>Changes</summary>

The memref.reshape docs list two forms: a shape operand of static length
with a ranked result, or a shape operand of dynamic length with an
unranked result. The verifier only checks this pairing when the result is
ranked, so the third combination (static length, unranked result) is
accepted:

```mlir
%0 = memref.reshape %src(%shape) : (memref<*xf32>, memref<2xindex>) -> memref<*xf32>
```

Both consumers of the op pick the ranked code path from the shape
operand alone and then cast the result type to MemRefType, which asserts
on the unranked result: MemRefReshapeOpLowering in MemRefToLLVM (the
reporter's -convert-openmp-to-llvm crash, also reachable through
-finalize-memref-to-llvm) and MemRefReshapeOpConverter in -memref-expand.

Add the missing check to the verifier so the op is rejected with a
regular diagnostic, mirroring the existing error for the opposite
mismatch. Nothing in tree produces the rejected form for memref (checked
all producers and tests). The form could instead be supported by making
both consumers dispatch on the result type, but the docs describe the
pairing as the contract, so this enforces it.

tensor.reshape has the same gap in its verifier; it is left alone here
since no tensor consumer crashes on it and two Shape canonicalization
tests use that form.

Fixes #<!-- -->205057

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


2 Files Affected:

- (modified) mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp (+4-1) 
- (modified) mlir/test/Dialect/MemRef/invalid.mlir (+8) 


``````````diff
diff --git a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
index 4b5a4c1780507..ddca779523a78 100644
--- a/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
+++ b/mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
@@ -3035,12 +3035,15 @@ LogicalResult ReshapeOp::verify() {
   if (resultMemRefType) {
     if (!resultMemRefType.getLayout().isIdentity())
       return emitOpError("result memref type should have identity affine map");
-    if (shapeSize == ShapedType::kDynamic)
+    if (ShapedType::isDynamic(shapeSize))
       return emitOpError("cannot use shape operand with dynamic length to "
                          "reshape to statically-ranked memref type");
     if (shapeSize != resultMemRefType.getRank())
       return emitOpError(
           "length of shape operand differs from the result's memref rank");
+  } else if (ShapedType::isStatic(shapeSize)) {
+    return emitOpError("cannot use shape operand with static length to "
+                       "reshape to unranked memref type");
   }
   return success();
 }
diff --git a/mlir/test/Dialect/MemRef/invalid.mlir b/mlir/test/Dialect/MemRef/invalid.mlir
index 7f2861985f6dc..788e38b19ac3d 100644
--- a/mlir/test/Dialect/MemRef/invalid.mlir
+++ b/mlir/test/Dialect/MemRef/invalid.mlir
@@ -261,6 +261,14 @@ func.func @memref_reshape_dst_ranked_shape_unranked(
 
 // -----
 
+func.func @memref_reshape_dst_unranked_shape_ranked(
+       %buf: memref<*xf32>, %shape: memref<1xi32>) {
+  // expected-error @+1 {{cannot use shape operand with static length to reshape to unranked memref type}}
+  memref.reshape %buf(%shape) : (memref<*xf32>, memref<1xi32>) -> memref<*xf32>
+}
+
+// -----
+
 func.func @memref_reshape_dst_shape_rank_mismatch(
        %buf: memref<*xf32>, %shape: memref<1xi32>) {
   // expected-error @+1 {{length of shape operand differs from the result's memref rank}}

``````````

</details>


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