rustc: Add `-C lto=val` option #47521

alexcrichton · 2018-01-17T17:52:36Z

This commit primarily adds the ability to control what kind of LTO happens when
rustc performs LTO, namely allowing values to be specified to the -C lto
option, such as -C lto=thin and -C lto=fat. (where "fat" is the previous
kind of LTO, throw everything in one giant module)

Along the way this also refactors a number of fields which store information
about whether LTO/ThinLTO are enabled to unify them all into one field through
which everything is dispatched, hopefully removing a number of special cases
throughout.

This is intended to help mitigate #47409 but will require a backport as well,
and this would unfortunately need to be an otherwise insta-stable option.

rust-highfive · 2018-01-17T17:52:39Z

r? @arielb1

(rust_highfive has picked a reviewer for you, use r? to override)

alexcrichton · 2018-01-17T17:54:19Z

r? @michaelwoerister

Mark-Simulacrum · 2018-01-17T18:29:07Z

src/librustc/session/config.rs

+
+    /// Do a local graph LTO with ThinLTO (only relevant for multiple codegen
+    /// units).
+    AutoThin,


Maybe LocalThin?

ThinLocal is what I would do.

Mark-Simulacrum · 2018-01-17T18:31:35Z

src/librustc/session/mod.rs

+
+        // If there's only one codegen unit and LTO isn't enabled then there's
+        // no need for ThinLTO so just return false.
+        if self.codegen_units() == 1 {


Hm, so I sort of feel like this logic should be in trans somewhere instead of here. Though it doesn't really matter.

Actually it would make sense to move that to trans at some point because we only really know after CGU partitioning how many object files there are. At moment it's likely that there's always more than one though.

Mark-Simulacrum · 2018-01-17T18:32:19Z

src/librustc_trans/back/write.rs

-        crate_types[0] == config::CrateTypeRlib;
+    // let crate_types = sess.crate_types.borrow();
+    // let only_rlib = crate_types.len() == 1 &&
+    //     crate_types[0] == config::CrateTypeRlib;


Forgotten comments?

michaelwoerister

Thanks, @alexcrichton! I left some comments about the implementation. We'll need buy-in from the whole @rust-lang/compiler team before merging, I think.

michaelwoerister · 2018-01-18T12:48:20Z

src/librustc/session/mod.rs

+
+        // If there's only one codegen unit and LTO isn't enabled then there's
+        // no need for ThinLTO so just return false.
+        if self.codegen_units() == 1 {


Actually it would make sense to move that to trans at some point because we only really know after CGU partitioning how many object files there are. At moment it's likely that there's always more than one though.

michaelwoerister · 2018-01-18T12:51:52Z

src/librustc_trans/back/lto.rs

+        Lto::AutoThin => SymbolExportLevel::Rust,
+
+        // We're doing LTO for the entire crate graph
+        _ => symbol_export::crates_export_threshold(&cgcx.crate_types),


Could you make this match exhaustive?

alexcrichton · 2018-01-18T15:36:55Z

Ok I've updated! This PR also now switches the default (the meaning of -C lto) back to "fat" LTO by default as discussed during the compiler triage meeting.

r? @michaelwoerister

This commit reverts a small portion of the switch to ThinLTO by default which changed the meaning of `-C lto` from "put the whole crate graph into one codegen unit" to "perform ThinLTO over the whole crate graph". This backport has no corresponding commit on master as rust-lang#47521 is making the same change but in a slightly different manner. This commit is intended to be a surgical change with low impact on beta. Closes rust-lang#47409

michaelwoerister · 2018-01-19T11:48:36Z

src/librustc/session/config.rs

+    No,
+
+    /// Do a full crate graph LTO. The flavor is determined by the compiler
+    /// (currently the default is "thin").


michaelwoerister · 2018-01-19T11:49:00Z

Thanks, Alex! r=me with the comment updated.

alexcrichton · 2018-01-19T14:54:08Z

@bors: r=michaelwoerister

bors · 2018-01-19T14:54:08Z

📌 Commit 43c5f62 has been approved by michaelwoerister

estebank · 2018-01-19T19:23:46Z

@bors r-

@alexcrichton https://travis-ci.org/rust-lang/rust/builds/330845572#L1220:

[00:19:06] Assembling stage1 compiler (x86_64-unknown-linux-gnu)
stage1-std
28.95sBuilding stage1 std artifacts (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[00:19:07]    Compiling cfg-if v0.1.2
[00:19:07]    Compiling cc v1.0.4
[00:19:07]    Compiling libc v0.2.36
[00:19:07]    Compiling unwind v0.0.0 (file:///checkout/src/libunwind)
[00:19:07]    Compiling core v0.0.0 (file:///checkout/src/libcore)
[00:19:08]    Compiling filetime v0.1.15
[00:19:10]    Compiling build_helper v0.1.0 (file:///checkout/src/build_helper)
[00:19:12]    Compiling std v0.0.0 (file:///checkout/src/libstd)
[00:19:13]    Compiling compiler_builtins v0.0.0 (file:///checkout/src/rustc/compiler_builtins_shim)
[00:19:13]    Compiling cmake v0.1.29
[00:19:16]    Compiling alloc_jemalloc v0.0.0 (file:///checkout/src/liballoc_jemalloc)
[00:19:18]    Compiling rustc_lsan v0.0.0 (file:///checkout/src/librustc_lsan)
[00:19:19]    Compiling rustc_msan v0.0.0 (file:///checkout/src/librustc_msan)
[00:19:20]    Compiling rustc_tsan v0.0.0 (file:///checkout/src/librustc_tsan)
[00:19:20]    Compiling rustc_asan v0.0.0 (file:///checkout/src/librustc_asan)
[00:19:35] LLVM ERROR: ThinLTO not available
[00:19:35] error: Could not compile `core`.
[00:19:35] 
[00:19:35] Caused by:
[00:19:35]   process didn't exit successfully: `/checkout/obj/build/bootstrap/debug/rustc --crate-name core libcore/lib.rs --color always --error-format json --crate-type lib --emit=dep-info,link -C opt-level=2 -C metadata=488df6a35606ef3a -C extra-filename=-488df6a35606ef3a --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps --target x86_64-unknown-linux-gnu -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/x86_64-unknown-linux-gnu/release/deps -L dependency=/checkout/obj/build/x86_64-unknown-linux-gnu/stage1-std/release/deps` (exit code: 1)
[00:19:35] thread 'main' panicked at 'command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "build" "--target" "x86_64-unknown-linux-gnu" "--release" "--locked" "--color" "always" "--features" "panic-unwind jemalloc backtrace" "--manifest-path" "/checkout/src/libstd/Cargo.toml" "--message-format" "json"
[00:19:35] expected success, got: exit code: 101', bootstrap/compile.rs:881:9
[00:19:35] note: Run with `RUST_BACKTRACE=1` for a backtrace.

alexcrichton · 2018-01-19T19:36:43Z

@bors: r=michaelwoerister

bors · 2018-01-19T19:36:44Z

📌 Commit 7517c66 has been approved by michaelwoerister

estebank · 2018-01-19T20:11:12Z

@alexcrichton https://travis-ci.org/rust-lang/rust/builds/330958787#L1202:

[00:16:46] error: unnecessary `unsafe` block
[00:16:46]     --> librustc_trans/back/write.rs:1307:35
[00:16:46]      |
[00:16:46] 1274 |         unsafe {
[00:16:46]      |         ------ because it's nested under this `unsafe` block
[00:16:46] ...
[00:16:46] 1307 |                 Lto::ThinLocal => unsafe {
[00:16:46]      |                                   ^^^^^^ unnecessary `unsafe` block
[00:16:46]      |
[00:16:46] note: lint level defined here
[00:16:46]     --> librustc_trans/lib.rs:20:9
[00:16:46]      |
[00:16:46] 20   | #![deny(warnings)]
[00:16:46]      |         ^^^^^^^^
[00:16:46]      = note: #[deny(unused_unsafe)] implied by #[deny(warnings)]
[00:16:46] 
[00:16:46] error: aborting due to previous error
[00:16:46] 
[00:16:46] error: Could not compile `rustc_trans`.

alexcrichton · 2018-01-19T20:17:16Z

@bors: r=michaelwoerister

wheeeeeee

bors · 2018-01-19T20:17:17Z

📌 Commit ee63bc6 has been approved by michaelwoerister

bors · 2018-01-21T12:09:41Z

☔ The latest upstream changes (presumably #45684) made this pull request unmergeable. Please resolve the merge conflicts.

alexcrichton · 2018-01-22T15:41:02Z

@bors: r=michaelwoerister

bors · 2018-01-22T15:41:03Z

📌 Commit dc902b1 has been approved by michaelwoerister

bors · 2018-01-23T20:01:01Z

☔ The latest upstream changes (presumably #47678) made this pull request unmergeable. Please resolve the merge conflicts.

This commit primarily adds the ability to control what kind of LTO happens when rustc performs LTO, namely allowing values to be specified to the `-C lto` option, such as `-C lto=thin` and `-C lto=fat`. (where "fat" is the previous kind of LTO, throw everything in one giant module) Along the way this also refactors a number of fields which store information about whether LTO/ThinLTO are enabled to unify them all into one field through which everything is dispatched, hopefully removing a number of special cases throughout. This is intended to help mitigate rust-lang#47409 but will require a backport as well, and this would unfortunately need to be an otherwise insta-stable option.

alexcrichton · 2018-01-23T22:14:03Z

@bors: r=michaelwoerister

bors · 2018-01-23T22:14:03Z

📌 Commit 8bde2ac has been approved by michaelwoerister

Rollup of 19 pull requests - Successful merges: #47415, #47437, #47439, #47453, #47460, #47502, #47529, #47600, #47607, #47618, #47626, #47656, #47668, #47696, #47701, #47705, #47710, #47711, #47719 - Failed merges: #47455, #47521

bors · 2018-01-26T20:36:52Z

☔ The latest upstream changes (presumably #47748) made this pull request unmergeable. Please resolve the merge conflicts.

rust-highfive assigned arielb1 Jan 17, 2018

alexcrichton mentioned this pull request Jan 17, 2018

Some DCE issues with ThinLTO leaving in construction of unused parameters #47409

Closed

rust-highfive assigned michaelwoerister and unassigned arielb1 Jan 17, 2018

kennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 17, 2018

Mark-Simulacrum requested changes Jan 17, 2018

View reviewed changes

michaelwoerister reviewed Jan 18, 2018

View reviewed changes

alexcrichton force-pushed the configure-lto branch from 41ce39b to d45a5b7 Compare January 18, 2018 15:35

alexcrichton mentioned this pull request Jan 18, 2018

[beta] Turn back on "fat" LTO by default #47548

Merged

kennytm added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 19, 2018

michaelwoerister reviewed Jan 19, 2018

View reviewed changes

alexcrichton force-pushed the configure-lto branch from d45a5b7 to 43c5f62 Compare January 19, 2018 14:54

Marwes mentioned this pull request Jan 19, 2018

Performance regressions of compiled code over the last year #47561

Open

alexcrichton force-pushed the configure-lto branch 2 times, most recently from 34c2fa8 to 7517c66 Compare January 19, 2018 19:36

alexcrichton force-pushed the configure-lto branch from 7517c66 to ee63bc6 Compare January 19, 2018 20:17

kennytm removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 22, 2018

alexcrichton force-pushed the configure-lto branch from 626921e to dc902b1 Compare January 22, 2018 15:40

kennytm 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jan 22, 2018

alexcrichton force-pushed the configure-lto branch from dc902b1 to 8bde2ac Compare January 23, 2018 22:13

alexcrichton mentioned this pull request Jan 25, 2018

Rollup of 19 pull requests #47748

Merged

alexcrichton mentioned this pull request Jan 26, 2018

ThinLTO bloats size of bare metal programs by up to 1200% #47770

Closed

bors merged commit 8bde2ac into rust-lang:master Jan 26, 2018

alexcrichton deleted the configure-lto branch January 26, 2018 21:47

alexcrichton mentioned this pull request Jan 26, 2018

Allow configuration of LTO in [profile] rust-lang/cargo#4984

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rustc: Add `-C lto=val` option #47521

rustc: Add `-C lto=val` option #47521

alexcrichton commented Jan 17, 2018

rust-highfive commented Jan 17, 2018

alexcrichton commented Jan 17, 2018

Mark-Simulacrum Jan 17, 2018

michaelwoerister Jan 18, 2018

Mark-Simulacrum Jan 17, 2018

michaelwoerister Jan 18, 2018

Mark-Simulacrum Jan 17, 2018

michaelwoerister left a comment

michaelwoerister Jan 18, 2018

michaelwoerister Jan 18, 2018

alexcrichton commented Jan 18, 2018

michaelwoerister Jan 19, 2018

michaelwoerister commented Jan 19, 2018

alexcrichton commented Jan 19, 2018

bors commented Jan 19, 2018

estebank commented Jan 19, 2018

alexcrichton commented Jan 19, 2018

bors commented Jan 19, 2018

estebank commented Jan 19, 2018

alexcrichton commented Jan 19, 2018

bors commented Jan 19, 2018

bors commented Jan 21, 2018

alexcrichton commented Jan 22, 2018

bors commented Jan 22, 2018

bors commented Jan 23, 2018

alexcrichton commented Jan 23, 2018

bors commented Jan 23, 2018

bors commented Jan 26, 2018

rustc: Add -C lto=val option #47521

rustc: Add -C lto=val option #47521

Conversation

alexcrichton commented Jan 17, 2018

rust-highfive commented Jan 17, 2018

alexcrichton commented Jan 17, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelwoerister left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexcrichton commented Jan 18, 2018

Choose a reason for hiding this comment

michaelwoerister commented Jan 19, 2018

alexcrichton commented Jan 19, 2018

bors commented Jan 19, 2018

estebank commented Jan 19, 2018

alexcrichton commented Jan 19, 2018

bors commented Jan 19, 2018

estebank commented Jan 19, 2018

alexcrichton commented Jan 19, 2018

bors commented Jan 19, 2018

bors commented Jan 21, 2018

alexcrichton commented Jan 22, 2018

bors commented Jan 22, 2018

bors commented Jan 23, 2018

alexcrichton commented Jan 23, 2018

bors commented Jan 23, 2018

bors commented Jan 26, 2018

rustc: Add `-C lto=val` option #47521

rustc: Add `-C lto=val` option #47521