Use target-agnostic LLD flags in bootstrap for `use-lld` by Kobzol · Pull Request #139378 · rust-lang/rust

Kobzol · 2025-04-04T16:11:53Z

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

Kobzol · 2025-04-04T16:14:52Z

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...

petrochenkov · 2025-04-04T16:19:53Z

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...

Compiletest builds thousands of small programs, for which linking is a large part of build time, LLD was supposed to speedup that process.

petrochenkov · 2025-04-04T16:23:58Z

I think that this should also make this work on Windows mingw, and thus @petrochenkov's workaround is no longer necessary.

No, -Zlinker-features=+lld will still trigger the incorrect response file escaping mode in bootstrap rustc.
But the workaround should go away in a few days? The new stable rustc was released, so the bootstrap compiler is also going to be bumped soon.

Kobzol · 2025-04-04T16:26:00Z

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 //@ force-host). But maybe there are larger differences on other platforms?

No, -Zlinker-features=+lld will still trigger the incorrect response file escaping mode in bootstrap rustc.
But the workaround should go away in a few days? The new stable rustc was released, so the bootstrap compiler is also going to be bumped soon.

Ok, in that case I will keep the workaround, and based on what gets merged first we'll resolve the merge conflict.

petrochenkov · 2025-04-04T16:29:43Z

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 //@ force-host). But maybe there are larger differences on other platforms?

I'll try to re-measure (on windows-gnu).

Kobzol · 2025-04-04T16:41:57Z

r? @petrochenkov

petrochenkov · 2025-04-04T17:32:28Z

@bors r+ rollup

bors · 2025-04-04T17:32:31Z

📌 Commit 76e1302 has been approved by petrochenkov

It is now in the queue for this repository.

…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`

petrochenkov · 2025-04-06T13:43:17Z

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 //@ force-host). But maybe there are larger differences on other platforms?

I'll try to re-measure (on windows-gnu).

time ./x t tests/ui on x86_64-pc-windows-gnu.
Without lld:

2m25.104s
2m17.629s
2m15.590s

With lld:

2m21.616s
2m19.646s
2m17.501s

So pretty much the same.
Also, 27 tests are failing with lld now.

Kobzol · 2025-04-06T13:45:19Z

Thanks for the benchmark! Yeah, it seems like for the tests it doesn't really matter. use-lld is quite a bit faster on Linux for rebuilding the compiler itself though.

Could you please post the test failures? I tried it on Linux and I think that everything succeeded there.

petrochenkov · 2025-04-06T17:55:42Z

Could you please post the test failures? I tried it on Linux and I think that everything succeeded there.

    [ui] tests\ui\debuginfo\windows_gnu_split_debuginfo_packed.rs#i686_g
    [ui] tests\ui\debuginfo\windows_gnu_split_debuginfo_packed.rs#i686_gl
    [ui] tests\ui\debuginfo\windows_gnu_split_debuginfo_packed.rs#aarch64_gl
    [ui] tests\ui\debuginfo\windows_gnu_split_debuginfo_packed.rs#x86_64_g
    [ui] tests\ui\debuginfo\windows_gnu_split_debuginfo_packed.rs#x86_64_gl
    [ui] tests\ui\debuginfo\windows_gnu_split_debuginfo_packed.rs#i686_uwp_g
    [ui] tests\ui\debuginfo\windows_gnu_split_debuginfo_packed.rs#x86_64_uwp_g
    [ui] tests\ui\debuginfo\windows_gnu_split_debuginfo_unpacked.rs#i686_g
    [ui] tests\ui\debuginfo\windows_gnu_split_debuginfo_unpacked.rs#aarch64_gl
    [ui] tests\ui\debuginfo\windows_gnu_split_debuginfo_unpacked.rs#i686_gl
    [ui] tests\ui\debuginfo\windows_gnu_split_debuginfo_unpacked.rs#i686_uwp_g
    [ui] tests\ui\debuginfo\windows_gnu_split_debuginfo_unpacked.rs#x86_64_g
    [ui] tests\ui\debuginfo\windows_gnu_split_debuginfo_unpacked.rs#x86_64_gl
    [ui] tests\ui\debuginfo\windows_gnu_split_debuginfo_unpacked.rs#x86_64_uwp_g
    [ui] tests\ui\feature-gates\env-flag.rs
    [ui] tests\ui\feature-gates\feature-gate-native_link_modifiers_as_needed.rs#in_flag
    [ui] tests\ui\linkage-attr\unstable-flavor.rs#bpf
    [ui] tests\ui\linkage-attr\unstable-flavor.rs#ptx
    [ui] tests\ui\linkage-attr\issue-10755.rs
    [ui] tests\ui\linkage-attr\linkage-attr-does-not-panic-llvm-issue-33992.rs
    [ui] tests\ui\print-request\stability.rs#check_cfg
    [ui] tests\ui\print-request\stability.rs#all_target_specs_json
    [ui] tests\ui\print-request\stability.rs#crate_root_lint_levels
    [ui] tests\ui\print-request\stability.rs#supported_crate_types
    [ui] tests\ui\print-request\stability.rs#target_spec_json
    [ui] tests\ui\symbol-mangling-version\unstable.rs#hashed
    [ui] tests\ui\symbol-mangling-version\unstable.rs#legacy

Kobzol · 2025-04-08T08:09:52Z

Thank you. All of these are either green or ignored on x64 Linux, so that should be fine regarding LLD on x64.

lqd · 2025-04-08T08:25:59Z

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 tests\ui\linkage-attr\unstable-flavor.rs#ptx for example? How's windows-gnu different from linux in that regard compared to a gcc + -fuse-ld 🤔

rustbot assigned lqd Apr 4, 2025

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) labels Apr 4, 2025

This was referenced Apr 4, 2025

Failed to build wasm32-unknown-unknown target with use-lld=true #139372

Closed

bootstrap: Pass correct linker-flavor flag for wasm targets #139375

Closed

This comment has been minimized.

Sign in to view

Kobzol changed the title ~~Use target agnostic LLD flags in bootstrap for use-lld~~ Use target-agnostic LLD flags in bootstrap for use-lld Apr 4, 2025

Kobzol force-pushed the bootstrap-use-lld-fix branch from a09620b to 5297c89 Compare April 4, 2025 16:28

rustbot added has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 4, 2025

This comment has been minimized.

Sign in to view

Use target-agnostic LLD flags in bootstrap for use-lld

76e1302

Kobzol force-pushed the bootstrap-use-lld-fix branch from 5297c89 to 76e1302 Compare April 4, 2025 16:29

rustbot removed has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Apr 4, 2025

rustbot assigned petrochenkov and unassigned lqd Apr 4, 2025

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 4, 2025

matthiaskrgr mentioned this pull request Apr 4, 2025

Rollup of 9 pull requests #139390

Merged

bors merged commit bad6b7b into rust-lang:master Apr 5, 2025
6 checks passed

rustbot added this to the 1.88.0 milestone Apr 5, 2025

Kobzol deleted the bootstrap-use-lld-fix branch April 5, 2025 06:11

Kobzol mentioned this pull request Apr 9, 2025

Bump boostrap compiler to new beta #139279

Merged

Uh oh!

Conversation

Kobzol commented Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kobzol commented Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

petrochenkov commented Apr 4, 2025

Uh oh!

petrochenkov commented Apr 4, 2025

Uh oh!

Kobzol commented Apr 4, 2025

Uh oh!

This comment has been minimized.

petrochenkov commented Apr 4, 2025

Uh oh!

Kobzol commented Apr 4, 2025

Uh oh!

petrochenkov commented Apr 4, 2025

Uh oh!

bors commented Apr 4, 2025

Uh oh!

Uh oh!

petrochenkov commented Apr 6, 2025

Uh oh!

Kobzol commented Apr 6, 2025

Uh oh!

petrochenkov commented Apr 6, 2025

Uh oh!

Kobzol commented Apr 8, 2025

Uh oh!

lqd commented Apr 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Kobzol commented Apr 4, 2025 •

edited

Loading

Kobzol commented Apr 4, 2025 •

edited

Loading