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

Yet more IREEGPUAttrs cleanup: drop get{A,B,C}SingleSubgroupLayout methods #19169

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bjacob
Copy link
Contributor

@bjacob bjacob commented Nov 15, 2024

These methods existed before we added the unified getSingleSubgroupLayout taking a MMAFragment argument. Now they can go away. Actually polymorphic callers, which motivated this being an interface method, are taken care of by a new overload of getSingleSubgroupLayout taking a MMAInterfaceAttr.

Signed-off-by: Benoit Jacob <[email protected]>
@bjacob bjacob force-pushed the drop-getXsinglesubgrouplayout branch 4 times, most recently from 77e3459 to 48fea31 Compare November 15, 2024 21:16
Signed-off-by: Benoit Jacob <[email protected]>
@bjacob bjacob force-pushed the drop-getXsinglesubgrouplayout branch from 48fea31 to 835eace Compare November 15, 2024 21:20
@bjacob bjacob marked this pull request as ready for review November 15, 2024 21:33
Copy link
Contributor

@krzysz00 krzysz00 left a comment

Choose a reason for hiding this comment

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

I'm not seeing anything obviously wrong here but I'm also new to the code so I'd rather not stamp prematurely

@@ -69,7 +69,7 @@ static bool is_AMD_WMMA(MMAIntrinsic intrinsic) {

static int64_t getIntrinsicSubgroupSize(MMAIntrinsic intrinsic) {
// Not using Wave64 at all at the moment, so the only place where the
// subgroup size is CDNA* architectures.
// subgroup size is 64 is CDNA* architectures.
Copy link
Contributor

Choose a reason for hiding this comment

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

on CDNA*

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.

2 participants