Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Brgemm register tiling for bf16 type #1005
base: main
Are you sure you want to change the base?
Brgemm register tiling for bf16 type #1005
Changes from 4 commits
7943e1e
310a21a
7653434
09d9ae7
ef08d66
974c3eb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has baked-in assumptions that are not verified.
As the pass now operates on generic, we need to strictly filter ops that are accepted. I think you need to at least ensure it is a VNNI contraction first - there should be some suitable helpers in
VnniUtils
.If f32 generic should be supported as well, it might need some extra checks there too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing out this.
Added more checks for
f32
andbf16
type. Used a check fromvnniutils
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I am understanding right, this transform is meant to operate on linalg ops. As I expect all the ops you want to support will implement TilingInterface, would it be possible to just use the
TileUsingFor
transform instead of manually implementing tiling?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it a target for linalg ops. But, now we are focusing only on
batch reduce matmul
. As far as I know, the plan is to maketiling
,hoisting
,vector-to-fma
, andvector-to-amx
as a oneregister blocking
transform schedule in the upstream.Also, we extended this pass to support
bf16
type, so not taught of usingTileUsingFor
transform schedule.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not one register blocking transform, but one schedule of multiple transforms that implement register blocking at the vector level. These should still be separate transforms and upstreamed not as "vector tiling" transforms, but "register blocking on vector" transforms.