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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:157-166
+  MatrixTy getMatrix(Value *MatrixVal, const ShapeInfo &SI,
+                     IRBuilder<> Builder) {
+    auto *VType = dyn_cast<VectorType>(MatrixVal->getType());
+    assert(VType && "MatrixVal must be a matrix type");
+    if (auto *C = dyn_cast<Constant>(MatrixVal)) {
+      return splitToColumnVectors(MatrixVal, VType->getElementType(), SI,
+                                  Builder);
----------------
anemet wrote:
> splitVector and splitToColumnVectors names are not very descriptive especially WRT their difference.
Now there's just getMatrix.


================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:243-271
+  /// Return the address of a column vector (\p EltType x \p Rows) at index (\p
+  /// Row, \p Col) of \p Base with original column size of \p Stride elements.
+  Value *computeColumnAddr(Value *Base, unsigned Row, unsigned Col,
+                           Value *Stride, Type *EltType, unsigned Rows,
+                           IRBuilder<> &Builder) {
+    Value *EltPtr =
+        computeEltAddr(Base, Builder.getInt32(Row), Builder.getInt32(Col),
----------------
anemet wrote:
> Commenting the differences between the overloads would be good, also can these be stand-alone static functions?
Moved the code out and the overload is gone now


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70456