Skip to content
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

More IREEGPUAttrs.cpp cleanups #19142

Merged
merged 1 commit into from
Nov 14, 2024

Conversation

bjacob
Copy link
Contributor

@bjacob bjacob commented Nov 13, 2024

Two things in this PR:

  1. Make a big switch statement more concise.
  2. Currently, DataTileMMAAttr::buildMmaOperation creates a MMAAttr just to call buildMmaOperation on it, to reuse that implementation. In addition to being roundabout, this required a comment explaining why we discarded the error status, as MMAAttr::buildMmaOperation is fallible but here we were already past validation and mutating IR. This PR refactors that to let both call a shared, infallible helper.

@bjacob bjacob force-pushed the decouple-create-mma-intrinsic branch from ba42931 to 09beed3 Compare November 13, 2024 21:40
@raikonenfnu raikonenfnu self-requested a review November 13, 2024 23:33
@bjacob bjacob force-pushed the decouple-create-mma-intrinsic branch from 71697a4 to 88e9fa5 Compare November 14, 2024 14:47
@bjacob bjacob changed the title Decouple buildMmaOperation methods More IREEGPUAttrs.cpp cleanups Nov 14, 2024
@bjacob bjacob requested a review from kuhar November 14, 2024 17:18
@bjacob bjacob marked this pull request as ready for review November 14, 2024 17:18
Signed-off-by: Benoit Jacob <[email protected]>
Copy link
Collaborator

@raikonenfnu raikonenfnu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup, thanks Benoit!

Comment on lines +1144 to +1145
VectorType intrinCType =
getVectorType(builder.getContext(), intrinsic, MMAFragment::Acc);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not required, but any thoughts on moving this into the createMmaOp itself?

@bjacob bjacob merged commit 2a2bd06 into iree-org:main Nov 14, 2024
35 of 36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants