[PATCH] D75566: [Matrix] Add initial tiling for load/multiply/store chains.

[LuoYuanke via Phabricator via llvm-commits]

LuoYuanke added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:1024
+        Builder.CreateAlloca(Load->getType(), Load->getPointerAddressSpace());
+    Builder.CreateMemCpy(NewLd, MaybeAlign(NewLd->getAlignment()),
+                         Load->getPointerOperand(), Load->getAlign(),
----------------
It seems the cost of Copy is not added to the cost model.


================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:1034
+    // Adjust DT.
+    DTUpdates.push_back({DT.Insert, Fusion, Copy});
+    DTUpdates.push_back({DT.Insert, Copy, Check1});
----------------
I'm confused about line 1034 and 1035. Should it be this? There is no edge from Fusion to Copy and from Copy to Check1.
DTUpdates.push_back({DT.Insert, Copy, Fusion});
DTUpdates.push_back({DT.Insert, Check1, Copy});


================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:1106
+    IRBuilder<> Builder(Store);
+    for (unsigned J = 0; J < C; J += TileSize)
+      for (unsigned I = 0; I < R; I += TileSize) {
----------------
It seems the tile size should be 2 dimension. 1 for row, and 1 for column.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75566