[PATCH] D70456: [Matrix] Add first set of matrix intrinsics and initial lowering pass.

[Gerolf Hoflehner via Phabricator via llvm-commits]

Gerolf added a comment.

I have one suggestion and one more question.

-Gerolf



================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:42
+namespace {
+// Given a \p MatrixPtr for the in-memory representation of a matrix,
+// compute the address of the element at index \p Row, \p Col.  \p Offset is
----------------
Could you add a picture/graphic showing a matrix, submatrix, and then their (lowered) memory layout and stride? This would make it easier to understand the computeAddress functions. 


================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:49
+// stride between the end of a column and the start of the next one.
+Value *computeEltAddr(Value *MatrixPtr, Value *Row, Value *Col, Type *EltType,
+                      Value *Offset, IRBuilder<> &Builder) {
----------------
Do you need this function? Why are Row,Col Value here vs unsigned in the caller (computeColumnAddr())?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70456