This is an archive of the discontinued LLVM Phabricator instance.

[Matrix] Simplify matmuls with scalars
ClosedPublic

Authored by thegameg on Jul 20 2022, 7:59 AM.

Details

Summary

If one of the operands is a transposed splat, the transpose can be
removed.

This is useful to simplify when transposes are distributed to operands
of a matmul:

  • k^T -> k
  • (A * k)^t -> A^t * k

Diff Detail

Event Timeline

thegameg created this revision.Jul 20 2022, 7:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 20 2022, 7:59 AM
thegameg requested review of this revision.Jul 20 2022, 7:59 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 20 2022, 7:59 AM
fhahn added inline comments.Aug 16 2022, 9:32 AM
llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
104

nit: is inline the right thing here or should this be static instead?

thegameg marked an inline comment as done.Sep 1 2022, 12:10 PM
thegameg added inline comments.
llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
104

Probably neither actually, thanks!

thegameg updated this revision to Diff 457375.Sep 1 2022, 1:16 PM
thegameg marked an inline comment as done.
fhahn added inline comments.Sep 2 2022, 1:08 AM
llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
894

Should we still print this when -debug is passed without PrintAfterTransposeOpt?

llvm/test/Transforms/LowerMatrixIntrinsics/transpose-opts.ll
1162 ↗(On Diff #457375)

This should probably check the operands are as expected? It might also make sense to keep the test that just use the print output to a separate file or also generate the IR checks for them.

thegameg updated this revision to Diff 457661.Sep 2 2022, 11:59 AM
thegameg marked an inline comment as done.
thegameg added inline comments.
llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
894

I assumed it's not that useful to dump the whole function in that case, but happy to make it work if you think otherwise.

fhahn accepted this revision.Sep 2 2022, 1:33 PM

LGTM, thanks!

llvm/test/Transforms/LowerMatrixIntrinsics/after-transpose-opts.ll
5

nit: could use -disable-output instead of -o /dev/null.

This revision is now accepted and ready to land.Sep 2 2022, 1:33 PM
This revision was landed with ongoing or failed builds.Sep 2 2022, 3:50 PM
This revision was automatically updated to reflect the committed changes.
thegameg marked an inline comment as done.