-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[AMD] Always swap operands of mfma and use mfma.transposed layout #4767
base: main
Are you sure you want to change the base?
Conversation
zhanglx13
commented
Sep 20, 2024
•
edited
Loading
edited
- Fixed the issue with getOrder for mfma layout
- Fixed the issue with reduceOp when dealing with mfma.transposed layout
Also fixed the issue with getOrder for mfma layout
In general, we should use getThreadOrder in most places where getOrder is called. Note that order and threadOrder can be different, and this is the case for mfma.transposed layout.
2065595
to
c525ee0
Compare
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.
Could you explain more how the transpose is done (I think via the logic in dot to llvm conversion at register level?) and how this is expected to improve global store in the commit message? good for others to understand why this change.
@@ -79,6 +79,8 @@ SmallVector<unsigned> getWarpOrder(Attribute layout); | |||
|
|||
SmallVector<unsigned> getOrder(Attribute layout); | |||
|
|||
SmallVector<unsigned> getThreadOrder(Attribute layout); |
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.
Add some doc to both this one and the above getOrder
so that it's easier to know the difference?
mfmaEnc = ttg::AMDMfmaEncodingAttr::get( | ||
oldRetType.getContext(), | ||
/*versionMajor*/ mfmaVersion, /*versionMinor*/ 0, warpsPerTile, | ||
/*instrShape*/ mDim, nDim, isTransposed, CTALayout); | ||
/*instrShape*/ mDim, nDim, /*isTransposed*/ true, CTALayout); |
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.
Can you add some comments in the above of why we want to always set it as transposed? It makes it easier to follow for others reading the code.