Properly implement the gpu-kernel ABI for amdgpu by Flakebi · Pull Request #162177 · rust-lang/rust

Flakebi · 2026-09-02T09:01:16Z

Add support to pass structs, arrays and vectors to amdgpu kernels.
Scalars and vectors are taken by value, aggregates are passed by byref
pointers. Structs containing a single scalar/vector are handled like
a scalar.

Judging from clang tests, nvptx seems to do somewhat the same, just
using byval instead of byref: https://github.com/llvm/llvm-project/blob/e4e18dba3d77f4a3eea58bcc9ccae5a5498ede7c/clang/test/CodeGenCUDA/kernel-args.cu

I tested a couple of the lit test signatures on real hardware and it
seems to work fine. Given the relatively simple implementation, I hope
this amount of testing is enough (the C calling convention seems like
a worse fit for Rust’s current ABI code, it’s still giving me headaches).

This adds two members to PassMode::Indirect.

address_space specifies the address space of an on_stack/byval or
by_ref pointer argument.

by_ref translates to LLVM’s byref, which is similar to on_stack/byval,
however, there is no extra copy made, the pointer may not point to the
stack but can point to some other address space, and the passed argument
should not be modified.

Both are used by the amdgpu target to implement the gpu-kernel
ABI.

Tracking issue for the gpu-kernel ABI: #135467
Tracking issue for the amdgpu target: #135024

If I read it correctly, I can’t notify the gpu-target group, so cc @kjetilkjeka, @kulst, @ZuseZ4, @workingjubilee

rustbot · 2026-09-02T09:01:20Z

rustc_codegen_cranelift is developed in its own repository. If possible, consider making this change to rust-lang/rustc_codegen_cranelift instead.

cc @bjorn3

This PR changes rustc_public

cc @oli-obk, @celinval, @ouz-a, @makai410

rustc_codegen_gcc is developed in its own repository. If possible, consider making this change to rust-lang/rustc_codegen_gcc instead.

cc @antoyo, @GuillaumeGomez

rustbot · 2026-09-02T09:01:23Z

r? @petrochenkov

rustbot has assigned @petrochenkov.
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

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: compiler
compiler expanded to 75 candidates
Random selection from 21 candidates

petrochenkov · 2026-09-02T10:24:22Z

@rustbot reroll

ZuseZ4 · 2026-09-02T12:03:29Z

I think this now sends our slices through the aggregate path, not the (Scalar)Pair anymore, can you add a test to confirm that? It would break Rust Offload, but for now we can make a PR to overwrite this change for functions that are a OFFLOAD_KERNEL. So if this otherwise more closely matches hip/clang, then I think that's still an improvement. With the planned change from libomptarget to Offload APIs Rust offload should also become more flexible and able to handle this directly, but I'll check.

Flakebi · 2026-09-02T15:21:11Z

Yes, scalar pair (and therefore all fat pointers, including slices) are handled as aggregates with this and passed as byref ptr.

Are you sure that this breaks Rust Offload?
The way arguments are passed from the CPU side is equivalent between direct values and byref ptr, it’s all directly stored in the argument buffer, without pointer indirections. So, no changes required on the CPU side to pass slices.
My guess/hope is that this does not break anything but only fixes things :)
(A case that breaks is if someone hackily passed real structs before by adding them as pointer in the CPU argument memory. I hope nobody did that and expects it to keep working…)

I will add a test that passes a slice.

Flakebi · 2026-09-03T07:27:18Z

Pre-committed the tests, fixed the now perma-link in the commit message and added a test taking a slice as argument. Total diff (just adding the slice test): https://github.com/rust-lang/rust/compare/371ff93ae5d9f8cbbc07c15451218ecbeab39a9c..b5b814cbbf6c93535e33ce4232f698570dec25ce

Flakebi · 2026-09-03T07:35:25Z

Sorry, one more force-push to fix the tests that failed in CI (amended in the first commit). Just --blessing them was enough to add the new PassMode::Indirect members in the stderr output. Diff: https://github.com/rust-lang/rust/compare/6a47e13cce5e4608e10e26d1bec4b5af6baba10f..ddd9a73295fbd2136470a8d0938a7fb565b6e0c5

Flakebi · 2026-09-03T08:27:23Z

And one more to fix the aarch64-only variant of the test. Diff: https://github.com/rust-lang/rust/compare/ddd9a73295fbd2136470a8d0938a7fb565b6e0c5..a90629b7e87febc01a7e6bd4c56bba2232472c05

mejrs · 2026-09-06T21:22:06Z

r? @ZuseZ4

bjorn3 · 2026-09-09T09:52:54Z

Is byref ptr different from ptr in any way other than byref ptr having more UB? Also please make sure to add a copy of byref ptr parameters. Rustc assumes that it can write through ptr arguments, but byref ptr makes that UB.

Flakebi · 2026-09-09T11:17:38Z

Is byref ptr different from ptr in any way other than byref ptr having more UB?

Yes, byref ptr (and byval ptr) are both different from ptr.

On the calling side, a ptr would be passed in as a pointer.
However, a byref ptr is passed in as a value.

Arguably, there’s not much difference in the called gpu-kernel itself if you look just at the IR going into LLVM. The argument is handled like a pointer. Later in the backend part that lowers arguments to reads from the argument memory region, byref ptr is handled differently than ptr, either reading a pointer from the argument memory, or reading the value directly.

For gpu-kernel, this is observable in “user code”, i.e. outside the compiler, because the user assembles a memory region to pass as arguments when launching a gpu-kernel on the GPU through some API (cuda/hip/hsa/sycl).
A ptr argument means one needs to write a pointer to that argument memory region.
A byref ptr means one needs to write the passed value into the argument memory region (i.e. no double indirection).

So, if the function signature in Rust is extern "gpu-kernel" fn mykernel(p: *const Struct), the user should write a pointer into the argument memory region and the rustc should use ptr.
If the function signature in Rust is extern "gpu-kernel" fn mykernel(s: Struct), the user should write the struct into the argument memory region and the rustc should use byref ptr.

Also please make sure to add a copy of byref ptr parameters. Rustc assumes that it can write through ptr arguments, but byref ptr makes that UB.

I think the change in compiler/rustc_codegen_ssa/src/mir/mod.rs should take care of that

bjorn3 · 2026-09-09T11:40:12Z

However, a byref ptr is passed in as a value.

That would be byval, right? On x86_64 there is absolutely no difference in emitted assembly between ptr and say ptr byref(<128 x i8>): https://rust.godbolt.org/z/T5joGfWWE

I think the change in compiler/rustc_codegen_ssa/src/mir/mod.rs should take care of that

👍

Flakebi · 2026-09-11T08:34:17Z

Renamed the enum variant to AmdgpuKernelArg (diff).

bjorn3 · 2026-09-11T09:17:19Z

            continue;
        }
-        classify_arg(cx, arg);
+        if fn_abi.conv == CanonAbi::GpuKernel {


You are no longer respecting pass_indirectly_in_non_rustic_abis for extern "C". Also is this target supposed to be able to link against existing C code?

View changes since the review

Also is this target supposed to be able to link against existing C code?

No, currently not.
I started implementing towards that but that is a much larger problem than the gpu-kernel ABI and will likely require larger additions to rustc’s ABI handling (concretely, passing a repr(C) struct by value needs to be an LLVM IR struct that is passed by value and I think we need the correct types there, so using the current cast does not work).

rustbot · 2026-09-17T07:28:54Z

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.

Flakebi · 2026-09-17T07:31:00Z

Rebased to fix merge conflicts. The only meaningful change are the comments on IndirectMode/PassMode seen in the second range-diff (the rustbot comment above this one).

Both will be used by the amdgpu target to implement the `gpu-kernel` ABI. `address_space` specifies the address space of an indirect argument. `AmdgpuKernelArg` translates to LLVM’s byref, which is similar to on_stack/byval, however, there is no extra copy made, the pointer may not point to the stack but can point to some other address space, and the passed argument should not be modified. byval and byref are mutually exclusive, so change on_stack to an enum with the new states, Pointer (none), OnStack and AmdgpuKernelArg.

Add support to pass structs, arrays and vectors to amdgpu kernels. Scalars and vectors are taken by value, aggregates are passed by byref pointers. Structs containing a single scalar/vector are handled like a scalar. Judging from clang tests, nvptx seems to do somewhat the same, just using byval instead of byref: https://github.com/llvm/llvm-project/blob/3a8affeef4da19d39191aac316e189eca3214a8c/clang/test/CodeGenCUDA/kernel-args.cu I tested a couple of the lit test signatures on real hardware and it seems to work fine. Given the relatively simple implementation, I hope this amount of testing is enough (the C calling convention seems like a worse fit for Rust’s current ABI code, it’s still giving me headaches).

bjorn3 · 2026-09-17T10:29:06Z

@bors r+

rust-bors · 2026-09-17T10:29:09Z

📌 Commit 0c57a55 has been approved by bjorn3

It is now in the queue for this repository.

Properly implement the gpu-kernel ABI for amdgpu Add support to pass structs, arrays and vectors to amdgpu kernels. Scalars and vectors are taken by value, aggregates are passed by byref pointers. Structs containing a single scalar/vector are handled like a scalar. Judging from clang tests, nvptx seems to do somewhat the same, just using byval instead of byref: https://github.com/llvm/llvm-project/blob/e4e18dba3d77f4a3eea58bcc9ccae5a5498ede7c/clang/test/CodeGenCUDA/kernel-args.cu I tested a couple of the lit test signatures on real hardware and it seems to work fine. Given the relatively simple implementation, I hope this amount of testing is enough (the C calling convention seems like a worse fit for Rust’s current ABI code, it’s still giving me headaches). This adds two members to `PassMode::Indirect`. `address_space` specifies the address space of an on_stack/byval or by_ref pointer argument. `by_ref` translates to LLVM’s byref, which is similar to on_stack/byval, however, there is no extra copy made, the pointer may not point to the stack but can point to some other address space, and the passed argument should not be modified. Both are used by the amdgpu target to implement the `gpu-kernel` ABI. Tracking issue for the `gpu-kernel` ABI: rust-lang#135467 Tracking issue for the amdgpu target: rust-lang#135024

…uwer Rollup of 24 pull requests Successful merges: - #161596 (coretests: Add more pattern tests.) - #162177 (Properly implement the gpu-kernel ABI for amdgpu) - #162411 (Make Receiver `#[rustc_dyn_incompatible_trait]`) - #162760 (yeet alias new_from_def_id) - #162796 (libtest: do not early exit from test runners) - #162844 (Add loan reachability traces to polonius MIR dumps) - #162876 (Move operations out of `rustc_middle::query::job`) - #160108 (Stabilize `windows_process_extensions_main_thread_handle`) - #160212 (traits: Fix rigid alias liveness matching) - #160544 (Stabilize `feature(trim_prefix_suffix)` (`{str, [T], Path}::trim_prefix` and `{str, [T]}::trim_suffix`)) - #161305 (Use the entire type of a dropped local to compute variance (edge direction) for Polonius alpha) - #161838 (tests: accept LLVM 24 optimization in this test) - #162312 (core: Rewrite docs for try_as_dyn) - #162785 (Avoid creating overlapping assignments in MatchBranchSimplification) - #162805 (Add `must_use` lint to `ExitCode`) - #162825 (core: Add examples for `debug_closure_helpers`) - #162841 (enable asm tests for xtensa targets) - #162842 (reintroduce check RibKind::ConstParamTy did in direct consts) - #162845 (mgca: fix issue with mismatched array valtree/valtree tys) - #162856 (Stabilize CommandExt::show_window) - #162865 (Complex conjugate, negation and default) - #162874 (Add support for `annotate_snippets::snippet::AnnotationKind::Visible`) - #162881 (Simplify the macro for forwarding Decoder methods ) - #162888 (Fix a typo on the Armv7-R platform docs page)

lqd · 2026-09-17T14:48:49Z

Likely cause of the failure in #162893 (comment) on the test-x86_64-gnu-nopt builder.

JonathanBrouwer · 2026-09-17T14:50:15Z

💔 I suspect this PR failed tests as part of a rollup
@bors r-

After fixing the problem, consider running a try job for the failed job before re-approving.

Link to failure: #162893 (comment)

rust-bors · 2026-09-17T14:50:24Z

This pull request was unapproved.

This PR was contained in a rollup (#162893), which was unapproved.

View changes since this unapproval

rustbot assigned petrochenkov Sep 2, 2026

This comment has been minimized.

Sign in to view

bjorn3 reviewed Sep 2, 2026

View reviewed changes

Comment thread compiler/rustc_target/src/callconv/mod.rs Outdated

rustbot assigned mejrs and unassigned petrochenkov Sep 2, 2026

Flakebi force-pushed the amdgpu-kernel-cc branch from 371ff93 to b5b814c Compare September 3, 2026 07:24

This comment has been minimized.

Sign in to view

Flakebi force-pushed the amdgpu-kernel-cc branch from b5b814c to 6a47e13 Compare September 3, 2026 07:26

Flakebi force-pushed the amdgpu-kernel-cc branch from 6a47e13 to ddd9a73 Compare September 3, 2026 07:33

This comment has been minimized.

Sign in to view

Flakebi force-pushed the amdgpu-kernel-cc branch from ddd9a73 to a90629b Compare September 3, 2026 08:26

rustbot assigned ZuseZ4 and unassigned mejrs Sep 6, 2026

bjorn3 reviewed Sep 9, 2026

View reviewed changes

Comment thread compiler/rustc_codegen_ssa/src/mir/mod.rs

bjorn3 reviewed Sep 11, 2026

View reviewed changes

Comment thread compiler/rustc_target/src/callconv/amdgpu.rs Outdated

bjorn3 reviewed Sep 11, 2026

View reviewed changes

Comment thread compiler/rustc_target/src/callconv/amdgpu.rs Outdated

bjorn3 reviewed Sep 11, 2026

View reviewed changes

Comment thread compiler/rustc_target/src/callconv/mod.rs

Flakebi force-pushed the amdgpu-kernel-cc branch from 552358c to 87f5315 Compare September 15, 2026 08:33

This comment has been minimized.

Sign in to view

bjorn3 approved these changes Sep 16, 2026

View reviewed changes

Flakebi force-pushed the amdgpu-kernel-cc branch from 87f5315 to cf09bf3 Compare September 17, 2026 07:19

This comment has been minimized.

Sign in to view

Flakebi force-pushed the amdgpu-kernel-cc branch from cf09bf3 to d2d9b88 Compare September 17, 2026 07:28

This comment has been minimized.

Sign in to view

Flakebi added 3 commits September 17, 2026 10:49

Pre-commit amdgpu gpu-kernel ABI test

1040915

Flakebi force-pushed the amdgpu-kernel-cc branch from d2d9b88 to 0c57a55 Compare September 17, 2026 08:50

rust-bors Bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 17, 2026

JonathanBrouwer mentioned this pull request Sep 17, 2026

Rollup of 24 pull requests #162893

Closed

rust-bors Bot 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 17, 2026

Uh oh!

Conversation

Flakebi commented Sep 2, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Sep 2, 2026

Uh oh!

rustbot commented Sep 2, 2026

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

petrochenkov commented Sep 2, 2026

Uh oh!

ZuseZ4 commented Sep 2, 2026

Uh oh!

Flakebi commented Sep 2, 2026

Uh oh!

This comment has been minimized.

Flakebi commented Sep 3, 2026

Uh oh!

Flakebi commented Sep 3, 2026

Uh oh!

This comment has been minimized.

Flakebi commented Sep 3, 2026

Uh oh!

mejrs commented Sep 6, 2026

Uh oh!

bjorn3 commented Sep 9, 2026

Uh oh!

Flakebi commented Sep 9, 2026

Uh oh!

Uh oh!

bjorn3 commented Sep 9, 2026

Uh oh!

Flakebi commented Sep 11, 2026

Uh oh!

Uh oh!

Uh oh!

bjorn3 Sep 11, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Flakebi Sep 11, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

rustbot commented Sep 17, 2026

Uh oh!

Flakebi commented Sep 17, 2026

Uh oh!

This comment has been minimized.

bjorn3 commented Sep 17, 2026

Uh oh!

rust-bors Bot commented Sep 17, 2026

Uh oh!

lqd commented Sep 17, 2026

Uh oh!

JonathanBrouwer commented Sep 17, 2026

Uh oh!

rust-bors Bot commented Sep 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

Flakebi commented Sep 2, 2026 •

edited by rustbot

Loading

bjorn3 Sep 11, 2026 •

edited by rustbot

Loading