[PATCH] D139329: [mlir][ExpandStridedMetadata] Handle collapse_shape of dim of size 1 gracefully

Mon Dec 5 07:36:14 PST 2022

qcolombet created this revision.
qcolombet added reviewers: ftynse, bkramer, nicolasvasilache, springerm.
qcolombet added a project: MLIR.
Herald added subscribers: Moerafaat, zero9178, bzcheeseman, sdasgup3, wenzhicui, wrengr, cota, teijeong, rdzhabarov, tatianashp, msifontes, jurahul, Kayjukh, grosul1, Joonsoo, liufengdb, aartbik, mgester, arpith-jacob, antiagainst, shauheen, rriddle, mehdi_amini.
Herald added a project: All.
qcolombet requested review of this revision.
Herald added a subscriber: stephenneuendorffer.

Collapsing dimensions of size 1 with random strides (a.k.a. non-contiguous w.r.t. collapsed dimensions) is a grey area that we'd like to clean-up. (See https://reviews.llvm.org/D136483#3909856)

That said, the implementation in `memref-to-llvm` currently skips dimensions of size 1 when computing the stride of a group.

While longer term we may want to clean that up, for now matches this behavior, at least in the static case.

For the dynamic case, for this patch we stick to `min(group strides)`. However, if we want to handle the dynamic cases correctly while allowing non-truly-contiguous dynamic size of 1, we would need to `if-then-else` every dynamic size. In other words `min(stride_i, for all i in group and dim_i != 1)`.

I didn't implement that in this patch at the moment since `memref-to-llvm` is technically broken in the general case for this. (It currently would only produce something sensible for row major tensors.)

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D139329

Files:
  mlir/lib/Dialect/MemRef/Transforms/ExpandStridedMetadata.cpp
  mlir/test/Dialect/MemRef/expand-strided-metadata.mlir

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D139329.480104.patch
Type: text/x-patch
Size: 6151 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221205/1676c5df/attachment.bin>