[PATCH] D81308: [Matrix] Use TileInfo to create tiled loop nest for matrix multiply.

[Adam Nemet via Phabricator via llvm-commits]

anemet added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:1203
+    for (unsigned I = 0; I < TileSize; I++) {
+      auto *Phi = Builder.CreatePHI(TileVecTy, 2);
+      Phi->addIncoming(ConstantAggregateZero::get(TileVecTy),
----------------
Should we name these something better than the default TMP?  I would even include “I” in the name.


================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:1218
+                            {TileSize, TileSize}, EltType, Builder);
+    emitMatrixMultiply(TileResult, A, B, AllowContract, Builder, true);
+    // Store result after the inner loop is done.
----------------
Can we name the Value for this as well (ok as a follow-on)?


================
Comment at: llvm/test/Transforms/LowerMatrixIntrinsics/multiply-fused.ll:9
 
-define void @multiply(<16 x double> * %A, <16 x double> * %B, <16 x double>* %C) {
+define void @multiply(<15 x double> * %A, <6 x double> * %B, <10 x double>* %C) {
 ; CHECK-LABEL: @multiply(
----------------
You should clarify in the description that you're changing this test to prevent tiling.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81308