-
-
Notifications
You must be signed in to change notification settings - Fork 14.3k
Add amdgpu_dispatch_ptr intrinsic #150607
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
base: main
Are you sure you want to change the base?
Conversation
|
I can help with design review but not for the implementation, sorry. Regarding the design, if the return type is "erased" I would suggest using a raw pointer instead of a reference. |
|
yoink. If anyone wants to chip in on the review, please feel free, I just want to make sure I have a gander before it ships so I can keep vaguely abreast of what's happening in this space. |
library/core/src/intrinsics/mod.rs
Outdated
| #[rustc_intrinsic] | ||
| #[cfg(target_arch = "amdgpu")] | ||
| #[must_use = "returns a reference that does nothing unless used"] | ||
| pub fn amdgpu_dispatch_ptr() -> &'static (); |
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.
Maybe we should have a new "amdgpu" file or so for this, to keep it separate from the typically more portable intrinsics in the rest of this file?
Or a new "gpu" file that offload also goes into? I don't know what a sensible grouping here would look like.
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.
gpu.rs seems like a good place to start, to me, that way other gpu targets are encouraged to reuse code from amdgpu by generalizing it instead of repeating it (as we have discussed before, GPU targets are like siblings: they make much of their tiny differences, while being mostly similar).
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.
Glancing at NVPTX, it seems they achieve the same things as AMDGPU here by having special registers that are read, whereas AMDGPU uses the struct pointer, so at least for this case they will differ.
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.
( given their code is JITted by the device driver anyways, for all I know they both actually use the same pattern in the actual machine code they lower to. )
Add a rustc intrinsic `amdgpu_dispatch_ptr` to access the kernel dispatch packet on amdgpu. The HSA kernel dispatch packet contains important information like the launch size and workgroup size. The Rust intrinsic lowers to the `llvm.amdgcn.dispatch.ptr` LLVM intrinsic, which returns a `ptr addrspace(4)`, plus an addrspacecast to `addrspace(0)`, so it can be returned as a Rust reference. The returned pointer/reference is valid for the whole program lifetime, and is therefore `'static`. The return type of the intrinsic (`*const ()`) does not mention the struct so that rustc does not need to know the exact struct type. An alternative would be to define the struct as lang item or add a generic argument to the function. Short version: ```rust #[cfg(target_arch = "amdgpu")] pub fn amdgpu_dispatch_ptr() -> *const (); ```
1d98b13 to
13d7a3c
Compare
|
Thanks for the quick reviews! cc @ZuseZ4 FYI for the |
There is an ongoing discussion in #150452 about using address spaces from the Rust language in some way.
As that discussion will likely not conclude soon, this PR adds one rustc_intrinsic with an addrspacecast to unblock getting basic information like launch and workgroup size and make it possible to implement something like
core::gpu.Add a rustc intrinsic
amdgpu_dispatch_ptrto access the kernel dispatch packet on amdgpu.The HSA kernel dispatch packet contains important information like the launch size and workgroup size.
The Rust intrinsic lowers to the
llvm.amdgcn.dispatch.ptrLLVM intrinsic, which returns aptr addrspace(4), plus an addrspacecast toaddrspace(0), so it can be returned as a Rust reference.The returned pointer/reference is valid for the whole program lifetime, and is therefore
'static.The return type of the intrinsic (
&'static ()) does not mention the struct so that rustc does not need to know the exact struct type. An alternative would be to define the struct as lang item or add a generic argument to the function.Is this ok or is there a better way (also, should it return a pointer instead of a reference)?
Short version:
Tracking issue: #135024
r? RalfJung as you are already aware of the background (feel free to re-assign)