Skip to content
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

Add a test ensuring all our targets set the features required by their ABI. #136208

Open
RalfJung opened this issue Jan 28, 2025 · 4 comments
Open
Labels
A-ABI Area: Concerning the application binary interface (ABI) A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@RalfJung
Copy link
Member

RalfJung commented Jan 28, 2025

With #136147, we emit a warning (and hopefully eventually a hard error) when a target does not enable all the target features required by the declared ABI, or when it enables a target feature incompatible with the declared ABI. Ideally, we'd ensure that all built-in targets pass this test, but that is non-trivial: we need to actually generate an LLVM TargetMachine for the target and ask LLVM about the enabled target features and then check that against our ABI compatibility list. I'm not sure how to best write such a test.

It could be a ui test compiling an empty program, with a revision for each target, but that seems annoying... I'd prefer something that iterates over all targets automatically, rather than having to list all targets in some ui test.

Cc @workingjubilee @rust-lang/wg-llvm

@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Jan 28, 2025
@nikic
Copy link
Contributor

nikic commented Jan 28, 2025

We already have a tests for basic assembly support for all targets in https://github.com/rust-lang/rust/tree/master/tests/assembly/targets -- does that implicitly cover this check as well?

@RalfJung
Copy link
Member Author

I guess it will once these warnings become hard errors. Currently I presume warnings would be ignored for that test?

@nikic
Copy link
Contributor

nikic commented Jan 28, 2025

Possibly, I don't actually know what happens to warnings in assembly/codegen tests, I guess they just get ignored. We could slap a deny on it though :)

@RalfJung
Copy link
Member Author

This is a emit_warn, not a lint, so I actually don't know how to make it a hard error just for a particular file.

@jieyouxu jieyouxu added A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. A-ABI Area: Concerning the application binary interface (ABI) and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jan 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ABI Area: Concerning the application binary interface (ABI) A-testsuite Area: The testsuite used to check the correctness of rustc C-enhancement Category: An issue proposing an enhancement or a PR with one. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants