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

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 3 inline comments as done.
fhahn 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),
----------------
anemet wrote:
> Should we name these something better than the default TMP?  I would even include “I” in the name.
Done, it's `result.vec.I` now.


================
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.
----------------
anemet wrote:
> Can we name the Value for this as well (ok as a follow-on)?
I'll do that as 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(
----------------
anemet wrote:
> You should clarify in the description that you're changing this test to prevent tiling.
I added a separate option to control loop generation for now, as there is a bit of tuning left to do to make sure there are no performance regressions. This is also handy for the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81308