-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
Properly test cross-language LTO #57438
Copy link
Copy link
Open
Labels
A-linkageArea: linking into static, shared libraries and binariesArea: linking into static, shared libraries and binariesA-testsuiteArea: The testsuite used to check the correctness of rustcArea: The testsuite used to check the correctness of rustcE-help-wantedCall for participation: Help is requested to fix this issue.Call for participation: Help is requested to fix this issue.T-bootstrapRelevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.T-infraRelevant to the infrastructure team, which will review and decide on the PR/issue.Relevant to the infrastructure team, which will review and decide on the PR/issue.
Metadata
Metadata
Assignees
Labels
A-linkageArea: linking into static, shared libraries and binariesArea: linking into static, shared libraries and binariesA-testsuiteArea: The testsuite used to check the correctness of rustcArea: The testsuite used to check the correctness of rustcE-help-wantedCall for participation: Help is requested to fix this issue.Call for participation: Help is requested to fix this issue.T-bootstrapRelevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap)T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.T-infraRelevant to the infrastructure team, which will review and decide on the PR/issue.Relevant to the infrastructure team, which will review and decide on the PR/issue.
Type
Fields
Give feedbackNo fields configured for issues without a type.
I'd like us to stabilize cross-language LTO as soon as possible but before we can do that we need more robust tests. Some background:
rustcand the foreign language compilers must roughly be the same (at least the same major version).-Zcross-lang-ltoinrustc. It has some tests that make sure libraries indeed contain LLVM bitcode instead of object files. This is the bare-minimum requirement for cross-language LTO to work.However, there is a problem with this approach:
target-cpuattribute for LLVM functions. For now this seems to be sufficient but for future versions of LLVM additional requirements might emerge and break cross-language LTO in a silent way (i.e. everything still compiles but the expected optimizations aren't happening).So, to test the feature in a more robust way, we have to have a test that actually compiles some mixed Rust/C++ code and then verifies that function inlining across language boundaries was successful.
The good news is that trivial functions (e.g. ones that just return a constant integer) are reliably inlined, so we have a good way of checking whether inlining happens at all.
The bad news is that, since Rust's LLVM is ahead of stable Clang releases, we need to build our own Clang in order to get a compatible version. We already have Clang in tree because we need it for building Rust-enabled LLDB, but this is not the default.
So my current proposal for implementing these tests is:
run-make-fulldepstests) that checks if a compatible Clang is available.compiletestthat allows to specify whether tests relying on Clang are optional or required.compiletestoption that requires Clang to be available.This way cross-language LTO will be tested by CI and hopefully
sccachewill make building Clang+LLDB cheap enough. But we also rely on not forgetting to set the appropriate flags in CI images, which might cause things to silently go untested.@rust-lang/infra & @rust-lang/compiler, do you have any thoughts? Or a better, less complicated approach? It would be great if we could require Clang unconditionally but I'm afraid that that would be too much of a burden for people running tests locally.