Skip to content

Conversation

@Flakebi
Copy link
Contributor

@Flakebi Flakebi commented Dec 14, 2025

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.

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

@rustbot
Copy link
Collaborator

rustbot commented Dec 14, 2025

r? @WaffleLapkin

rustbot has assigned @WaffleLapkin.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 14, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 14, 2025

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.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

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.
@WaffleLapkin
Copy link
Member

r? workingjubilee

As I'm completely missing context

@rustbot
Copy link
Collaborator

rustbot commented Dec 15, 2025

workingjubilee is currently at their maximum review capacity.
They may take a while to respond.

Copy link
Member

@workingjubilee workingjubilee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AST-level code looks good.

Some details on messaging here. I'm not committed to a precise message on these, which is why it's a bit "multiple choice" here, just wondering if these could be improved. In one or two cases it is a must-change.

Should we be enforcing a maximum number of arguments, also? Probably not if there's no cross-driver consensus on that, but maybe?

View changes since this review

functional record update syntax requires a struct
hir_typeck_gpu_kernel_abi_cannot_be_called =
functions with the "gpu-kernel" ABI cannot be called
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we be more specific?

Suggested change
functions with the "gpu-kernel" ABI cannot be called
functions with the "gpu-kernel" ABI cannot be called from Rust
Suggested change
functions with the "gpu-kernel" ABI cannot be called
functions with the "gpu-kernel" ABI cannot be called, even in device code

hir_typeck_gpu_kernel_abi_cannot_be_called =
functions with the "gpu-kernel" ABI cannot be called
.note = an `extern "gpu-kernel"` function can only be launched on the GPU through an API
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure "can" is right? Maybe "should"? And "an API" is somewhat vague, what kind of API? Surely not a standard library one, for instance...

Suggested change
.note = an `extern "gpu-kernel"` function can only be launched on the GPU through an API
.note = an `extern "gpu-kernel"` function should only be launched on the GPU through a driver's API

Or "must"?

...I don't know if "kernel loader" is a real term for the thing that the GPU driver "does", it just came up intuitively while trying to grasp for words for the thing.

Suggested change
.note = an `extern "gpu-kernel"` function can only be launched on the GPU through an API
.note = an `extern "gpu-kernel"` function must only be launched on the GPU by the kernel loader

Comment on lines +48 to +49
declare_lint! {
/// The `missing_gpu_kernel_export_name` lint detects `gpu-kernel` functions that have a mangled name.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And we can't simply use an unmangled name by default because that would be unsafe, right.


/// `ImproperGpuKernelLint` checks `gpu-kernel` function definitions:
///
/// - `extern "gpu-kernel" fn` arguments should be simple.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should avoid "simple", even in a concise format like this: we mean "primitive types".

Suggested change
/// - `extern "gpu-kernel" fn` arguments should be simple.
/// - `extern "gpu-kernel" fn` arguments should be primitive types.

/// This lint is issued when it detects a probable mistake in a signature.
IMPROPER_GPU_KERNEL_ARG,
Warn,
"simple arguments of gpu-kernel functions"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line should capture the reason for the lint, not what it is checking for, so something like

Suggested change
"simple arguments of gpu-kernel functions"
"GPU kernel entry points have a limited calling convention"

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 16, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 16, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants