c-variadic: make `va_arg` match on `Arch` exhaustive by folkertdev · Pull Request #150831 · rust-lang/rust

folkertdev · 2026-01-08T20:08:07Z

tracking issue: #44930

Continuing from #150094, the more annoying cases remain. These are mostly very niche targets without Clang va_arg implementations, and so it might just be easier to defer to LLVM instead of us getting the ABI subtly wrong. That does mean we cannot stabilize c-variadic on those targets I think.

Alternatively we could ask target maintainers to contribute an implementation. I'd honestly prefer they make that change to LVM though (likely by just using CodeGen::emitVoidPtrVAArg) that we can mirror.

r? @workingjubilee

rustbot · 2026-01-08T20:08:12Z

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

folkertdev · 2026-01-08T20:09:20Z

compiler/rustc_codegen_llvm/src/va_arg.rs

-        Arch::LoongArch32 => emit_ptr_va_arg(
+        Arch::RiscV32 if target.abi == Abi::Ilp32e => {
+            // FIXME: clang manually adjusts the alignment for this ABI. It notes:
+            //
+            // > To be compatible with GCC's behaviors, we force arguments with
+            // > 2×XLEN-bit alignment and size at most 2×XLEN bits like `long long`,
+            // > `unsigned long long` and `double` to have 4-byte alignment. This
+            // > behavior may be changed when RV32E/ILP32E is ratified.
+            bx.va_arg(addr.immediate(), bx.cx.layout_of(target_ty).llvm_type(bx.cx))
+        }
+        Arch::RiscV32 | Arch::LoongArch32 => emit_ptr_va_arg(


https://github.com/llvm/llvm-project/blob/289a3292be0c6a3df86bcdf5be7dd05b79a5570c/clang/lib/CodeGen/Targets/RISCV.cpp#L756

cc @almindor @dkhayes117 @romancardenas @MabezDev @jessebraham @rmsyn

This special case for Ilp32e is unfortunate. Can any of you shine any light on this?

folkertdev · 2026-01-08T20:09:58Z

compiler/rustc_codegen_llvm/src/va_arg.rs

+        Arch::Sparc64 => emit_ptr_va_arg(
+            bx,
+            addr,
+            target_ty,
+            if target_ty_size > 2 * 8 { PassMode::Indirect } else { PassMode::Direct },
+            SlotSize::Bytes8,
+            AllowHigherAlign::Yes,
+            ForceRightAdjust::No,
+        ),


https://github.com/llvm/llvm-project/blob/289a3292be0c6a3df86bcdf5be7dd05b79a5570c/clang/lib/CodeGen/Targets/Sparc.cpp#L327

cc @jonathanpallant

apparently there is no actual sparc64 rust target? just sparc is handled below.

sparc64-unknown-linux-gnu?

Oh right that does exist but does not get a page here

https://doc.rust-lang.org/beta/rustc/platform-support.html

and therefore apparently does not have a target maintainer?

Correct, afaik.

Oh, also more specifically: "sparcv9" is a somewhat obfuscated (or "more correct", if you prefer) way of saying "sparc64": https://doc.rust-lang.org/nightly/rustc/platform-support/solaris.html

cc @psumbera @kulikjak

Yes, sparcv9-sun-solaris is 64 bit. Is there anything we should check?

github renders weirdly, there is some context in #150831 (review)

folkertdev · 2026-01-08T20:11:54Z

compiler/rustc_codegen_llvm/src/va_arg.rs

+        Arch::SpirV => bug!("spirv does not support c-variadic functions"),
+
+        Arch::Mips | Arch::Mips32r6 | Arch::Mips64 | Arch::Mips64r6 => {
+            // FIXME: port MipsTargetLowering::lowerVAARG.


The implementation is at https://github.com/llvm/llvm-project/blob/289a3292be0c6a3df86bcdf5be7dd05b79a5570c/llvm/lib/Target/Mips/MipsISelLowering.cpp#L2338

I suspect that is ultimately just emitVoidPtrVAArg but it's hard to tell.

cc @Gelbpunkt

Do you have any insights here? Perhaps we could make a PR to LLVM to clean things up?

folkertdev · 2026-01-08T20:24:26Z

compiler/rustc_codegen_llvm/src/va_arg.rs

+        Arch::Sparc | Arch::Avr | Arch::M68k | Arch::Msp430 => {
+            // Clang uses the LLVM implementation for these architectures.
+            bx.va_arg(addr.immediate(), bx.cx.layout_of(target_ty).llvm_type(bx.cx))
+        }


That implementation is via DefaultABIInfo::EmitVAArg

https://github.com/llvm/llvm-project/blob/c379a88c09af4657d609888a46ada7719b2c9da9/clang/lib/CodeGen/ABIInfoImpl.cpp#L77-L83

To EmitVAArgInstr

https://github.com/llvm/llvm-project/blob/c379a88c09af4657d609888a46ada7719b2c9da9/clang/lib/CodeGen/ABIInfoImpl.cpp#L404

To the LLVM CreateVAArg I believe this will eventually call expandVAArg

https://github.com/llvm/llvm-project/blob/21a1e6e6a70d70635c68b9aaa54f816ae36a6416/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp#L2515

That really looks like emitVoidPtrVAArg to me, but it's hard to be completely sure.

rust-bors · 2026-01-15T01:01:25Z

☔ The latest upstream changes (presumably #151144) made this pull request unmergeable. Please resolve the merge conflicts.

folkertdev

Hello target maintainers, just making sure that you all see this. Overall we're just copying what clang does, but let us know if there is anything that doesn't look right or if you have useful background information. Otherwise maybe just thumbs-up the message to indicate that it looks good.

Some context you might be missing: I'm trying to stabilize c-variadic function definitions in rust. Historically, just relying on LLVM has been fragile, so we're replicating what clang does in this file. Doing (some of) our own codegen means we can make stronger promises about the exact behavior.

By making this match exhaustive, we'll know that we've at least considered all of the different targets.

View changes since this review

folkertdev · 2026-01-25T19:11:28Z

compiler/rustc_codegen_llvm/src/va_arg.rs

-        Arch::LoongArch32 => emit_ptr_va_arg(
+        Arch::RiscV32 if target.abi == Abi::Ilp32e => {
+            // FIXME: clang manually adjusts the alignment for this ABI. It notes:
+            //
+            // > To be compatible with GCC's behaviors, we force arguments with
+            // > 2×XLEN-bit alignment and size at most 2×XLEN bits like `long long`,
+            // > `unsigned long long` and `double` to have 4-byte alignment. This
+            // > behavior may be changed when RV32E/ILP32E is ratified.
+            bx.va_arg(addr.immediate(), bx.cx.layout_of(target_ty).llvm_type(bx.cx))
+        }
+        Arch::RiscV32 | Arch::LoongArch32 => emit_ptr_va_arg(


cc @almindor @dkhayes117 @romancardenas @MabezDev @jessebraham @rmsyn

This special case for Ilp32e is unfortunate. Can any of you shine any light on this?

folkertdev · 2026-01-25T19:13:21Z

compiler/rustc_codegen_llvm/src/va_arg.rs

+        Arch::SpirV => bug!("spirv does not support c-variadic functions"),
+
+        Arch::Mips | Arch::Mips32r6 | Arch::Mips64 | Arch::Mips64r6 => {
+            // FIXME: port MipsTargetLowering::lowerVAARG.


cc @Gelbpunkt

Do you have any insights here? Perhaps we could make a PR to LLVM to clean things up?

folkertdev · 2026-01-25T19:14:20Z

compiler/rustc_codegen_llvm/src/va_arg.rs

+        Arch::Sparc64 => emit_ptr_va_arg(
+            bx,
+            addr,
+            target_ty,
+            if target_ty_size > 2 * 8 { PassMode::Indirect } else { PassMode::Direct },
+            SlotSize::Bytes8,
+            AllowHigherAlign::Yes,
+            ForceRightAdjust::No,
+        ),


cc @jonathanpallant

apparently there is no actual sparc64 rust target? just sparc is handled below.

workingjubilee · 2026-02-04T21:04:17Z

@folkertdev 10 days, that's, eh, an FCP length? :^) Let's error on the RV32E case (that's RiscV32 && Ilp32e) until it becomes clearer what is going on there, and then I think we can accept this implementation for everyone else.

workingjubilee · 2026-02-04T21:26:54Z

@rustbot author

rustbot · 2026-02-04T21:26:59Z

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

rustbot · 2026-02-04T23:41:31Z

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.

folkertdev · 2026-02-04T23:45:04Z

CI willing

@rustbot ready

workingjubilee · 2026-02-05T02:53:26Z

Once more into the breach! Good luck!

@bors r+

rust-bors · 2026-02-05T02:53:29Z

📌 Commit d2b5ba2 has been approved by workingjubilee

It is now in the queue for this repository.

…uwer Rollup of 9 pull requests Successful merges: - #150831 (c-variadic: make `va_arg` match on `Arch` exhaustive) - #152113 (Fix GitHub CI summary in CodeBuild) - #152153 (Incorporate query description functions into `QueryVTable`) - #152070 (Convert to inline diagnostics in `rustc_pattern_analysis`) - #152106 (Convert to inline diagnostics in `rustc_ast_passes`) - #152109 (Convert to inline diagnostics in `rustc_errors`) - #152119 (Convert to inline diagnostics in `rustc_middle`) - #152121 (Convert to inline diagnostics in `rustc_builtin_macros`) - #152133 (library/std: Rename `ON_BROKEN_PIPE_FLAG_USED` to `ON_BROKEN_PIPE_USED`) Failed merges: - #152107 (Convert to inline diagnostics in `rustc_borrowck`) - #152117 (Convert to inline diagnostics in `rustc_trait_selection`) - #152126 (Convert to inline diagnostics in `rustc_mir_build`) - #152131 (Port rustc_no_implicit_bounds attribute to parser.)

@workingjubilee

Rollup merge of #150831 - folkertdev:more-va-arg-2, r=workingjubilee c-variadic: make `va_arg` match on `Arch` exhaustive tracking issue: #44930 Continuing from #150094, the more annoying cases remain. These are mostly very niche targets without Clang `va_arg` implementations, and so it might just be easier to defer to LLVM instead of us getting the ABI subtly wrong. That does mean we cannot stabilize c-variadic on those targets I think. Alternatively we could ask target maintainers to contribute an implementation. I'd honestly prefer they make that change to LVM though (likely by just using `CodeGen::emitVoidPtrVAArg`) that we can mirror. r? @workingjubilee

folkertdev added the F-c_variadic `#![feature(c_variadic)]` label Jan 8, 2026

rustbot assigned workingjubilee Jan 8, 2026

folkertdev commented Jan 8, 2026

View reviewed changes

This comment has been minimized.

Sign in to view

folkertdev force-pushed the more-va-arg-2 branch from f6f9040 to b388d92 Compare January 8, 2026 21:39

folkertdev marked this pull request as ready for review January 9, 2026 21:08

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 9, 2026

folkertdev force-pushed the more-va-arg-2 branch from b388d92 to 1d7b6e2 Compare January 16, 2026 19:05

This comment has been minimized.

Sign in to view

folkertdev commented Jan 25, 2026

View reviewed changes

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 Feb 4, 2026

folkertdev added 2 commits February 5, 2026 00:38

c-variadic: minor cleanups of va_arg

aa0ce23

c-variadic: make va_arg match on Arch exhaustive

d2b5ba2

folkertdev force-pushed the more-va-arg-2 branch from 1d7b6e2 to d2b5ba2 Compare February 4, 2026 23:41

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Feb 4, 2026

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 Feb 5, 2026

JonathanBrouwer mentioned this pull request Feb 5, 2026

Rollup of 9 pull requests #152163

Merged

rust-bors bot merged commit 82b5849 into rust-lang:main Feb 5, 2026
11 checks passed

rustbot added this to the 1.95.0 milestone Feb 5, 2026

Uh oh!

Conversation

folkertdev commented Jan 8, 2026

Uh oh!

rustbot commented Jan 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

workingjubilee Jan 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

rust-bors bot commented Jan 15, 2026

Uh oh!

This comment has been minimized.

folkertdev left a comment • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

workingjubilee commented Feb 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

workingjubilee commented Feb 4, 2026

Uh oh!

rustbot commented Feb 4, 2026

Uh oh!

rustbot commented Feb 4, 2026

Uh oh!

folkertdev commented Feb 4, 2026

Uh oh!

workingjubilee commented Feb 5, 2026

Uh oh!

rust-bors bot commented Feb 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

workingjubilee Jan 26, 2026 •

edited

Loading

folkertdev left a comment •

edited by rustbot

Loading

workingjubilee commented Feb 4, 2026 •

edited

Loading