[PATCH] D70897: [Matrix] Add forward shape propagation and first shape aware lowerings.

[Adam Nemet via Phabricator via llvm-commits]

anemet added a comment.

In D70897#1769062 <https://reviews.llvm.org/D70897#1769062>, @fhahn wrote:

> In D70897#1768320 <https://reviews.llvm.org/D70897#1768320>, @anemet wrote:
>
> > Also the existing test diffs are hard to read, please explain what's going on there.
>
>
> I've added comments to break up the check lines


Thanks for that.  Can you also explain the nature of the changes with one example.  I am assuming we're removing embedVectors/extractVectors, i.e. bunch of shuffles, pointing to one example would be useful.



================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:116
+///  2.1. Get column vectors for each argument. If we already lowered the
+///       definition of an argument, use the produce column vectors directly.
+///       If not, split the operand vector containing an embedded matrix into
----------------
produced


================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:121
+///       a set of column vectors containing result matrix. Note that we lower
+///       all instructions we have shape information. Besides the intrinsics,
+///       this includes stores for example.
----------------
all instruction *that* we have


================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:130
+///    obsolete instructions.
+///
 class LowerMatrixIntrinsics {
----------------
Nice write-up!


================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:560
+      Use &U = *I++;
+      if (ShapeMap.find(U.getUser()) == ShapeMap.end()) {
+        if (!Flattened)
----------------
nit: ShapeMap.count?


================
Comment at: llvm/test/Transforms/LowerMatrixIntrinsics/propagate-forward.ll:37
+;
+; SHAPE-LABEL: @transpose(
+; SHAPE-NEXT:  entry:
----------------
FileCheck is never executed with the SHAPE prefix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70897