This is an archive of the discontinued LLVM Phabricator instance.

[Matrix] Add TileInfo abstraction for tiled matrix code-gen.
ClosedPublic

Authored by fhahn on Apr 6 2020, 7:19 AM.

Details

Summary

This patch adds a TileInfo abstraction and utilities to
create a 3-level loop nest for tiling.

Diff Detail

Event Timeline

fhahn created this revision.Apr 6 2020, 7:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 6 2020, 7:19 AM
fhahn updated this revision to Diff 256036.Apr 8 2020, 8:48 AM

Update loop info.

fhahn retitled this revision from [Matrix] Add TileInfo abstraction for tiled matrix code-gen (WIP). to [Matrix] Add TileInfo abstraction for tiled matrix code-gen..
fhahn edited the summary of this revision. (Show Details)
anemet added a subscriber: anemet.Jul 13 2020, 10:10 AM
anemet added inline comments.
llvm/include/llvm/Transforms/Utils/MatrixUtils.h
27

Please comment the purpose for this class

61

All of these need comments too

anemet requested changes to this revision.Jul 13 2020, 10:11 AM
This revision now requires changes to proceed.Jul 13 2020, 10:11 AM

BTW, will this be shared between .cpp's? I.e. what is the justification for a header?

fhahn updated this revision to Diff 279230.Jul 20 2020, 6:52 AM

Thanks Adam! Added comments.

BTW, will this be shared between .cpp's? I.e. what is the justification for a header?

I guess they could in theory also be added directly to LowerMatrixIntrinsics.cpp. But I think it makes sense to have a separate file for various matrix-related utilities that are not directly tied to the intrinsics. I don't anticipate users outside of LowerMatrixIntrinsics.cpp anytime soon upstream, so I could also move them there. I think in the long-term we want to specify specialized, target-specific lowerings outside of LowerMatrixIntrinsics.cpp, so we might as well start with more generally accessible helpers.

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

BTW, will this be shared between .cpp's? I.e. what is the justification for a header?

I guess they could in theory also be added directly to LowerMatrixIntrinsics.cpp. But I think it makes sense to have a separate file for various matrix-related utilities that are not directly tied to the intrinsics. I don't anticipate users outside of LowerMatrixIntrinsics.cpp anytime soon upstream, so I could also move them there. I think in the long-term we want to specify specialized, target-specific lowerings outside of LowerMatrixIntrinsics.cpp, so we might as well start with more generally accessible helpers.

Works for me!

LGTM.

llvm/include/llvm/Transforms/Utils/MatrixUtils.h
27

Super-minor nit: can you put a new line before the 'for' and indent properly.

This revision is now accepted and ready to land.Jul 20 2020, 9:47 AM