-
-
Notifications
You must be signed in to change notification settings - Fork 14.2k
custom VaList layout for Hexagon
#149967
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
custom VaList layout for Hexagon
#149967
Conversation
|
|
Thanks for the PR - I will give it a look! |
Yeah - can do. |
Nice, the relevant test here is |
Thanks! It passes with the changes in androm3da@5e7eca0 - now sent as three PRs: #150008, #150009, #150010 |
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.
library/core/src/ffi/va_list.rs
Outdated
| // | ||
| // The LLVM `BuiltinVaListKind` enumerates the `va_list` variations that LLVM supports. |
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.
This is incorrect: this is clang's library code.
You are trying to leave a note here that says "you can look at this to see what kind of support a C compiler uses". So just do that.
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.
Sure, fixed that
2909b96 to
023f38f
Compare
|
Thanks. This otherwise seems fine! @bors r+ rollup |
…uwer Rollup of 11 pull requests Successful merges: - #147939 (Make `const BorrowMut` require `const Borrow` and make `const Fn` require `const FnMut`) - #149734 (Mirror GCC 9.5.0) - #149767 (Tidying up tests/ui/issues 33 tests [4/N]) - #149804 (chore: fix some minor issues in the comments) - #149967 (custom `VaList` layout for Hexagon) - #150025 (dont create unnecessary `DefId`s under mgca) - #150032 (Use annotate-snippet as default emitter on stable) - #150033 (Add try_as_dyn and try_as_dyn_mut) - #150042 (rustc-dev-guide subtree update) - #150063 (Remove deny of manual-let-else) - #150064 (std: io: error: Add comment for UEFI unpacked repr use) Failed merges: - #150044 (Avoid unhelpful suggestion when crate name is invalid) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of #149967 - folkertdev:va-list-hexagon, r=workingjubilee custom `VaList` layout for Hexagon I noticed while browsing LLVM source that we use an incorrect `VaList` definition for the musl hexagon target. relevant links - https://github.com/llvm/llvm-project/blob/0cdc1b6dd4a870fc41d4b15ad97e0001882aba58/clang/include/clang/Basic/TargetInfo.h#L333 - https://github.com/llvm/llvm-project/blob/0cdc1b6dd4a870fc41d4b15ad97e0001882aba58/clang/lib/CodeGen/Targets/Hexagon.cpp#L407-L417 cc target maintainer `@androm3da` can you confirm that this looks OK? In particular the `#[rustc_pass_indirectly_in_non_rustic_abis]` attribute is used to simulate pointer decay (like if the struct were wrapped in a 1-element array in C). The clang comment suggests that the Tag is wrapped in such a single-element array, but I haven't actually been able to confirm it. For stabilizing `c_variadic` (on the hexagon targets) we will also need a custom `va_arg` implementation to mirror the one in `clang` in [va_arg.rs](https://github.com/rust-lang/rust/blob/main/compiler/rustc_codegen_llvm/src/va_arg.rs). Would you be able to contribute one? r? `@workingjubilee`
…uwer Rollup of 11 pull requests Successful merges: - rust-lang/rust#147939 (Make `const BorrowMut` require `const Borrow` and make `const Fn` require `const FnMut`) - rust-lang/rust#149734 (Mirror GCC 9.5.0) - rust-lang/rust#149767 (Tidying up tests/ui/issues 33 tests [4/N]) - rust-lang/rust#149804 (chore: fix some minor issues in the comments) - rust-lang/rust#149967 (custom `VaList` layout for Hexagon) - rust-lang/rust#150025 (dont create unnecessary `DefId`s under mgca) - rust-lang/rust#150032 (Use annotate-snippet as default emitter on stable) - rust-lang/rust#150033 (Add try_as_dyn and try_as_dyn_mut) - rust-lang/rust#150042 (rustc-dev-guide subtree update) - rust-lang/rust#150063 (Remove deny of manual-let-else) - rust-lang/rust#150064 (std: io: error: Add comment for UEFI unpacked repr use) Failed merges: - rust-lang/rust#150044 (Avoid unhelpful suggestion when crate name is invalid) r? `@ghost` `@rustbot` modify labels: rollup
I noticed while browsing LLVM source that we use an incorrect
VaListdefinition for the musl hexagon target.relevant links
cc target maintainer @androm3da can you confirm that this looks OK? In particular the
#[rustc_pass_indirectly_in_non_rustic_abis]attribute is used to simulate pointer decay (like if the struct were wrapped in a 1-element array in C). The clang comment suggests that the Tag is wrapped in such a single-element array, but I haven't actually been able to confirm it.For stabilizing
c_variadic(on the hexagon targets) we will also need a customva_argimplementation to mirror the one inclangin va_arg.rs. Would you be able to contribute one?r? @workingjubilee