-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
c_variadic: provide our own va_arg implementation for more targets
#150094
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
|
|
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.
These are (or, should be) the straightforward cases where LLVM just calls emitVoidPtrVAArg.
| Arch::LoongArch32 => emit_ptr_va_arg( | ||
| bx, | ||
| addr, | ||
| target_ty, | ||
| if target_ty_size > 2 * 4 { PassMode::Indirect } else { PassMode::Direct }, | ||
| SlotSize::Bytes4, | ||
| AllowHigherAlign::Yes, | ||
| ForceRightAdjust::No, | ||
| ), | ||
| Arch::LoongArch64 => 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, | ||
| ), |
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.
zero-sized types can't reach this logic currently, but we could handle them. This logic is very similar for riscv, but it has an annoying exception for the 32-bit embedded ABI which lowers alignment for f64 and i64/u64 from 8 to 4. Apparently that may change as things are ratified.
| Arch::AmdGpu => emit_ptr_va_arg( | ||
| bx, | ||
| addr, | ||
| target_ty, | ||
| PassMode::Direct, | ||
| SlotSize::Bytes4, | ||
| AllowHigherAlign::No, | ||
| ForceRightAdjust::No, | ||
| ), |
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.
| Arch::Nvptx64 => emit_ptr_va_arg( | ||
| bx, | ||
| addr, | ||
| target_ty, | ||
| PassMode::Direct, | ||
| SlotSize::Bytes1, | ||
| AllowHigherAlign::Yes, | ||
| ForceRightAdjust::No, | ||
| ), |
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.
| Arch::Wasm32 | Arch::Wasm64 => emit_ptr_va_arg( | ||
| bx, | ||
| addr, | ||
| target_ty, | ||
| if layout.is_aggregate() || layout.is_zst() || layout.is_1zst() { | ||
| PassMode::Indirect | ||
| } else { | ||
| PassMode::Direct | ||
| }, | ||
| SlotSize::Bytes4, | ||
| AllowHigherAlign::Yes, | ||
| ForceRightAdjust::No, | ||
| ), |
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.
curiously the slot size is always 4, independent of whether wasm32 or wasm64 is used. I suppose that may change in the future if/when wasm64 gets more attention.
| Arch::CSky => emit_ptr_va_arg( | ||
| bx, | ||
| addr, | ||
| target_ty, | ||
| PassMode::Direct, | ||
| SlotSize::Bytes4, | ||
| AllowHigherAlign::Yes, | ||
| ForceRightAdjust::No, | ||
| ), |
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.
|
☔ The latest upstream changes (presumably #150089) made this pull request unmergeable. Please resolve the merge conflicts. |
1c516dc to
9d0af69
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. |
tracking issue: #44930
Provide our own implementations in order to guarantee the behavior of
va_arg. We will only be able to stabilizec_variadicon targets where we know and guarantee the properties ofva_arg.r? workingjubilee