Allow using self-contained LLD in bootstrap #135001

Kobzol · 2025-01-01T18:04:11Z

In #116278, I added a "self-contained" mode to the rust.use-lld bootstrap option, which was designed for using the built-in LLD for linking compiler artifacts. However, this was later reverted in #118810.

This PR brings the old logic back, which switches LLD in bootstrap from -fuse-ld=lld to MCP510's way of passing linker flags to enable LLD (both external and self-contained). So this does two changes:

Goes from -fuse-ld=lld to MCP510
Actually makes it possible to use the self-contained LLD to compile compiler artifacts

Regarding the second commit: Since #86113, we have been passing -fuse-ld=lld as a target flag to all tests when use-lld = true is enabled. This kind of worked for all tests, since it was just a linker argument, which has bypassed any compiler checks, and probably resulted only in some warning if the given target linker didn't actually support LLD. However, after the first commit, some tests actually start failing with this approach:

error: linker flavor `gnu-lld-cc` is incompatible with the current target
   |
   = note: compatible flavors are: llbc, ptx

So the second commit removes the passing of LLD flags as target flags to tests. I don't think that it's a good idea to pass specific compiler flags to all tests unconditionally, tbh. The doctest command from #86113 doesn't go through compiletest anymore, and doctests should be quite a lot faster since #126245 in general.

CC @the8472

If someone has a beefy machine, it would be nice to test whether this doesn't regress test execution speed. How to do that:

Enable rust.use-lld = true and rust.lld = true in config.toml
Benchmark ./x test tests/ui --force-rerun between master and this PR

Once this is tested in the wild, I would like to make the self-contained LLD the default in CI, hopefully to make CI builds faster.

r? @onur-ozkan

…ntained"` is used Before, we just used the global `lld` anyway.

Not all targets support these flags, so we cannot just pass them to the tests unconditionally. Before, we were using a linker arg (`-Clink-arg=-fuse-ld=lld`), which circumvented this in a hacky way.

saethlin · 2025-01-01T20:25:03Z

If someone has a beefy machine

I have a 3970X (which may or may not qualify these days). On master, I see

test result: ok. 18033 passed; 0 failed; 171 ignored; 0 measured; 0 filtered out; finished in 134.13s

On this PR, I see

test result: ok. 18033 passed; 0 failed; 171 ignored; 0 measured; 0 filtered out; finished in 133.18s

running again:

test result: ok. 18033 passed; 0 failed; 171 ignored; 0 measured; 0 filtered out; finished in 133.44s

(the difference caused by this PR is close to jitter)

Kobzol · 2025-01-01T20:28:33Z

Thanks, it doesn't seem like there's any difference then. Actually, I wonder if the stage1 compiler uses LLD anyway, since it's now enabled by default on nightly 🤔 In any case, the second commit shouldn't regress perf. anyway.

the8472 · 2025-01-01T20:46:34Z

Measured on a 24c/48t Zen2 CPU

372442fe5ff1a2d06f4119f2b2e7d1e42388a0d3 (master):

$ hyperfine -w 1 './x test tests/ui --force-rerun --stage 1'
Benchmark #1: ./x test tests/ui --force-rerun --stage 1
  Time (mean ± σ):     50.948 s ±  0.286 s    [User: 899.882 s, System: 1086.781 s]
  Range (min … max):   50.665 s … 51.433 s    10 runs

$ hyperfine -w 1 './x.py test library/core --doc --stage 0'
Benchmark #1: ./x.py test library/core --doc --stage 0
  Time (mean ± σ):     22.122 s ±  0.159 s    [User: 380.522 s, System: 425.409 s]
  Range (min … max):   21.885 s … 22.407 s    10 runs

3d69dd1661361479dad4b867cca91bff9e8dd13b (pr):

$ hyperfine -w 1 './x test tests/ui --force-rerun --stage 1'
Benchmark #1: ./x test tests/ui --force-rerun --stage 1
  Time (mean ± σ):     51.772 s ±  0.408 s    [User: 899.511 s, System: 1098.635 s]
  Range (min … max):   51.229 s … 52.273 s    10 runs

$ hyperfine -w 1 './x.py test library/core --doc --stage 0'
Benchmark #1: ./x.py test library/core --doc --stage 0
  Time (mean ± σ):     22.004 s ±  0.031 s    [User: 379.170 s, System: 426.521 s]
  Range (min … max):   21.952 s … 22.063 s    10 runs

Doc test ranges overlap. But it seems to be a small slowdown for UI tests.

Thanks, it doesn't seem like there's any difference then. Actually, I wonder if the stage1 compiler uses LLD anyway

Not sure what you mean? LLD was already used before to build the tests, that's why bootstrap has some handling to set the thread count for lld (UI tests should be running with --threads 1).

Kobzol · 2025-01-01T20:50:20Z

I meant that the tests are already linked with LLD even without using use-lld = true, because the nightly compiler now uses its self-contained LLD by default (

rust/src/bootstrap/src/core/build_steps/compile.rs

Line 1181 in ceb0441

if builder.config.lld_enabled

). So the second commit should essentially be a no-op on x64 Linux.

Note: this is only true for stage 1+, since on stage0 the bootstrap compiler is beta, which doesn't use LLD by default, so use-lld = true still makes sense there.

jieyouxu · 2025-01-02T06:44:14Z

I was going to check the time on native msvc, but it turns out with rust.lld = true and rust.use-lld = true set on native x86_64-pc-windows-msvc, this ui test fails for me locally (with or without this PR; it already fails on master checkout):

`tests\ui\asan-odr-win\asan_odr_windows.rs`

--- stderr -------------------------------
error: linking with `lld` failed: exit code: 1
   |
   = note: "lld" "-flavor" "link" "/NOLOGO" "C:\\Users\\ADMINI~1\\AppData\\Local\\Temp\\rustcEwtKzP\\symbols.o" "/WHOLEARCHIVE:X:\\repos\\rust\\build\\x86_64-pc-windows-msvc\\stage1\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib\\librustc-dev_rt.asan.a" "<1 object files omitted>" "X:\\repos\\rust\\build\\x86_64-pc-windows-msvc\\test\\ui\\asan-odr-win\\asan_odr_windows\\auxiliary/{libothercrate.rlib}" "X:\\repos\\rust\\build\\x86_64-pc-windows-msvc\\stage1\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib\\std-972c03a8627cad3b.dll.lib" "X:\\repos\\rust\\build\\x86_64-pc-windows-msvc\\stage1\\lib\\rustlib\\x86_64-pc-windows-msvc\\lib/{libcompiler_builtins-5ad709b9972a6089.rlib}" "kernel32.lib" "kernel32.lib" "advapi32.lib" "ntdll.lib" "userenv.lib" "ws2_32.lib" "dbghelp.lib" "/defaultlib:msvcrt" "C:\\Users\\ADMINI~1\\AppData\\Local\\Temp\\rustcEwtKzP\\api-ms-win-core-synch-l1-2-0.dll_imports_indirect.lib" "C:\\Users\\ADMINI~1\\AppData\\Local\\Temp\\rustcEwtKzP\\bcryptprimitives.dll_imports_indirect.lib" "/NXCOMPAT" "/LIBPATH:X:\\repos\\rust\\build\\x86_64-pc-windows-msvc\\native\\rust-test-helpers" "/LIBPATH:X:\\repos\\rust\\build\\x86_64-pc-windows-msvc\\test\\ui\\asan-odr-win\\asan_odr_windows\\auxiliary" "/OUT:X:\\repos\\rust\\build\\x86_64-pc-windows-msvc\\test\\ui\\asan-odr-win\\asan_odr_windows\\a.exe" "/OPT:REF,ICF" "/DEBUG" "/PDBALTPATH:%_PDB%"
   = note: [...]
   = note: lld: error: could not open 'X:\repos\rust\build\x86_64-pc-windows-msvc\stage1\lib\rustlib\x86_64-pc-windows-msvc\lib\librustc-dev_rt.asan.a': no such file or directory␍


error: aborting due to 1 previous error
------------------------------------------

EDIT: opened #135013 to track that test failure.

onur-ozkan · 2025-01-02T07:03:03Z

src/bootstrap/src/core/build_steps/test.rs

@@ -1901,7 +1901,6 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the

        let mut targetflags = flags;
        targetflags.push(format!("-Lnative={}", builder.test_helpers_out(target).display()));
-        targetflags.extend(linker_flags(builder, compiler.host, LldThreads::No));


Shouldn't we design linker_flags in a way that preserves the old logic for tests instead of removing it entirely?

I saw only very minimal gains with LLD vs the default Linux linker on UI tests, but it's true that on some other platforms the difference might be larger.

The problem with the old approach is that it cannot respect the "self-contained" mode. So when you use lld = "self-contained", the old approach ignores that and just uses the global lld for the tests.

I guess that a compromise could be to use the linker flag with use-lld = true and the new compiler flag with use-lld = "self-contained", but I would eventually like to set use-lld = "self-contained" to be the default on CI; with this approach, some tests would simply fail..

I guess that a compromise could be to use the linker flag with use-lld = true and the new compiler flag with use-lld = "self-contained", but I would eventually like to set use-lld = "self-contained" to be the default on CI; with this approach, some tests would simply fail..

This sounds reasonable I think.

But there's still the "some tests would fail" part 😆 Another alternative would be to actually modify the target configuration of the compiler when use-lld is enabled, but this means that the compiler itself would be modified and would use lld by default; right now, we just enable lld in a hacky way in bootstrap.

You said "eventually", I am not sure why we should concern on that part right now.

It's not just about CI. Right now, use-lld is quite useful to make local compiler builds faster. But if we actually make self-contained working as intended, and keep the compiletest target flags, then use-lld = "self-contained" will also cause UI tests to fail for people that use this config value.

I see. In this case I am fine with landing it as is. We can create an issue to track that part.

onur-ozkan · 2025-01-03T18:08:28Z

@bors r+

bors · 2025-01-03T18:08:31Z

📌 Commit 3d69dd1 has been approved by onur-ozkan

It is now in the queue for this repository.

…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#133964 (core: implement `bool::select_unpredictable`) - rust-lang#135001 (Allow using self-contained LLD in bootstrap) - rust-lang#135055 (Report impl method has stricter requirements even when RPITIT inference gets in the way) - rust-lang#135064 (const-in-pattern: test that the PartialEq impl does not need to be const) - rust-lang#135066 (bootstrap: support `./x check run-make-support`) - rust-lang#135069 (remove unused function params) - rust-lang#135084 (Update carrying_mul_add test to tolerate `nuw`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#135001 - Kobzol:bootstrap-mcp-510, r=onur-ozkan Allow using self-contained LLD in bootstrap In rust-lang#116278, I added a `"self-contained"` mode to the `rust.use-lld` bootstrap option, which was designed for using the built-in LLD for linking compiler artifacts. However, this was later reverted in rust-lang#118810. This PR brings the old logic back, which switches LLD in bootstrap from `-fuse-ld=lld` to [MCP510](rust-lang/compiler-team#510 way of passing linker flags to enable LLD (both external and self-contained). So this does two changes: 1) Goes from `-fuse-ld=lld` to MCP510 2) Actually makes it possible to use the self-contained LLD to compile compiler artifacts Regarding the second commit: Since rust-lang#86113, we have been passing `-fuse-ld=lld` as a target flag to all tests when `use-lld = true` is enabled. This kind of worked for all tests, since it was just a linker argument, which has bypassed any compiler checks, and probably resulted only in some warning if the given target linker didn't actually support LLD. However, after the first commit, some tests actually start failing with this approach: ``` error: linker flavor `gnu-lld-cc` is incompatible with the current target | = note: compatible flavors are: llbc, ptx ``` So the second commit removes the passing of LLD flags as target flags to tests. I don't think that it's a good idea to pass specific compiler flags to all tests unconditionally, tbh. The doctest command from rust-lang#86113 doesn't go through compiletest anymore, and doctests should be quite a lot faster since rust-lang#126245 in general. CC `@the8472` If someone has a beefy machine, it would be nice to test whether this doesn't regress test execution speed. How to do that: 1) Enable `rust.use-lld = true` and `rust.lld = true` in `config.toml` 2) Benchmark `./x test tests/ui --force-rerun` between `master` and this PR Once this is tested in the wild, I would like to make the self-contained LLD the default in CI, hopefully to make CI builds faster. r? `@onur-ozkan`

bors · 2025-01-04T12:49:31Z

⌛ Testing commit 3d69dd1 with merge f17cf74...

jieyouxu · 2025-01-09T00:39:05Z

This is already merged, bors.

@bors retry r-

Kobzol added 2 commits January 1, 2025 18:49

Actually use self-contained lld in bootstrap when `use-lld = "self-co…

ab5446c

…ntained"` is used Before, we just used the global `lld` anyway.

Do not pass LLD flags to targets flags of compiletest

3d69dd1

Not all targets support these flags, so we cannot just pass them to the tests unconditionally. Before, we were using a linker arg (`-Clink-arg=-fuse-ld=lld`), which circumvented this in a hacky way.

rustbot assigned onur-ozkan Jan 1, 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 Jan 1, 2025

jieyouxu self-assigned this Jan 2, 2025

onur-ozkan reviewed Jan 2, 2025

View reviewed changes

jieyouxu removed their assignment Jan 2, 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 Jan 3, 2025

matthiaskrgr mentioned this pull request Jan 4, 2025

Rollup of 7 pull requests #135095

Merged

bors merged commit 8744b44 into rust-lang:master Jan 4, 2025
6 checks passed

rustbot added this to the 1.86.0 milestone Jan 4, 2025

Kobzol deleted the bootstrap-mcp-510 branch January 4, 2025 13:33

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jan 9, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow using self-contained LLD in bootstrap #135001

Allow using self-contained LLD in bootstrap #135001

Kobzol commented Jan 1, 2025

saethlin commented Jan 1, 2025 •

edited

Loading

Kobzol commented Jan 1, 2025

the8472 commented Jan 1, 2025 •

edited

Loading

Kobzol commented Jan 1, 2025 •

edited

Loading

jieyouxu commented Jan 2, 2025 •

edited

Loading

onur-ozkan Jan 2, 2025

Kobzol Jan 3, 2025 •

edited

Loading

onur-ozkan Jan 3, 2025

Kobzol Jan 3, 2025

onur-ozkan Jan 3, 2025

Kobzol Jan 3, 2025

onur-ozkan Jan 3, 2025

onur-ozkan commented Jan 3, 2025

bors commented Jan 3, 2025

bors commented Jan 4, 2025

jieyouxu commented Jan 9, 2025

Allow using self-contained LLD in bootstrap #135001

Allow using self-contained LLD in bootstrap #135001

Conversation

Kobzol commented Jan 1, 2025

saethlin commented Jan 1, 2025 • edited Loading

Kobzol commented Jan 1, 2025

the8472 commented Jan 1, 2025 • edited Loading

Kobzol commented Jan 1, 2025 • edited Loading

jieyouxu commented Jan 2, 2025 • edited Loading

onur-ozkan Jan 2, 2025

Choose a reason for hiding this comment

Kobzol Jan 3, 2025 • edited Loading

Choose a reason for hiding this comment

onur-ozkan Jan 3, 2025

Choose a reason for hiding this comment

Kobzol Jan 3, 2025

Choose a reason for hiding this comment

onur-ozkan Jan 3, 2025

Choose a reason for hiding this comment

Kobzol Jan 3, 2025

Choose a reason for hiding this comment

onur-ozkan Jan 3, 2025

Choose a reason for hiding this comment

onur-ozkan commented Jan 3, 2025

bors commented Jan 3, 2025

bors commented Jan 4, 2025

jieyouxu commented Jan 9, 2025

saethlin commented Jan 1, 2025 •

edited

Loading

the8472 commented Jan 1, 2025 •

edited

Loading

Kobzol commented Jan 1, 2025 •

edited

Loading

jieyouxu commented Jan 2, 2025 •

edited

Loading

Kobzol Jan 3, 2025 •

edited

Loading