-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Properly test cross-language LTO #57438
Comments
I think we'll definitely want to only run these tests on an opt-in basis rather than by default, and then we'll just want to carefully make sure that CI is running these tests. We can probably just pick a builder that's already finishing pretty quickly and build clang on it from source (which LLDB uses as well), and rely on sccache to make things fast. |
Thanks, @alexcrichton! I should have a PR soon. |
compiletest: Support opt-in Clang-based run-make tests and use them for testing xLTO. Some cross-language run-make tests need a Clang compiler that matches the LLVM version of `rustc`. Since such a compiler usually isn't available these tests (marked with the `needs-matching-clang` directive) are ignored by default. For some CI jobs we do need these tests to run unconditionally though. In order to support this a `--force-clang-based-tests` flag is added to compiletest. If this flag is specified, `compiletest` will fail if it can't detect an appropriate version of Clang. @rust-lang/infra The PR doesn't yet enable the tests yet. Do you have any recommendation for which jobs to enable them? cc #57438 r? @alexcrichton
compiletest: Support opt-in Clang-based run-make tests and use them for testing xLTO. Some cross-language run-make tests need a Clang compiler that matches the LLVM version of `rustc`. Since such a compiler usually isn't available these tests (marked with the `needs-matching-clang` directive) are ignored by default. For some CI jobs we do need these tests to run unconditionally though. In order to support this a `--force-clang-based-tests` flag is added to compiletest. If this flag is specified, `compiletest` will fail if it can't detect an appropriate version of Clang. @rust-lang/infra The PR doesn't yet enable the tests yet. Do you have any recommendation for which jobs to enable them? cc #57438 r? @alexcrichton
compiletest: Support opt-in Clang-based run-make tests and use them for testing xLTO. Some cross-language run-make tests need a Clang compiler that matches the LLVM version of `rustc`. Since such a compiler usually isn't available these tests (marked with the `needs-matching-clang` directive) are ignored by default. For some CI jobs we do need these tests to run unconditionally though. In order to support this a `--force-clang-based-tests` flag is added to compiletest. If this flag is specified, `compiletest` will fail if it can't detect an appropriate version of Clang. @rust-lang/infra The PR doesn't yet enable the tests yet. Do you have any recommendation for which jobs to enable them? cc #57438 r? @alexcrichton
compiletest: Support opt-in Clang-based run-make tests and use them for testing xLTO. Some cross-language run-make tests need a Clang compiler that matches the LLVM version of `rustc`. Since such a compiler usually isn't available these tests (marked with the `needs-matching-clang` directive) are ignored by default. For some CI jobs we do need these tests to run unconditionally though. In order to support this a `--force-clang-based-tests` flag is added to compiletest. If this flag is specified, `compiletest` will fail if it can't detect an appropriate version of Clang. @rust-lang/infra The PR doesn't yet enable the tests yet. Do you have any recommendation for which jobs to enable them? cc #57438 r? @alexcrichton
compiletest: Support opt-in Clang-based run-make tests and use them for testing xLTO. Some cross-language run-make tests need a Clang compiler that matches the LLVM version of `rustc`. Since such a compiler usually isn't available these tests (marked with the `needs-matching-clang` directive) are ignored by default. For some CI jobs we do need these tests to run unconditionally though. In order to support this a `--force-clang-based-tests` flag is added to compiletest. If this flag is specified, `compiletest` will fail if it can't detect an appropriate version of Clang. @rust-lang/infra The PR doesn't yet enable the tests yet. Do you have any recommendation for which jobs to enable them? cc #57438 r? @alexcrichton
compiletest: Support opt-in Clang-based run-make tests and use them for testing xLTO. Some cross-language run-make tests need a Clang compiler that matches the LLVM version of `rustc`. Since such a compiler usually isn't available these tests (marked with the `needs-matching-clang` directive) are ignored by default. For some CI jobs we do need these tests to run unconditionally though. In order to support this a `--force-clang-based-tests` flag is added to compiletest. If this flag is specified, `compiletest` will fail if it can't detect an appropriate version of Clang. @rust-lang/infra The PR doesn't yet enable the tests yet. Do you have any recommendation for which jobs to enable them? cc #57438 r? @alexcrichton
compiletest: Support opt-in Clang-based run-make tests and use them for testing xLTO. Some cross-language run-make tests need a Clang compiler that matches the LLVM version of `rustc`. Since such a compiler usually isn't available these tests (marked with the `needs-matching-clang` directive) are ignored by default. For some CI jobs we do need these tests to run unconditionally though. In order to support this a `--force-clang-based-tests` flag is added to compiletest. If this flag is specified, `compiletest` will fail if it can't detect an appropriate version of Clang. @rust-lang/infra The PR doesn't yet enable the tests yet. Do you have any recommendation for which jobs to enable them? cc #57438 r? @alexcrichton
Implemented in #57514. |
@michaelwoerister Should we perhaps reopen this to track re-enabling a few of the tests on Windows? I believe you mentioned it wasn't super important to have that testing... but that might've been in reference to short term rather than long term. |
It's important to test it on some platform. It is a little less important to test it on all platforms. But yes, let's re-open so we don't forget. |
@rust-lang/infra Now that we've switched to a new CI vendor, do we have the resources to test xLTO on all tier 1 platforms? That would be really nice! |
We don't have 4 cores yet on Azure Pipelines, so we have even less capacity than before (a single build is now around 3h30m). |
OK, let's revisit at some later point in time then. |
I think we have more spare capacity on Windows builders now than we have in the past; would it still be useful to test this on Windows? If it hasn't failed since 2019 that maybe points to no ... but I think we should no longer need a fork of Clang, hopefully, so maybe we can test this without blowing up CI times too much? |
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:
rustc
and the foreign language compilers must roughly be the same (at least the same major version).-Zcross-lang-lto
inrustc
. 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-cpu
attribute 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-fulldeps
tests) that checks if a compatible Clang is available.compiletest
that allows to specify whether tests relying on Clang are optional or required.compiletest
option that requires Clang to be available.This way cross-language LTO will be tested by CI and hopefully
sccache
will 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.
The text was updated successfully, but these errors were encountered: