[PATCH] D131125: [Matrix] Add special case dot product lowering

[Vir Narula via Phabricator via llvm-commits]

virnarula marked 2 inline comments as done.
virnarula added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:1236
+    Function *Add = Intrinsic::getDeclaration(Func.getParent(), AddOpCode,
+                                    ElementType);
+
----------------
thegameg wrote:
> Not sure what's going on here. Intrinsic with `Instruction::Add`? It also doesn't seem to be really used.
I use it to get the function type when estimating sequential add cost. line 1243. Do you suggest doing it a different way?


================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:1252
+    auto GetBuiltinLoad = [](Value *Val) -> CallInst * {
+      if (match(Val, m_Intrinsic<Intrinsic::matrix_column_major_load>())) {
+        return cast<CallInst>(Val);
----------------
thegameg wrote:
> You cold match the stride directly here:
> 
> ```
> ConstantInt *LoadStride = nullptr;
> if (match(Val, m_Intrinsic<Intrinsic::matrix_column_major_load>(
>                        m_Value(), m_ConstantInt(LoadStride), m_Value(), m_Value()) &&
>     LoadStride->getZExtValue() == Stride) {
> ```
> 
> and pass `Stride` as an argument of `GetBuiltinLoad`.
I tried this but I realized that I would have to have a control flow to check if the load has size of `n` anyways. This is because if we return null, it could mean there's a LoadInst (instead of a builtin load) or the stride in incorrect, which require different behaviors.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131125