Add support for 3d kernel launches#4931
Open
music-dino wants to merge 3 commits into
Open
Conversation
pfultz2
reviewed
Jun 2, 2026
| @@ -41,7 +41,11 @@ struct code_object_op | |||
| value::binary code_object{}; | |||
| std::string symbol_name = ""; | |||
| std::size_t global = 0; | |||
Collaborator
There was a problem hiding this comment.
Can we define this as std::array<std::size_t, 3> instead?
Contributor
Author
There was a problem hiding this comment.
We could but I'm not sure we gain much by it. We lose some semantical clarity because accessing the individual values boils down to indexing the array instead of using member names.
pfultz2
reviewed
Jun 2, 2026
| @@ -71,18 +71,46 @@ struct MIGRAPHX_GPU_EXPORT kernel | |||
|
|
|||
| void launch(hipStream_t stream, | |||
| std::size_t global, | |||
Collaborator
There was a problem hiding this comment.
Can we add an overload that takes std::array<std::size_t, 3> instead?
pfultz2
reviewed
Jun 2, 2026
| @@ -38,7 +38,11 @@ struct context; | |||
| struct hip_compile_options | |||
| { | |||
| std::size_t global; | |||
Collaborator
There was a problem hiding this comment.
Can we define this as std::variant<std::size_t, std::array<std::size_t, 3>>? If the constructor is ambiguous we can use the picked_variant to resolve the ambiguity.
Contributor
Author
There was a problem hiding this comment.
Yes picked_variant would be necessary due to call sites that use signed integer literals.
I'm also not sure if making the change is worth it, the existing version with the correct defaults is applicable for 1d, 2d and 3d, for the variant we'd have to also add as std::array<std::size_t, 2> to the variant as well to match that(or leave it to the caller to use the 3d).
I've implemented it locally, but I think it adds some complexity to the code that is not necessary. However, if you want, I can push what I've implemented.
Collaborator
|
Although it wont be used by ck tile, we need to update the |
Regressions detected 🔴 |
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #4931 +/- ##
===========================================
+ Coverage 92.55% 92.63% +0.08%
===========================================
Files 587 587
Lines 30383 30469 +86
===========================================
+ Hits 28118 28222 +104
+ Misses 2265 2247 -18 🚀 New features to boost your workflow:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Comment on lines
+82
to
83
| // Returns the block size. When workgroup has non-uniform block, this returns size of the uniform | ||
| // block. |
Contributor
There was a problem hiding this comment.
[format.py] reported by reviewdog 🐶
Suggested change
| // Returns the block size. When workgroup has non-uniform block, this returns size of the uniform | |
| // block. | |
| // Returns the block size. When workgroup has non-uniform block, this returns size of the | |
| // uniform block. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Motivation
Enable providing 3-dimensional launch params for kernels with an eye for ck tile integration.
Convenience overloads to match the previous 1d launches are added.
Technical Details
Changelog Category
Add a
CHANGELOG.mdentry for any option other thanNot Applicable