Repository navigation
Conversation
⏳ Waiting on Required Checks⏳ Required checks have not all concluded yet. This bot run is exiting without failing so those checks can report independently.
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
|
⏳ Waiting for required checks to start or finish:
|
evedovelli
left a comment
evedovelli
left a comment
There was a problem hiding this comment.
The current PR isn't including support to contractions.
Besides the change I've suggested in ck.hpp, there is another required change in amd_wmma.hpp:
diff --git a/projects/composablekernel/include/ck/utility/amd_wmma.hpp b/projects/composablekernel/include/ck/utility/amd_wmma.hpp
index ed2c8a2226..8028278002 100644
--- a/projects/composablekernel/include/ck/utility/amd_wmma.hpp
+++ b/projects/composablekernel/include/ck/utility/amd_wmma.hpp
@@ -12,7 +12,8 @@ namespace ck {
#if defined(__gfx1100__) || defined(__gfx1101__) || defined(__gfx1102__) || \
defined(__gfx1103__) || defined(__gfx1150__) || defined(__gfx1151__) || \
- defined(__gfx1152__) || defined(__gfx1153__) || defined(__gfx11_generic__)
+ defined(__gfx1152__) || defined(__gfx1153__) || defined(__gfx1170__) || \
+ defined(__gfx1171__) || defined(__gfx11_generic__)
#define __gfx11__
#endif
| #endif | ||
| #if defined(__gfx1100__) || defined(__gfx1101__) || defined(__gfx1102__) || \ | ||
| defined(__gfx1103__) || defined(__gfx1150__) || defined(__gfx1151__) || \ | ||
| defined(__gfx1152__) || defined(__gfx1153__) || defined(__gfx11_generic__) |
There was a problem hiding this comment.
The current implementation doesn't add gfx1170 support for the contraction target. Define the new archs as part of __gfx11__ to add the missing support:
| defined(__gfx1152__) || defined(__gfx1153__) || defined(__gfx1170__) || \ | |
| defined(__gfx1171__) || defined(__gfx11_generic__) |
There was a problem hiding this comment.
Based on this: https://github.com/ROCm/rocm-systems/pull/11977/changes#diff-18ce3ca3324bd340f932d819048cbc60660051cac2d62705529379b3f4bbe2fbR455 gfx117X is defined as gfx11-7-generic so I am not sure that we really should add gfx117X targets under __gfx11__
There was a problem hiding this comment.
In this case, they should be defined gfx11-7-generic in here, and in projects/composablekernel/include/ck/utility/amd_wmma.hpp the gfx117 should properly map to the valid wmma operations.
There was a problem hiding this comment.
I'll remove my blocking review from this PR, but without the proper mapping for the WMMAs, the contraction kernels won't work properly.
|
Not sure who authorized this. I expected the gfx117 stuff to be still in the internal repo. This PR clearly won't build correctly. |
All enablement job has been done in the internal repo and we were merging develop branch into private branch regularly. After adding gfx117X support inside rocm-systems: ROCm/rocm-systems#11977 we got approval from management and BU to continue with adding gfx117X support to rocm-libraries |
5384357 to
6c2cf93
Compare
evedovelli
left a comment
evedovelli
left a comment
There was a problem hiding this comment.
Removing the blocking review, but please, add the missing support so hipTensor can also be enabled for gfx1170.
Removing the blocking review, but please, add the missing support so hipTensor can also be enabled for gfx1170.
Have you actually tried building this branch for gfx117? |
|
Created a new PR #13696, made sure it actually builds for gfx1170. |
JIRA ID: ROCM-21096, AIRADSW-699, AIRADSW-588
Motivation
Enable Composable Kernel on gfx1170 and gfx1171.
Technical Details
Register gfx1170 and gfx1171 as Composable Kernel GPU targets. Use the gfx11 buffer-resource setting for those targets, and enable the WMMA path for gfx1170 and gfx1171.
Test Plan
Test Result
Pending.
Submission Checklist
Co-authored-by: Artem Kuzmitckii Artem.Kuzmitckii@amd.com
Co-authored-by: Aaryaman Vasishta aaryaman.vasishta@amd.com
Co-authored-by: Milica Stankovic Milica.Stankovic@amd.com