This is an archive of the discontinued LLVM Phabricator instance.

[mlir][MemRef] Allow transposed layouts in ExpandShapeOp.
ClosedPublic

Authored by nicolasvasilache on Mar 31 2022, 12:36 PM.

Details

Summary

https://reviews.llvm.org/D122641 introduced fixes to the ExpandShapeOp verifier
but also introduced an artificial layout limitation that prevents the consideration of transposed layouts.

This revision fixes the omissions and reimplements the logic using saturated arithmetic which is more
idiomatic and avoids leaking internal implementation details.

Tests cases are added for transposed layouts.

Diff Detail

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2022, 12:36 PM
nicolasvasilache requested review of this revision.Mar 31 2022, 12:36 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 31 2022, 12:36 PM
springerm accepted this revision.Apr 1 2022, 2:37 AM
springerm added inline comments.
mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
1724

function can probably be deleted

This revision is now accepted and ready to land.Apr 1 2022, 2:37 AM
springerm added inline comments.Apr 1 2022, 2:45 AM
mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
1834

nit: can use reassoc.pop_back here

1840

statically

springerm added inline comments.Apr 1 2022, 2:56 AM
mlir/lib/Dialect/MemRef/IR/MemRefOps.cpp
1841

We have a crash due to this check here in the bufferization.

I think we should check for

if (stride != Wrapper::stride(srcStrides[idx - 1]) && srcShape[idx] != 1)

or something like that.

Then add this test to tensor/bufferize.mlir:

// CHECK-DAG: #[[$MAP3:.*]] = affine_map<(d0, d1) -> (d0 * 2 + d1)>
...

// CHECK-LABEL: func @tensor.expand_shape_of_slice2(
//  CHECK-SAME:     %[[t1:.*]]: tensor<1x2xf32>
func @tensor.expand_shape_of_slice2(%t1: tensor<1x2xf32>) -> tensor<1xf32> {
  // CHECK: memref.subview {{.*}} : memref<1x2xf32> to memref<1x1xf32, #[[$MAP3]]>
  %0 = tensor.extract_slice %t1[0, 0][1, 1][1, 1] : tensor<1x2xf32> to tensor<1x1xf32>
  // CHECK: memref.collapse_shape %{{.*}} [
  // CHECK-SAME: [0, 1]] : memref<1x1xf32, #[[$MAP3]]> into memref<1xf32>
  %1 = tensor.collapse_shape %0 [[0, 1]] : tensor<1x1xf32> into tensor<1xf32>
  return %1 : tensor<1xf32>
}

(fixes https://github.com/llvm/llvm-project/issues/54249#issuecomment-1084831552)

This sentence should be removed from MemRefOps.td with this change:

Note: This op currently assumes that the inner strides are of the source/result layout map are the faster-varying ones.
nicolasvasilache marked 3 inline comments as done.

Address comments and rebase.

This revision was landed with ongoing or failed builds.Apr 6 2022, 1:19 AM
This revision was automatically updated to reflect the committed changes.