[PATCH] D81472: [Matrix] Update load/store intrinsics.

[John McCall via Phabricator via llvm-commits]

rjmccall added a comment.

In D81472#2086096 <https://reviews.llvm.org/D81472#2086096>, @fhahn wrote:

> In D81472#2086062 <https://reviews.llvm.org/D81472#2086062>, @rjmccall wrote:
>
> > [snip]
> >
> > > Oh right, the 'special value' to indicate row/column major would be setting either stride to 1. As long as exactly one of those is  1, the layout of the result/operand should be clear. Personally I find including layout included in the name a bit easier to follow, as it is more explicit. But it might be preferable to have a single variant that handles row/column major depending on the strides (as long as we enforce that exactly one stride has to be 1.), once we add those variants.
> >
> > Why have the restriction that exactly one stride has to be 1?  If you can optimize that as a constant, great, do it, but otherwise just do the separate loads/stores, and impose an UB restriction that the strides have to make them non-overlapping.
>
>
> Besides making assumptions about the layout of the access memory, the intrinsic also specifies the layout of the loaded/stored values (=layout in the flattened vector). If either stride is constant 1, we could use that to determine the layout of the loaded/stored value. I may be missing something, but if both are != 1 or arbitrary values, it is not clear what we should pick for the in-vector layout.


Wait, what?  I assumed you used a canonical layout (presumably column-major) in the flattened vector.  Are you planning to make all your intrinsics support either column-major or row-major layout?  That seems like a lot of complexity in the backend that you're mostly not actually going to be using because the frontend will use a canonical layout.  Are you anticipating that it's going to be important to e.g. peephole a row-major load that's fed into a row-major store so that you don't unnecessarily shuffle the vector?

Also, if you *are* trying to proactively support multiple flattened-vector layouts, I feel like stopping at row-major vs. column-major is probably unnecessarily limiting and you should really allow a whole enumeration's worth of possibilities in case you want to e.g. incorporate internal padding into the vector.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81472