-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Use target-agnostic LLD flags in bootstrap for use-lld
#139378
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
Conversation
After this fix, we might want to revert 3d69dd1, although I'm not sure if it was a good idea to pass LLD flags to compiletest to use for compiling the test programs in the first place... |
This comment has been minimized.
This comment has been minimized.
use-lld
use-lld
Compiletest builds thousands of small programs, for which linking is a large part of build time, LLD was supposed to speedup that process. |
No, |
We didn't find any perf. differences on Linux in #135001 though (AFAIK, compiletest should use targetflags even when host == target, unless the test specifies
Ok, in that case I will keep the workaround, and based on what gets merged first we'll resolve the merge conflict. |
a09620b
to
5297c89
Compare
This comment has been minimized.
This comment has been minimized.
5297c89
to
76e1302
Compare
I'll try to re-measure (on windows-gnu). |
@bors r+ rollup |
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#139041 (Remove `rustc_middle::ty::util::ExplicitSelf`.) - rust-lang#139328 (Fix 2024 edition doctest panic output) - rust-lang#139339 (unstable book: document tait) - rust-lang#139348 (AsyncDestructor: replace fields with impl_did) - rust-lang#139353 (Fix `Debug` impl for `LateParamRegionKind`.) - rust-lang#139366 (ToSocketAddrs: fix typo) - rust-lang#139374 (Use the span of the whole bound when the diagnostic talks about a bound) - rust-lang#139378 (Use target-agnostic LLD flags in bootstrap for `use-lld`) - rust-lang#139384 (Add `compiletest` adhoc_group for `r? compiletest`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#139378 - Kobzol:bootstrap-use-lld-fix, r=petrochenkov Use target-agnostic LLD flags in bootstrap for `use-lld` [Before](rust-lang#135001), I hardcoded LLD flags that pretty much only worked on GNU. The right way is to use `-Zlinker-features` instead though. I *think* that this should also make this work on Windows mingw, and thus `@petrochenkov's` workaround is no longer necessary. Fixes: rust-lang#139372 Closes: rust-lang#139375 r? `@lqd`
With lld:
So pretty much the same. |
Thanks for the benchmark! Yeah, it seems like for the tests it doesn't really matter. Could you please post the test failures? I tried it on Linux and I think that everything succeeded there. |
|
Thank you. All of these are either green or ignored on x64 Linux, so that should be fine regarding LLD on x64. |
Some of these seem surprising to fail for linker reasons, apart from a failure about lld itself (but then I don't understand why it's on so few tests). What's the link between the linker and |
Before, I hardcoded LLD flags that pretty much only worked on GNU. The right way is to use
-Zlinker-features
instead though.I think that this should also make this work on Windows mingw, and thus @petrochenkov's workaround is no longer necessary.
Fixes: #139372
Closes: #139375
r? @lqd