[PATCH] D77550: [Matrix] Add TileInfo abstraction for tiled matrix code-gen.

Adam Nemet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 20 09:47:50 PDT 2020


anemet accepted this revision.
anemet added a comment.
This revision is now accepted and ready to land.

> In D77550#2151655 <https://reviews.llvm.org/D77550#2151655>, @anemet wrote:
> 
>> 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.



================
Comment at: llvm/include/llvm/Transforms/Utils/MatrixUtils.h:27
+/// A helper struct to create IR loop nests for tiling in IR of the following
+/// form for CurrentColumn = 0..NumColumns
+///   for CurrentRow = 0..NumRows
----------------
Super-minor nit: can you put a new line before the 'for' and indent properly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77550





More information about the llvm-commits mailing list