-
Notifications
You must be signed in to change notification settings - Fork 13.4k
implement va_arg
for powerpc
#141622
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
implement va_arg
for powerpc
#141622
Conversation
cc @programmerjake I don't know if you give a damn but |
This comment was marked as resolved.
This comment was marked as resolved.
…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
1607312
to
2259fcc
Compare
This comment has been minimized.
This comment has been minimized.
…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
2259fcc
to
55e7d9f
Compare
☔ The latest upstream changes (presumably #141753) made this pull request unmergeable. Please resolve the merge conflicts. |
55e7d9f
to
3cf63b6
Compare
r=me with the @bors delegate+ |
✌️ @folkertdev, you can now approve this pull request! If @workingjubilee told you to " |
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
ce7919f
to
be13ce3
Compare
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
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
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
…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
tracking issue: #44930
The llvm
va_arg
implementation is well-known to have serious limitations. Some planned changes to rust'sVaList
make it much more likely that LLVM miscompilesva_arg
, so this PR adds support for the various powerpc targets. Now at least the targets thatcore
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:
cc @daltenty (target maintainer)
r? @workingjubilee
@rustbot label: +F-c_variadic