This is an archive of the discontinued LLVM Phabricator instance.

[Matrix] Use TileInfo to create tiled loop nest for matrix multiply.
ClosedPublic

Authored by fhahn on Jun 5 2020, 2:24 PM.

Details

Summary

This patch uses the TileInfo introduced in D77550 to generate a loop
nest for tiled matrix multiplication, instead of generating the
unrolled code for the whole multiplication. This makes code-generation
more scalable for larger matrixes.

Initially loops are only used if both the number of rows and columns are
divisible by the tile size. Other cases will be added as follow-up.

Diff Detail

Event Timeline

fhahn created this revision.Jun 5 2020, 2:24 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 5 2020, 2:24 PM
LuoYuanke added inline comments.Jun 6 2020, 10:01 PM
llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
1196

Line 1194 and 1196 seems unnecessary. The insert point is specified in line 1202.

fhahn updated this revision to Diff 269268.Jun 8 2020, 9:34 AM

Remove unnecessary insertion point settings, thanks!

fhahn marked 2 inline comments as done.Jun 8 2020, 9:35 AM
fhahn added inline comments.
llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
1196

Nice catch, thanks! Seems like a leftover from earlier iterations. Removed!

aartbik added a subscriber: aartbik.Jul 2 2020, 1:23 PM
anemet added inline comments.Jul 13 2020, 10:33 AM
llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
1203

Should we name these something better than the default TMP? I would even include “I” in the name.

1218

Can we name the Value for this as well (ok as a follow-on)?

llvm/test/Transforms/LowerMatrixIntrinsics/multiply-fused.ll
9

You should clarify in the description that you're changing this test to prevent tiling.

fhahn planned changes to this revision.Jul 15 2020, 9:42 AM
fhahn marked an inline comment as done.
fhahn updated this revision to Diff 279255.Jul 20 2020, 8:12 AM

Add -fuse-matrix-use-loops option to use new behavior, defaulting to false for now. Use this option in tests.

fhahn marked 3 inline comments as done.Jul 20 2020, 8:14 AM
fhahn added inline comments.
llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
1203

Done, it's result.vec.I now.

1218

I'll do that as follow-on.

llvm/test/Transforms/LowerMatrixIntrinsics/multiply-fused.ll
9

I added a separate option to control loop generation for now, as there is a bit of tuning left to do to make sure there are no performance regressions. This is also handy for the test.

anemet accepted this revision.Jul 20 2020, 9:54 AM

LGTM.

llvm/test/Transforms/LowerMatrixIntrinsics/multiply-fused-volatile.ll
2 ↗(On Diff #279255)

Did you also want this -fuse-matrix-use-loops=false ?

I am fine either way, just want to double-check your intentions.

This revision is now accepted and ready to land.Jul 20 2020, 9:54 AM
This revision was automatically updated to reflect the committed changes.