[PATCH] D102733: [Matrix] Factor and distribute transposes across multiplies

Fri May 21 08:59:22 PDT 2021

anemet marked an inline comment as done.
anemet added inline comments.

================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:679
+  /// Try moving transposes in order to fold them away or into multiplies.
+  void optimizeTransposes() {
+    // First sink all transposes inside matmuls, hoping that we end up with NN,
----------------
anemet wrote:
> fhahn wrote:
> > For the later transforms, we collect a worklist once which contains all matrix instructions. Could we use the same here to avoid having to iterate over each function again?
> Unless we really think this is a performance issue, I'd like to avoid the extra bookkeeping and just represent everything in the IR and no on-the-side data structure that needs updating.  As I was saying offline I think we already have too much bookkeeping going on (e.g. for the remarks) so it's hard to know what to update at times ().  Having a backward and a forward matrix algebraic simplification pass (which is what optimizeTransposes is) that is logically separated from the lowering pass I think makes a good sense in terms of "separation of concerns".  What do you think?
(The only state that is live across optimizeTransposes is the shape-info so that we have gather as much shape info as possible before removing shape-carrying operations like a double-transpose.  I should probably add a comment about this.)

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102733