This is an archive of the discontinued LLVM Phabricator instance.

[Matrix] Add option to use row-major matrix layout as default.
ClosedPublic

Authored by fhahn on Mar 17 2020, 2:10 PM.

Details

Summary

This patch adds a -matrix-default-layout option which can be used to
set the default matrix layout to row-major or column-major (default).

The initial patch updates codegen for loads, stores, binary operators
and matrix multiply.

TODO: additional tests.

Diff Detail

Event Timeline

fhahn created this revision.Mar 17 2020, 2:10 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 17 2020, 2:10 PM
fhahn updated this revision to Diff 251609.Mar 20 2020, 4:37 AM

Rebased on top of D75566.

TODO: additional tests.

Is this WIP?

llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
325

column vector?

789

is this computeColumnAddr or computeVectorAddr now?

989–993

This interface got me confused in particular in the context of this patch. Comment says tile-sized but then there is a flag whether it's tiled at all. Anyway we can improve this?

Sounds like this should be just called emitMatrixMultiply? You might want to insert a patch before this one that fixes this.

anemet requested changes to this revision.Mar 29 2020, 12:32 PM
This revision now requires changes to proceed.Mar 29 2020, 12:32 PM
fhahn updated this revision to Diff 253550.Mar 30 2020, 4:26 AM

TODO: additional tests.

Is this WIP?

Originally it was missing tests for binary operators/loads/stores. Added now additional tests now.

Update renaming function names/docs only referring to column vectors. O

fhahn updated this revision to Diff 253557.Mar 30 2020, 5:00 AM

Adjust documentation in a few more places, add assertions to make sure operands of binary ops agree on the layout.

fhahn marked 4 inline comments as done.Mar 30 2020, 5:36 AM
fhahn added inline comments.
llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp
789

Fix and also in various other places.

989–993
anemet accepted this revision.Apr 5 2020, 1:54 PM

LGTM!

This revision is now accepted and ready to land.Apr 5 2020, 1:54 PM
This revision was automatically updated to reflect the committed changes.
fhahn marked an inline comment as done.