[PATCH] D136483: [mlir][MemRefToLLVM] Reuse existing lowering for collaspe/expand_shape

[Matthias Springer via Phabricator via llvm-commits]

springerm added a comment.

A problematic case would be:

  memref<3x1xi16, strided<[8, 1]>>

This memref can be collapsed to `memref<3xi16, strided<[8]>>`. Using stride `1` (as your new code computes) would be incorrect here, we have to skip the stride of the dim of size 1.

Note, the code that you are looking at is for the "dynamic" case, where strides etc. must be computed at runtime based on data in the memref descriptor. We also have a "static" case, where we infer/verify the static layout map in case there are no `?` strides and/or dims. (If there are a few `?`, only part of the layout map can be inferred/verified.) This code path mirrors the code path that you are looking at and is probably easier to understand and experiment with than the dynamic case. But it implements (or at least should implement) the same logic.

In particular, there is this comment in `computeCollapsedLayoutMap` (MemRefOps.cpp):

  The result stride of a reassociation group is the stride of the last entry
  of the reassociation. (...) Dimensions of size 1 should be skipped, because
  their strides are meaningless and could have any arbitrary value.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D136483/new/

https://reviews.llvm.org/D136483