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

[John McCall via Phabricator via llvm-commits]

rjmccall added a comment.

My immediate concern is just that I think the memory layout of the matrix should be orthogonal to the component layout of the vector.  If you want the matrix intrinsics to support a variety of vector layouts, you should pass down the expected layout as a constant argument to the intrinsic rather than picking it up purely from whether the matrix is being loaded from a row-major or column-major layout in memory.  I would guess that making that constant an  i32 is probably sufficiently future-proof; if you ever need more structure than that, you'd probably be better off biting the bullet and adding an `llvm::MatrixType`.

My thinking here is that, for example, you're adding a builtin to Clang to do a column-major load.  You want that to produce a column-major vector layout because that's your canonical layout within Clang.  But you should also eventually add a builtin to do a row-major load, because there are quite a few reasons people might have a matrix in memory in row-major order: for example, if they've declared a nested C array, they'll naturally get row-major order.  That builtin *also* needs to produce a column-major vector layout.  So tying the two things together is bad intrinsic design.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81472