Wrapper for `-Z gcc-ld=lld` to invoke rust-lld with the correct flavor#89288

hkratz · 2021-09-26T20:56:59Z

This PR adds an lld-wrapper tool which is installed as ld and ld64 in lib\rustlib\<host_target>\bin\gcc-ld directory and whose sole purpose is to invoke rust-lld in the parent directory with the correct flavor. Lld decides which flavor to use from either the first two commandline arguments or from the name of the executable (ld for GNU/ld flavor, ld64 for Darwin/Macos/ld64 flavor and so on). Symbolic links could not be used as they are not supported by rustup and on Windows.

The wrapper replaces full copies of rust-lld which added some significant bloat. On UNIXish operating systems it exec rust-lld, on Windows it spawns it as a child process.

Fixes #88869.

r? @Mark-Simulacrum
cc @nagisa @petrochenkov @1000teslas

nagisa · 2021-09-26T21:06:03Z

Out of curiosity, what's the size of this wrapper?

hkratz · 2021-09-26T21:36:07Z

Out of curiosity, what's the size of this wrapper?

On Linux the wrapper is 548kb, on Macos 424kb, haven't checked Windows yet.

The rust-lld distributed with the current bootstrap compiler is only about 6.5mb on Linux with libLLVM-13.so linked in dynamically. On Macos rust-lld as distributed with the current bootstrap compiler does apparently not link to the llvm library dynamically and weighs in at about 73mb, haven't checked if that is on purpose.

est31 · 2021-09-27T00:42:02Z

I wonder if it can be reduced further with https://github.com/johnthagen/min-sized-rust or so.

src/bootstrap/tool.rs

src/tools/lld-wrapper/src/main.rs

tschuett · 2021-09-27T06:12:45Z

Could this be a Python script instead to save space?

est31 · 2021-09-27T06:21:57Z

@tschuett then it'd require python to be installed on the system. Same goes for bash scripts. I wonder though whether one could build something smaller, like something with no_std that just calls execve on unixes and whatever windows has on windows. Half a megabyte is definitely a lot for a binary whose main job is to just call another binary.

Kobzol · 2021-09-27T07:26:09Z

FWIW, just out of curiosity, I tried to build it with

[profile.release]
opt-level = "z"
lto = true
codegen-units = 1
panic = "abort"

and build-std (cargo +nightly build -Z build-std=std,panic_abort -Z build-std-features=panic_immediate_abort --target x86_64-unknown-linux-gnu --release --features ld).

On Linux, it got the executable size after stripping from ~250 KiB to 50 KiB.

est31 · 2021-09-27T07:41:26Z

Stripping is probably non trivial but one could probably add debug = false to the profile.

Kobzol · 2021-09-27T07:45:14Z

I probably measured it wrong before, even without stripping it sits at 75 KiB (debug = false doesn't change anything, I suppose that it's the default value in release?).

I also looked at why the binary is so big with cargo-bloat (but for that I had to turn off LTO, so the results might be skewed.. Most of the space was taken by backtrace and symbol resolving (addr2line, gimli). Maybe also Command can be an issue (#64140). But with build-std it's reduced quite well.

hkratz · 2021-09-27T08:45:34Z

Tried the minimization on Windows, the binary went from 300kb to 150kb.

The installed size of the x86_64 Windows toolchains are:

$ du -sm .rustup/toolchains/*
1056    .rustup/toolchains/beta-x86_64-pc-windows-msvc
1083    .rustup/toolchains/nightly-x86_64-pc-windows-msvc
961     .rustup/toolchains/stable-x86_64-pc-windows-msvc

So while reducing the ld.exe + ld64.exe overhead from 120mb to 600kb has an impact, further reducing it from 600kb to 300kb or less is not really worth the build time increase of -Z build-std IMHO.

Kobzol · 2021-09-27T08:48:49Z

It might be a bit overkill, I agree.

Although, on Linux at least, stripping the binary should be trivial, and could be done on a "best effort" basis (if strip is not available, just don't strip it).

hkratz · 2021-09-27T13:05:14Z

@Mark-Simulacrum If I use ToolBootstrap I have to always use the bootstrap compiler otherwise it asserts. Isn't that a problem when cross-compiling? I will do some testing with cross-compilation.

Mark-Simulacrum · 2021-09-27T13:06:45Z

I don't think we need to bother with trying to get these down to even smaller sizes; several hundred kilobytes is tiny compared to the rest of the rustc tarball, and so cutting that even a little ultimately doesn't matter that much IMO.

@Mark-Simulacrum If I use ToolBootstrap I have to always use the bootstrap compiler otherwise it asserts. Isn't that a problem when cross-compiling? I will do some testing with cross-compilation.

I'm not sure I follow. Can you say what assertion you're seeing? The bootstrap compiler should be capable of cross-compiling to ~any architecture (unless it's been added within the last release cycle, but I don't think we need to worry about that -- targets don't jump to having host tools from the get go). You might need a builder.ensure(compile::Std { compiler, target }); though.

hkratz · 2021-09-27T13:17:03Z

It only panics if the wrapper is built as ToolBootstrap with a stage 1 compiler. That is probably to be expected:

thread 'main' panicked at 'assertion failed: !use_snapshot || stage == 0 || self.local_rebuild', src/bootstrap/builder.rs:1077:9
s

With the latest commit it always uses the bootstrap compiler so the panic is gone. If using the bootstrap compiler is fine for cross-compiling the PR is ready for re-review.

Mark-Simulacrum · 2021-09-27T13:21:37Z

Yes, that's expected, but shouldn't happen with the latest changes as you indicated :)

OK, great, then I think we can likely go ahead with this. I'm not sure that current_exe() will work in all cases -- I know we've had problems with it in the past -- but I also don't see an easy alternative, so it seems OK for now. We can always iterate on this in the future.

Can you squash the commits? r=me with that done.

tschuett · 2021-09-27T15:09:47Z

You can't have cc pass -flavor darwin to the linker (rust-lld)?

hkratz · 2021-09-27T15:20:24Z

You can't have cc pass -flavor darwin to the linker (rust-lld)?

With either -Wl,<option> or -Xlinker <option> clang adds <option> after other options. lld requires them to be the first two arguments.

hkratz · 2021-09-28T06:32:09Z

@Mark-Simulacrum Squashed.

Is it too late to consider it for the beta? Even if there were problems with the wrapper, -Z gcc-ld=lld does not work on stable anyway without RUSTC_BOOTSTRAP=1 shenanigans.

Mark-Simulacrum · 2021-10-02T01:42:15Z

Yes, we don't generally backport anything except regression fixes (which this isn't, as it targets an unstable feature).

@bors r+

bors · 2021-10-02T01:42:16Z

📌 Commit ac081e1 has been approved by Mark-Simulacrum

hkratz · 2021-10-02T05:31:29Z

@Mark-Simulacrum I think I was a bit unclear. The installed size increase of >100MB from stable to beta on Windows/Macos affects everyone and not just users of the unstable feature. That is why the linked issue #88869 is marked as regression-from-stable-to-beta with medium priority.

The risk of a backport is low as even if the wrapper has bugs it would not affect people using beta/stable.

Mark-Simulacrum · 2021-10-02T12:13:16Z

Ah, I see. I think a backport makes sense then. Thanks!

Mark-Simulacrum · 2021-10-07T14:56:36Z

Alright, r=me with commits squashed. Thanks!

The wrapper is installed as `ld` and `ld64` in the `lib\rustlib\<host_target>\bin\gcc-ld` directory and its sole purpose is to invoke `rust-lld` in the parent directory with the correct flavor.

Mark-Simulacrum · 2021-10-07T15:03:36Z

@bors r+

bors · 2021-10-07T15:04:21Z

📌 Commit 6162fc0 has been approved by Mark-Simulacrum

…ulacrum Wrapper for `-Z gcc-ld=lld` to invoke rust-lld with the correct flavor This PR adds an `lld-wrapper` tool which is installed as `ld` and `ld64` in `lib\rustlib\<host_target>\bin\gcc-ld` directory and whose sole purpose is to invoke `rust-lld` in the parent directory with the correct flavor. Lld decides which flavor to use from either the first two commandline arguments or from the name of the executable (`ld` for GNU/ld flavor, `ld64` for Darwin/Macos/ld64 flavor and so on). Symbolic links could not be used as they are not supported by rustup and on Windows. The wrapper replaces full copies of rust-lld which added some significant bloat. On UNIXish operating systems it exec rust-lld, on Windows it spawns it as a child process. Fixes rust-lang#88869. r? `@Mark-Simulacrum` cc `@nagisa` `@petrochenkov` `@1000teslas`

…ulacrum Wrapper for `-Z gcc-ld=lld` to invoke rust-lld with the correct flavor This PR adds an `lld-wrapper` tool which is installed as `ld` and `ld64` in `lib\rustlib\<host_target>\bin\gcc-ld` directory and whose sole purpose is to invoke `rust-lld` in the parent directory with the correct flavor. Lld decides which flavor to use from either the first two commandline arguments or from the name of the executable (`ld` for GNU/ld flavor, `ld64` for Darwin/Macos/ld64 flavor and so on). Symbolic links could not be used as they are not supported by rustup and on Windows. The wrapper replaces full copies of rust-lld which added some significant bloat. On UNIXish operating systems it exec rust-lld, on Windows it spawns it as a child process. Fixes rust-lang#88869. r? ``@Mark-Simulacrum`` cc ``@nagisa`` ``@petrochenkov`` ``@1000teslas``

…ingjubilee Rollup of 8 pull requests Successful merges: - rust-lang#87918 (Enable AutoFDO.) - rust-lang#88137 (On macOS, make strip="symbols" not pass any options to strip) - rust-lang#88772 (Fixed confusing wording on Result::map_or_else.) - rust-lang#89025 (Implement `#[link_ordinal(n)]`) - rust-lang#89082 (Implement rust-lang#85440 (Random test ordering)) - rust-lang#89288 (Wrapper for `-Z gcc-ld=lld` to invoke rust-lld with the correct flavor) - rust-lang#89476 (Correct decoding of foreign expansions during incr. comp.) - rust-lang#89622 (Use correct edition for panic in [debug_]assert!().) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup

[beta] backports - 2229: Consume IfLet expr rust-lang#89282 - Wrapper for -Z gcc-ld=lld to invoke rust-lld with the correct flavor rust-lang#89288 - Fix unsound optimization with explicit variant discriminants rust-lang#89489 - Fix stabilization version for bindings_after_at rust-lang#89605 - Turn vtable_allocation() into a query rust-lang#89619 - Revert "Stabilize Iterator::intersperse()" rust-lang#89638 - Ignore type of projections for upvar capturing rust-lang#89648 - ~~Add Poll::ready and~~ revert stabilization of task::ready! rust-lang#89651 - CI: Use mirror for libisl downloads for more docker dist builds rust-lang#89661 - Use correct edition for panic in [debug_]assert!(). rust-lang#89622 - Switch to our own mirror of libisl plus ct-ng oldconfig fixes rust-lang#89599 - Emit item no type error even if type inference fails rust-lang#89585 - Revert enum discriminants rust-lang#89884

petrochenkov · 2022-05-24T12:12:45Z

I missed this PR and only noticed the lld-wrapper tool later.

I strongly suspect that separate tool is an unproportionately large hammer and not the best solution, so I made #97352.
(I didn't yet look at this in detail though.)

rust-highfive assigned Mark-Simulacrum Sep 26, 2021

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 26, 2021

hkratz mentioned this pull request Sep 26, 2021

Include copies of rust-lld for -Z gcc-ld=lld only in nightly builds. #89218

Closed

Mark-Simulacrum reviewed Sep 27, 2021

View reviewed changes

src/bootstrap/tool.rs Outdated Show resolved Hide resolved

Mark-Simulacrum reviewed Sep 27, 2021

View reviewed changes

src/tools/lld-wrapper/src/main.rs Outdated Show resolved Hide resolved

Mark-Simulacrum reviewed Sep 27, 2021

View reviewed changes

src/tools/lld-wrapper/src/main.rs Outdated Show resolved Hide resolved

hkratz force-pushed the lld_wrapper branch from c6594fa to ac081e1 Compare September 27, 2021 13:42

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 Oct 2, 2021

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 5, 2021

Add wrapper for -Z gcc-ld=lld to invoke rust-lld with the correct flavor

6162fc0

The wrapper is installed as `ld` and `ld64` in the `lib\rustlib\<host_target>\bin\gcc-ld` directory and its sole purpose is to invoke `rust-lld` in the parent directory with the correct flavor.

hkratz force-pushed the lld_wrapper branch from ccf3b26 to 6162fc0 Compare October 7, 2021 15:00

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 Oct 7, 2021

Manishearth mentioned this pull request Oct 7, 2021

Rollup of 10 pull requests #89645

Closed

Manishearth mentioned this pull request Oct 8, 2021

Rollup of 9 pull requests #89656

Closed

workingjubilee mentioned this pull request Oct 8, 2021

Rollup of 8 pull requests #89659

Merged

bors merged commit 1c1c6ed into rust-lang:master Oct 8, 2021

rustbot added this to the 1.57.0 milestone Oct 8, 2021

cuviper mentioned this pull request Oct 13, 2021

[beta] backports #89854

Merged

cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Oct 13, 2021

cuviper modified the milestones: 1.57.0, 1.56.0 Oct 13, 2021

hkratz deleted the lld_wrapper branch November 6, 2021 20:37

petrochenkov mentioned this pull request May 24, 2022

Find a way to get rid of src\tools\lld-wrapper #97352

Closed

mati865 mentioned this pull request Feb 23, 2023

Respect config.lld_enabled when building windows-gnu. #108140

Closed

Uh oh!

Conversation

hkratz commented Sep 26, 2021

Uh oh!

nagisa commented Sep 26, 2021

Uh oh!

hkratz commented Sep 26, 2021

Uh oh!

est31 commented Sep 27, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tschuett commented Sep 27, 2021

Uh oh!

est31 commented Sep 27, 2021

Uh oh!

Kobzol commented Sep 27, 2021

Uh oh!

est31 commented Sep 27, 2021

Uh oh!

Kobzol commented Sep 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hkratz commented Sep 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kobzol commented Sep 27, 2021

Uh oh!

hkratz commented Sep 27, 2021

Uh oh!

Mark-Simulacrum commented Sep 27, 2021

Uh oh!

hkratz commented Sep 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mark-Simulacrum commented Sep 27, 2021

Uh oh!

tschuett commented Sep 27, 2021

Uh oh!

hkratz commented Sep 27, 2021

Uh oh!

hkratz commented Sep 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mark-Simulacrum commented Oct 2, 2021

Uh oh!

bors commented Oct 2, 2021

Uh oh!

hkratz commented Oct 2, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Mark-Simulacrum commented Oct 2, 2021

Uh oh!

Mark-Simulacrum commented Oct 7, 2021

Uh oh!

Mark-Simulacrum commented Oct 7, 2021

Uh oh!

bors commented Oct 7, 2021

Uh oh!

petrochenkov commented May 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

14 participants

Kobzol commented Sep 27, 2021 •

edited

Loading

hkratz commented Sep 27, 2021 •

edited

Loading

hkratz commented Sep 27, 2021 •

edited

Loading

hkratz commented Sep 28, 2021 •

edited

Loading

hkratz commented Oct 2, 2021 •

edited

Loading

petrochenkov commented May 24, 2022 •

edited

Loading