-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
Add checks for gpu-kernel calling conv #149991
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
|
rustbot has assigned @WaffleLapkin. Use |
b6cb0c2 to
0a9373c
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
0a9373c to
1e9b1dc
Compare
This comment has been minimized.
This comment has been minimized.
The `gpu-kernel` calling convention has several restrictions that were not enforced by the compiler until now. Add the following restrictions: 1. Cannot be async 2. Cannot be called 3. Cannot return values, return type must be `()` or `!` 4. Arguments should be simple, i.e. passed by value. More complicated types can work when you know what you are doing, but it is rather unintuitive, one needs to know ABI/compiler internals. 5. Export name should be unmangled, either through `no_mangle` or `export_name`. Kernels are searched by name on the CPU side, having a mangled name makes it hard to find and probably almost always unintentional.
1e9b1dc to
10b32d6
Compare
|
The job Click to see the possible cause of the failure (guessed by this bot) |
The
gpu-kernelcalling convention has several restrictions that were not enforced by the compiler until now.Add the following restrictions:
()or!no_mangleorexport_name. Kernels are searched by name on the CPU side, having a mangled name makes it hard to find and probably almost always unintentional.Tracking issue: #135467
amdgpu target tracking issue: #135024
@workingjubilee, these should be all the restrictions we talked about a year ago.
cc @RDambrosio016 @kjetilkjeka for nvptx