[PATCH] D101762: [Matrix] Fold the transpose into the matmul operand used to fetch scalars

[Adam Nemet via Phabricator via llvm-commits]

anemet added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:1361
 
+    auto *EltType = cast<VectorType>(MatMul->getType())->getElementType();
+    ShapeInfo LShape(MatMul->getArgOperand(2), MatMul->getArgOperand(3));
----------------
fhahn wrote:
> anemet wrote:
> > fhahn wrote:
> > >  Those changes seem quite isolated form the other code in the function. Might be worth to have them in a separate function and call it before `LowerMatrixMultiplyFused`? Otherwise I think it would be good to adjust the name/comment for the function.
> > I think it's good to have this central function to dispatch all fusing opportunities for multiply.  Let me update the comment.
> Sounds good. I still think it might help to keep things readable if the code for this new transform would be in a separate function, called from there. Or at least Move the shape variables & co inside the `if()` block where they are used?
Thanks!  I moved most of it under the if.  I expect that the interface we want for these functions would have the matmul already cracked open (i.e. A, B, R, M, C) that is why I had this outside but that's probably too early to decide at this point.


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

https://reviews.llvm.org/D101762