Skip to content

Commit

Permalink
Fix memref.expand_shape verifier (#91501)
Browse files Browse the repository at this point in the history
Torch-mlir integration is currently blocked on `memref.expand_shape`
verifier errors of the form

```
'memref.expand_shape' op invalid output shape provided at pos 1
```

The verifier code generating these errors was introduced in
#91245. I have commented there
why I believe it's incorrect. This PR has my suggested fix.

Unfortunately, this does not seem to be directly testable on `memref`
IR, because `static_output_shape` is not directly exposed in the custom
assembly format.
  • Loading branch information
bjacob authored May 8, 2024
1 parent bb6df08 commit dabdec1
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 8 deletions.
11 changes: 5 additions & 6 deletions mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2356,12 +2356,11 @@ LogicalResult ExpandShapeOp::verify() {
// Verify if provided output shapes are in agreement with output type.
DenseI64ArrayAttr staticOutputShapes = getStaticOutputShapeAttr();
ArrayRef<int64_t> resShape = getResult().getType().getShape();
unsigned staticShapeNum = 0;

for (auto [pos, shape] : llvm::enumerate(resShape))
if (!ShapedType::isDynamic(shape) &&
shape != staticOutputShapes[staticShapeNum++])
emitOpError("invalid output shape provided at pos ") << pos;
for (auto [pos, shape] : llvm::enumerate(resShape)) {
if (!ShapedType::isDynamic(shape) && shape != staticOutputShapes[pos]) {
return emitOpError("invalid output shape provided at pos ") << pos;
}
}

return success();
}
Expand Down
2 changes: 1 addition & 1 deletion mlir/test/Dialect/MemRef/fold-memref-alias-ops.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ func.func @fold_dynamic_subview_with_memref_store_expand_shape(%arg0 : memref<16
// CHECK-SAME: (%[[ARG0:.*]]: memref<2048x16xf32>, %[[ARG1:.*]]: index, %[[ARG2:.*]]: index, %[[ARG3:.*]]: index, %[[ARG4:.*]]: index)
func.func @fold_memref_alias_expand_shape_subview_load_store_dynamic_dim(%alloc: memref<2048x16xf32>, %c10: index, %c5: index, %c0: index, %sz0: index) {
%subview = memref.subview %alloc[%c5, 0] [%c10, 16] [1, 1] : memref<2048x16xf32> to memref<?x16xf32, strided<[16, 1], offset: ?>>
%expand_shape = memref.expand_shape %subview [[0], [1, 2, 3]] output_shape [1, 16, %sz0, 1] : memref<?x16xf32, strided<[16, 1], offset: ?>> into memref<?x1x8x2xf32, strided<[16, 16, 2, 1], offset: ?>>
%expand_shape = memref.expand_shape %subview [[0], [1, 2, 3]] output_shape [%sz0, 1, 8, 2] : memref<?x16xf32, strided<[16, 1], offset: ?>> into memref<?x1x8x2xf32, strided<[16, 16, 2, 1], offset: ?>>
%dim = memref.dim %expand_shape, %c0 : memref<?x1x8x2xf32, strided<[16, 16, 2, 1], offset: ?>>

affine.for %arg6 = 0 to %dim step 64 {
Expand Down
7 changes: 6 additions & 1 deletion mlir/test/Dialect/MemRef/ops.mlir
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,8 @@ func.func @expand_collapse_shape_dynamic(%arg0: memref<?x?x?xf32>,
%arg3: memref<?x42xf32, strided<[42, 1], offset: 0>>,
%arg4: index,
%arg5: index,
%arg6: index) {
%arg6: index,
%arg7: memref<4x?x4xf32>) {
// CHECK: memref.collapse_shape {{.*}} {{\[}}[0, 1], [2]]
// CHECK-SAME: memref<?x?x?xf32> into memref<?x?xf32>
%0 = memref.collapse_shape %arg0 [[0, 1], [2]] :
Expand Down Expand Up @@ -248,6 +249,10 @@ func.func @expand_collapse_shape_dynamic(%arg0: memref<?x?x?xf32>,
// CHECK-SAME: memref<?xf32, strided<[1]>> into memref<?x42xf32>
%r3 = memref.expand_shape %3 [[0, 1]] output_shape [%arg6, 42] :
memref<?xf32, strided<[1]>> into memref<?x42xf32>

// CHECK: memref.expand_shape {{.*}} {{\[}}[0, 1], [2], [3, 4]]
%4 = memref.expand_shape %arg7 [[0, 1], [2], [3, 4]] output_shape [2, 2, %arg4, 2, 2]
: memref<4x?x4xf32> into memref<2x2x?x2x2xf32>
return
}

Expand Down

0 comments on commit dabdec1

Please sign in to comment.