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

[Florian Hahn via Phabricator via llvm-commits]

fhahn marked 4 inline comments as done.
fhahn added a comment.

In D70897#1791861 <https://reviews.llvm.org/D70897#1791861>, @anemet wrote:

> 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.


I've updated the description of the patch to include an example.



================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:560
+      Use &U = *I++;
+      if (ShapeMap.find(U.getUser()) == ShapeMap.end()) {
+        if (!Flattened)
----------------
anemet wrote:
> nit: ShapeMap.count?
 I usually prefer find(), but I guess for DenseMap it does not matter performance wise and I can change it before committing if you prefer.


================
Comment at: llvm/test/Transforms/LowerMatrixIntrinsics/propagate-forward.ll:37
+;
+; SHAPE-LABEL: @transpose(
+; SHAPE-NEXT:  entry:
----------------
anemet wrote:
> FileCheck is never executed with the SHAPE prefix.
I've dropped those, but added an explanation of what we are checking.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70897