Enable ThinLTO for rustc on x64 msvc by lqd · Pull Request #103591 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation21 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

Member

@lqd lqd commented

Oct 26, 2022

• Loading

This applies the great work from @bjorn3 and @Kobzol in #101403 to x64 msvc.

Here are the local results for the try build 68c5c85ed759334a11f0b0e586f5032a23f85ce4, compared to its parent 0a6b941df354c59b546ec4c0d27f2b9b0cb1162c. Looking better than my previous local builds.

(I can't show cycle counts, as that option is failing on the windows version of the perf collector, but I'll try to analyze and debug this soon)

This will be the first of a few tests for rustc / llvm / both ThinLTO on the windows and mac targets.

rustbot added A-testsuite

Area: The testsuite used to check the correctness of rustc

T-infra

Relevant to the infrastructure team, which will review and decide on the PR/issue.

labels

Oct 26, 2022

⌛ Trying commit 0abf95bdd7e7f98b53c310649ecb03bc60d986f6 with merge 00df0000c5e43d256786737e9f49379e1b3054c4...

bors added the S-waiting-on-author

Status: This is awaiting some action (such as code changes or more information) from the author.

label

Oct 26, 2022

This comment was marked as outdated.

⌛ Trying commit 0e16a37a9f376091f4573feb33d0bcd603c811e8 with merge 68c5c85ed759334a11f0b0e586f5032a23f85ce4...

lqd mentioned this pull request

Oct 26, 2022

30 tasks

☀️ Try build successful - checks-actions
Build commit: 68c5c85ed759334a11f0b0e586f5032a23f85ce4 (68c5c85ed759334a11f0b0e586f5032a23f85ce4)

To compare with the size increase we see on linux:

rustc_driver.dll: 3% increase (≈4MB), from 124 069 888 to 128 485 888
rustc_driver.pdb: 5% increase (≈4MB), from 87 977 984 to 92 418 048

lqd changed the title~~[perf] Enable ThinLTO for rustc on x64 msvc~~ Enable ThinLTO for rustc on x64 msvc

Oct 26, 2022

Member Author

lqd commented

Oct 26, 2022

• Loading

I've updated the PR description with the results of the try build, and dropped the temporary CI hacks.

I will look at CI time tomorrow, as well as the sizes of the rustc-dev component, just in case we see an increase like we did on linux. Otherwise, I think this is ready to review, feel free to re-roll:

r? @Mark-Simulacrum

lqd marked this pull request as ready for review

October 26, 2022 23:41

For CI time, it's going to be hard to say with a sample of 1, and t-infra should have better data like the variance on that builder, but here goes: on the 5 most recent merged PRs on the dist-x86_64-msvc builder, the "run the build" step took: 1h46, 2h09, 1h45, 2h10, 2h02. The try build above took 2h15.

For the rustup rustc-dev component:

parent 0a6b941df354c59b546ec4c0d27f2b9b0cb1162c: 175.03 MB
try build 68c5c85ed759334a11f0b0e586f5032a23f85ce4: 253.46 MB

So it's a similar scale to #103538, which saw an 80MB increase. The possible fix mentioned in that issue would work on all targets I assume.

I wouldn't think it's a blocking issue though.

@rustbot ready

I don't have access to the MSVC builder times graph that t-infra has, but from the tests above, it seems the additional time wouldn't necessarily be impactful on CI times, and that we could land this change @Mark-Simulacrum ?

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

Dec 7, 2022

This is the current state of the world:

So msvc dist is ~3rd slowest today. I think this should be OK to merge though, we can adjust further as we iterate on the CI times -- I expect our planned improvements around LLVM caching on Linux should fairly cleanly map directly to the msvc builder here.

@bors r+

📌 Commit 684663e has been approved by Mark-Simulacrum

It is now in the queue for this repository.

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

Dec 8, 2022

⌛ Testing commit 684663e with merge da2270c1464b4af6c57a1193a1600cd7dd352c98...

bors added S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

and removed S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

labels

Dec 8, 2022

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

Dec 8, 2022

This comment was marked as outdated.

Finished benchmarking commit (657eefe): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.0%	[3.0%, 3.0%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.3%	[-2.0%, -0.7%]	2
All ❌✅ (primary)	-	-	0

Cycles

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.1%	[-2.1%, -2.1%]	1
All ❌✅ (primary)	-	-	0

lqd deleted the win-lto branch

December 11, 2022 22:06

Note: the perf CI result showed no change because it only measures on Windows. See the image at the top of this PR for some previously-measured Windows results.

This was referenced

Dec 13, 2022

Aaron1011 pushed a commit to Aaron1011/rust that referenced this pull request

Jan 6, 2023

Enable ThinLTO for rustc on x64 msvc

This applies the great work from @bjorn3 and @Kobzol in rust-lang#101403 to x64 msvc.

(I can't show cycle counts, as that option is failing on the windows version of the perf collector, but I'll try to analyze and debug this soon)

This will be the first of a few tests for rustc / llvm / both ThinLTO on the windows and mac targets.

ehuss mentioned this pull request

Mar 13, 2023

bors added a commit to rust-lang-ci/rust that referenced this pull request

Mar 13, 2023

…jyn514

Revert "enable ThinLTO for rustc on x86_64-pc-windows-msvc dist builds"

This lead to a miscompilation in at least char::is_whitespace and probably in more unknown places.....

See rust-lang#109067

This reverts commit 684663e, PR rust-lang#103591.

wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request

Mar 20, 2023

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request

Apr 8, 2023

Pkgsrc changes:

Adjust patches (add & remove) and cargo checksums to new versions.
It's conceivable that the workaround for LLVM based NetBSD works even less in this version (ref. PKGSRC_HAVE_LIBCPP not having a corresponding patch anymore).

Upstream changes:

Version 1.68.2 (2023-03-28)

[Update the GitHub RSA host key bundled within Cargo] (rust-lang/cargo#11883). The key was [rotated by GitHub] (https://github.blog/2023-03-23-we-updated-our-rsa-ssh-host-key/) on 2023-03-24 after the old one leaked.
[Mark the old GitHub RSA host key as revoked] (rust-lang/cargo#11889). This will prevent Cargo from accepting the leaked key even when trusted by the system.
[Add support for @revoked and a better error message for @cert-authority in Cargo's SSH host key verification] (rust-lang/cargo#11635)

Version 1.68.1 (2023-03-23)

[Fix miscompilation in produced Windows MSVC artifacts] (rust-lang/rust#109094) This was introduced by enabling ThinLTO for the distributed rustc which led to miscompilations in the resulting binary. Currently this is believed to be limited to the -Zdylib-lto flag used for rustc compilation, rather than a general bug in ThinLTO, so only rustc artifacts should be affected.
[Fix --enable-local-rust builds] (rust-lang/rust#109111)
[Treat $prefix-clang as clang in linker detection code] (rust-lang/rust#109156)
[Fix panic in compiler code] (rust-lang/rust#108162)

Version 1.68.0 (2023-03-09)

Language

[Stabilize default_alloc_error_handler] (rust-lang/rust#102318) This allows usage of alloc on stable without requiring the definition of a handler for allocation failure. Defining custom handlers is still unstable.
[Stabilize efiapi calling convention.] (rust-lang/rust#105795)
[Remove implicit promotion for types with drop glue] (rust-lang/rust#105085)

Compiler

[Change bindings_with_variant_name to deny-by-default] (rust-lang/rust#104154)
[Allow .. to be parsed as let initializer] (rust-lang/rust#105701)
[Add armv7-sony-vita-newlibeabihf as a tier 3 target] (rust-lang/rust#105712)
[Always check alignment during compile-time const evaluation] (rust-lang/rust#104616)
[Disable "split dwarf inlining" by default.] (rust-lang/rust#106709)
[Add vendor to Fuchsia's target triple] (rust-lang/rust#106429)
[Enable sanitizers for s390x-linux] (rust-lang/rust#107127)

Libraries

[Loosen the bound on the Debug implementation of Weak.] (rust-lang/rust#90291)
[Make std::task::Context !Send and !Sync] (rust-lang/rust#95985)
[PhantomData layout guarantees] (rust-lang/rust#104081)
[Don't derive Debug for OnceWith & RepeatWith] (rust-lang/rust#104163)
[Implement DerefMut for PathBuf] (rust-lang/rust#105018)
[Add O(1) Vec -> VecDeque conversion guarantee] (rust-lang/rust#105128)
[Leak amplification for peek_mut() to ensure BinaryHeap's invariant is always met] (rust-lang/rust#105851)

Stabilized APIs

[{core,std}::pin::pin!] (https://doc.rust-lang.org/stable/std/pin/macro.pin.html)
[impl From<bool> for {f32,f64}] (https://doc.rust-lang.org/stable/std/primitive.f32.html#impl-From%3Cbool%3E-for-f32)
[std::path::MAIN_SEPARATOR_STR] (https://doc.rust-lang.org/stable/std/path/constant.MAIN_SEPARATOR_STR.html)
[impl DerefMut for PathBuf] (https://doc.rust-lang.org/stable/std/path/struct.PathBuf.html#impl-DerefMut-for-PathBuf)

These APIs are now stable in const contexts:

[VecDeque::new] (https://doc.rust-lang.org/stable/std/collections/struct.VecDeque.html#method.new)

Cargo

[Stabilize sparse registry support for crates.io] (rust-lang/cargo#11224)
[cargo build --verbose tells you more about why it recompiles.] (rust-lang/cargo#11407)
[Show progress of crates.io index update even net.git-fetch-with-cli option enabled] (rust-lang/cargo#11579)

Misc

Compatibility Notes

[Add SEMICOLON_IN_EXPRESSIONS_FROM_MACROS to future-incompat report] (rust-lang/rust#103418)
[Only specify --target by default for -Zgcc-ld=lld on wasm] (rust-lang/rust#101792)
[Bump IMPLIED_BOUNDS_ENTAILMENT to Deny + ReportNow] (rust-lang/rust#106465)
[std::task::Context no longer implements Send and Sync] (rust-lang/rust#95985)

nternal Changes

These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools.

[Encode spans relative to the enclosing item] (rust-lang/rust#84762)
[Don't normalize in AstConv] (rust-lang/rust#101947)
[Find the right lower bound region in the scenario of partial order relations] (rust-lang/rust#104765)
[Fix impl block in const expr] (rust-lang/rust#104889)
[Check ADT fields for copy implementations considering regions] (rust-lang/rust#105102)
[rustdoc: simplify JS search routine by not messing with lev distance] (rust-lang/rust#105796)
[Enable ThinLTO for rustc on x86_64-pc-windows-msvc] (rust-lang/rust#103591)
[Enable ThinLTO for rustc on x86_64-apple-darwin] (rust-lang/rust#103647)

Version 1.67.0 (2023-01-26)

Language

[Make Sized predicates coinductive, allowing cycles.] (rust-lang/rust#100386)
[#[must_use] annotations on async fn also affect the Future::Output.] (rust-lang/rust#100633)
[Elaborate supertrait obligations when deducing closure signatures.] (rust-lang/rust#101834)
[Invalid literals are no longer an error under cfg(FALSE).] (rust-lang/rust#102944)
[Unreserve braced enum variants in value namespace.] (rust-lang/rust#103578)

Compiler

[Enable varargs support for calling conventions other than C or cdecl.] (rust-lang/rust#97971)
[Add new MIR constant propagation based on dataflow analysis.] (rust-lang/rust#101168)
[Optimize field ordering by grouping m*2^n-sized fields with equivalently aligned ones.] (rust-lang/rust#102750)
[Stabilize native library modifier verbatim.] (rust-lang/rust#104360)

Added and removed targets:

[Add a tier 3 target for PowerPC on AIX] (rust-lang/rust#102293), powerpc64-ibm-aix.
[Add a tier 3 target for the Sony PlayStation 1] (rust-lang/rust#102689), mipsel-sony-psx.
[Add tier 3 no_std targets for the QNX Neutrino RTOS] (rust-lang/rust#102701), aarch64-unknown-nto-qnx710 and x86_64-pc-nto-qnx710.
[Remove tier 3 linuxkernel targets] (rust-lang/rust#104015) (not used by the actual kernel).

Refer to Rust's [platform support page][platform-support-doc] for more information on Rust's tiered platform support.

Libraries

[Merge crossbeam-channel into std::sync::mpsc.] (rust-lang/rust#93563)
[Fix inconsistent rounding of 0.5 when formatted to 0 decimal places.] (rust-lang/rust#102935)
[Derive Eq and Hash for ControlFlow.] (rust-lang/rust#103084)
[Don't build compiler_builtins with -C panic=abort.] (rust-lang/rust#103786)

Stabilized APIs

[{integer}::checked_ilog] (https://doc.rust-lang.org/stable/std/primitive.i32.html#method.checked_ilog)
[{integer}::checked_ilog2] (https://doc.rust-lang.org/stable/std/primitive.i32.html#method.checked_ilog2)
[{integer}::checked_ilog10] (https://doc.rust-lang.org/stable/std/primitive.i32.html#method.checked_ilog10)
[{integer}::ilog] (https://doc.rust-lang.org/stable/std/primitive.i32.html#method.ilog)
[{integer}::ilog2] (https://doc.rust-lang.org/stable/std/primitive.i32.html#method.ilog2)
[{integer}::ilog10] (https://doc.rust-lang.org/stable/std/primitive.i32.html#method.ilog10)
[NonZeroU*::ilog2] (https://doc.rust-lang.org/stable/std/num/struct.NonZeroU32.html#method.ilog2)
[NonZeroU*::ilog10] (https://doc.rust-lang.org/stable/std/num/struct.NonZeroU32.html#method.ilog10)
[NonZero*::BITS] (https://doc.rust-lang.org/stable/std/num/struct.NonZeroU32.html#associatedconstant.BITS)

These APIs are now stable in const contexts:

[char::from_u32] (https://doc.rust-lang.org/stable/std/primitive.char.html#method.from_u32)
[char::from_digit] (https://doc.rust-lang.org/stable/std/primitive.char.html#method.from_digit)
[char::to_digit] (https://doc.rust-lang.org/stable/std/primitive.char.html#method.to_digit)
[core::char::from_u32] (https://doc.rust-lang.org/stable/core/char/fn.from_u32.html)
[core::char::from_digit] (https://doc.rust-lang.org/stable/core/char/fn.from_digit.html)

Compatibility Notes

[The layout of repr(Rust) types now groups m*2^n-sized fields with equivalently aligned ones.] (rust-lang/rust#102750) This is intended to be an optimization, but it is also known to increase type sizes in a few cases for the placement of enum tags. As a reminder, the layout of repr(Rust) types is an implementation detail, subject to change.
[0.5 now rounds to 0 when formatted to 0 decimal places.] (rust-lang/rust#102935) This makes it consistent with the rest of floating point formatting that rounds ties toward even digits.
[Chains of && and || will now drop temporaries from their sub-expressions in evaluation order, left-to-right.] (rust-lang/rust#103293) Previously, it was "twisted" such that the first expression dropped its temporaries last, after all of the other expressions dropped in order.
[Underscore suffixes on string literals are now a hard error.] (rust-lang/rust#103914) This has been a future-compatibility warning since 1.20.0.
[Stop passing -export-dynamic to wasm-ld.] (rust-lang/rust#105405)
[main is now mangled as __main_void on wasm32-wasi.] (rust-lang/rust#105468)
[Cargo now emits an error if there are multiple registries in the configuration with the same index URL.] (rust-lang/cargo#10592)

Internal Changes

These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools.

[Rewrite LLVM's archive writer in Rust.] (rust-lang/rust#97485)

Labels

A-testsuite

Area: The testsuite used to check the correctness of rustc

merged-by-bors

This PR was explicitly merged by bors.

S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

T-infra

Relevant to the infrastructure team, which will review and decide on the PR/issue.