implement va_arg for powerpc
#141622
Merged
implement va_arg for powerpc
#141622
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
|
cc @programmerjake I don't know if you give a damn but |
This comment was marked as resolved.
This comment was marked as resolved.
matthiaskrgr
added a commit
to matthiaskrgr/rust
that referenced
this pull request
May 27, 2025
…r=workingjubilee use custom types to clarify arguments to `emit_ptr_va_arg` tracking issue: rust-lang#44930 split out of rust-lang#141622 r? `@workingjubilee` `@rustbot` label: +F-c_variadic
folkertdev
force-pushed
the
powerpc-va_arg
branch
from
1607312 to
2259fcc
Compare
This comment has been minimized.
This comment has been minimized.
compiler-errors
added a commit
to compiler-errors/rust
that referenced
this pull request
May 27, 2025
…r=workingjubilee use custom types to clarify arguments to `emit_ptr_va_arg` tracking issue: rust-lang#44930 split out of rust-lang#141622 r? ``@workingjubilee`` ``@rustbot`` label: +F-c_variadic
rust-timer
added a commit
that referenced
this pull request
May 27, 2025
folkertdev
force-pushed
the
powerpc-va_arg
branch
from
2259fcc to
55e7d9f
Compare
|
☔ The latest upstream changes (presumably #141753) made this pull request unmergeable. Please resolve the merge conflicts. |
folkertdev
force-pushed
the
powerpc-va_arg
branch
from
55e7d9f to
3cf63b6
Compare
rustbot
added
the
A-LLVM
|
r=me with the @bors delegate+ |
|
✌️ @folkertdev, you can now approve this pull request! If @workingjubilee told you to " |
workingjubilee
added
S-waiting-on-author
folkertdev
force-pushed
the
powerpc-va_arg
branch
from
3cf63b6 to
ce7919f
Compare
This actually fixes a bug where before only 20 arguments could be passed. As far as I can tell, an arbitrary number of arguments is now supported
folkertdev
force-pushed
the
powerpc-va_arg
branch
from
ce7919f to
be13ce3
Compare
bors
added
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
Jun 1, 2025
Rollup of 9 pull requests Successful merges: - #140370 (Improve diagnostics for usage of qualified paths within tuple struct exprs/pats) - #141224 (terminology: allocated object → allocation) - #141622 (implement `va_arg` for `powerpc`) - #141666 (source_span_for_markdown_range: fix utf8 violation) - #141789 (Exclude `CARGO_HOME` from `generate-copyright` in-tree determination) - #141823 (Drive-by refactor: use `OnceCell` for the reverse region SCC graph) - #141834 (Add unimplemented `current_dll_path()` for WASI) - #141846 (Fix TLS model on bootstrap for cygwin) - #141852 (resolve if-let-chain FIXME on bootstrap) r? `@ghost` `@rustbot` modify labels: rollup
rust-timer
added a commit
that referenced
this pull request
Jun 1, 2025
Rollup merge of #141622 - folkertdev:powerpc-va_arg, r=workingjubilee implement `va_arg` for `powerpc` tracking issue: #44930 The llvm `va_arg` implementation is well-known to have serious limitations. Some planned changes to rust's `VaList` make it much more likely that LLVM miscompiles `va_arg`, so this PR adds support for the various powerpc targets. Now at least the targets that `core` has explicit support for will continue to work. For `powerpc` (the 32-bit variant) this implementation also fixes a bug where only up to 20 variadic arguments were supported. Locally (with qemu), these targets now pass the tests in https://github.com/rust-lang/rust/blob/master/tests/run-make/c-link-to-rust-va-list-fn/checkrust.rs. That test does not actually run for the powerpc targets in CI though. The implementation is based on clang: - handling of big endian architectures https://github.com/llvm/llvm-project/blob/3c8089d1ea53232d5a7cdc33f0cb43ef7d6f723b/clang/lib/CodeGen/ABIInfoImpl.cpp#L191-L193 - 64-bit https://github.com/llvm/llvm-project/blob/3c8089d1ea53232d5a7cdc33f0cb43ef7d6f723b/clang/lib/CodeGen/Targets/PPC.cpp#L969 - 32-bit https://github.com/llvm/llvm-project/blob/3c8089d1ea53232d5a7cdc33f0cb43ef7d6f723b/clang/lib/CodeGen/Targets/PPC.cpp#L430 cc `@daltenty` (target maintainer) r? `@workingjubilee` `@rustbot` label: +F-c_variadic
github-actions bot
pushed a commit
to rust-lang/miri
that referenced
this pull request
Jun 3, 2025
Rollup of 9 pull requests Successful merges: - rust-lang/rust#140370 (Improve diagnostics for usage of qualified paths within tuple struct exprs/pats) - rust-lang/rust#141224 (terminology: allocated object → allocation) - rust-lang/rust#141622 (implement `va_arg` for `powerpc`) - rust-lang/rust#141666 (source_span_for_markdown_range: fix utf8 violation) - rust-lang/rust#141789 (Exclude `CARGO_HOME` from `generate-copyright` in-tree determination) - rust-lang/rust#141823 (Drive-by refactor: use `OnceCell` for the reverse region SCC graph) - rust-lang/rust#141834 (Add unimplemented `current_dll_path()` for WASI) - rust-lang/rust#141846 (Fix TLS model on bootstrap for cygwin) - rust-lang/rust#141852 (resolve if-let-chain FIXME on bootstrap) r? `@ghost` `@rustbot` modify labels: rollup
github-actions bot
pushed a commit
to model-checking/verify-rust-std
that referenced
this pull request
Jun 3, 2025
…llaumeGomez Rollup of 9 pull requests Successful merges: - rust-lang#140370 (Improve diagnostics for usage of qualified paths within tuple struct exprs/pats) - rust-lang#141224 (terminology: allocated object → allocation) - rust-lang#141622 (implement `va_arg` for `powerpc`) - rust-lang#141666 (source_span_for_markdown_range: fix utf8 violation) - rust-lang#141789 (Exclude `CARGO_HOME` from `generate-copyright` in-tree determination) - rust-lang#141823 (Drive-by refactor: use `OnceCell` for the reverse region SCC graph) - rust-lang#141834 (Add unimplemented `current_dll_path()` for WASI) - rust-lang#141846 (Fix TLS model on bootstrap for cygwin) - rust-lang#141852 (resolve if-let-chain FIXME on bootstrap) r? `@ghost` `@rustbot` modify labels: rollup
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
tracking issue: #44930
The llvm
va_argimplementation is well-known to have serious limitations. Some planned changes to rust'sVaListmake it much more likely that LLVM miscompilesva_arg, so this PR adds support for the various powerpc targets. Now at least the targets thatcorehas explicit support for will continue to work.For
powerpc(the 32-bit variant) this implementation also fixes a bug where only up to 20 variadic arguments were supported.Locally (with qemu), these targets now pass the tests in https://github.com/rust-lang/rust/blob/master/tests/run-make/c-link-to-rust-va-list-fn/checkrust.rs. That test does not actually run for the powerpc targets in CI though.
The implementation is based on clang:
cc @daltenty (target maintainer)
r? @workingjubilee
@rustbot label: +F-c_variadic