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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

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.

>> I've put up D81498 <https://reviews.llvm.org/D81498>. It just adds `volatile` to the generated loads/stores, if `IsVolatile` is true. Do you think `volatile` should/has to prevent splitting the memory operation into multiple loads/stores?
> 
> No, I think the semantics here are more like a volatile memcpy: we're demanding that the access be done, but not guaranteeing anything about access widths.

Sounds good, that's what the patch assumes :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81472