[experimental]: Build LLVM with ThinLTO enabled (2nd attempt) #53245
Conversation
This file contains 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
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
@bors try |
|
⌛ Trying commit c7e0eb1 with merge 48dabd2392ea669dc2ce78ca92e9bd524b623922... |
|
💔 Test failed - status-travis |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
bors
added
S-waiting-on-author
|
@bors retry (timeout) |
bors
added a commit
that referenced
this pull request
Aug 10, 2018
[experimental]: Build LLVM with ThinLTO enabled (2nd attempt) This is #51207 revived. This time, I'd like to run actual performance tests to see if it improves compile times.
|
💔 Test failed - status-travis |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
⌛ Trying commit c7e0eb1 with merge 3e0509a9d5dfc83ade7f68694b4efe692180be1c... |
|
@rust-lang/infra, is it possible the final binaries of LLVM are not cached? It looks like the last 5% (i.e. linking everything, including ThinLTO with these settings) takes 80 minutes |
|
💔 Test failed - status-travis |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
@aidanhs and @alexcrichton would be the most familiar, but hypothetically I'd imagine they should be or could be, we might need to add support to sccache though. |
|
@michaelwoerister correct yeah, we purely rely on One possible solution may be to build LLVM with a dynamic library, hopefully only performing that ThinLTO once. We'd still need a static version to be produced, however, to ThinLTO with rustc itself. I'm not sure whether this'll make the linking stage of rustc prohibitively expensive though... Is there a way to tell LLVM to not actually ThinLTO its binaries? We only want ThinLTO with rustc's codegen backend, right? |
Yes, that's my guess too. It's the same problem we've had with MIR-only RLIBS, all the optimization work is multiplied.
We already build the dylib, afaict. There seem to be options for making the tools link to LLVM dynamically (
The travis log suggests that it takes 10 minutes to build the codegen backend with ThinLTO:
Yeah we don't need the tools to be that optimized. Unfortunately, the "object files" that clang with ThinLTO produces are just semi-optimized bitcode files, so the linker would at least have to re-do codegen. The dylib approach is our best shot, I think. |
|
@bors try |
bors
added a commit
that referenced
this pull request
Aug 13, 2018
[experimental]: Build LLVM with ThinLTO enabled (2nd attempt) This is #51207 revived. This time, I'd like to run actual performance tests to see if it improves compile times.
|
Note to self: Disable ThinLTO for |
|
☀️ Test successful - status-travis |
|
Ah right, forgot about that! |
|
@michaelwoerister hm so one thing I just remembered, this doesn't change LLVM's building strategy for Windows, right? That means that on Windows we'd have to build LLVM statically, which means that all the LLVM tools would be built with ThinLTO separately, being prohibitively expensive as we saw earlier I think? I think I might personally be leaning towards reverting this until we can figure out a cross-platform story that fits with ThinLTO-with-rustc that all fits within our CI budget. It currently looks like ThinLTO-with-rustc will blow our CI budget on Linux (the fastest of the CI platforms) and enabling just this PR will blow the budget on Windows? |
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Sep 4, 2018
This commit tweaks the layout of a few components that we distribute to hopefully fix across all platforms the recent issues with LLD being unable to find the LLVM shared object. In rust-lang#53245 we switched to building LLVM as a dynamic library, which means that LLVM tools by default link to LLVM dynamically rather than statically. This in turn means that the tools, at runtime, need to find the LLVM shared library. LLVM's shared library is currently distributed as part of the rustc component. This library is located, however, at `$sysroot/lib`. The LLVM tools we ship are in two locations: * LLD is shipped at `$sysroot/lib/rustlib/$host/bin/rust-lld` * Other LLVM tools are shipped at `$sysroot/bin` Each LLVM tool has an embedded rpath directive indicating where it will search for dynamic libraries. This currently points to `../lib` and is presumably inserted by LLVM's build system. Unfortunately, though, this directive is only correct for the LLVM tools at `$sysroot/bin`, not LLD! This commit is targeted at fixing this situation by making two changes: * LLVM tools other than LLD are moved in the distribution to `$sysroot/lib/rustlib/$host/bin`. This moves them next to LLD and should position them for... * The LLVM shared object is moved to `$sysroot/lib/rustlib/$host/lib` Together this means that all tools should natively be able to find the shared object and the shared object should be installed all the time for the various tools. Overall this should... Closes rust-lang#53813
bors
added a commit
that referenced
this pull request
Sep 4, 2018
rustbuild: Tweak LLVM distribution layout This commit tweaks the layout of a few components that we distribute to hopefully fix across all platforms the recent issues with LLD being unable to find the LLVM shared object. In #53245 we switched to building LLVM as a dynamic library, which means that LLVM tools by default link to LLVM dynamically rather than statically. This in turn means that the tools, at runtime, need to find the LLVM shared library. LLVM's shared library is currently distributed as part of the rustc component. This library is located, however, at `$sysroot/lib`. The LLVM tools we ship are in two locations: * LLD is shipped at `$sysroot/lib/rustlib/$host/bin/rust-lld` * Other LLVM tools are shipped at `$sysroot/bin` Each LLVM tool has an embedded rpath directive indicating where it will search for dynamic libraries. This currently points to `../lib` and is presumably inserted by LLVM's build system. Unfortunately, though, this directive is only correct for the LLVM tools at `$sysroot/bin`, not LLD! This commit is targeted at fixing this situation by making two changes: * LLVM tools other than LLD are moved in the distribution to `$sysroot/lib/rustlib/$host/bin`. This moves them next to LLD and should position them for... * The LLVM shared object is moved to `$sysroot/lib/rustlib/$host/lib` Together this means that all tools should natively be able to find the shared object and the shared object should be installed all the time for the various tools. Overall this should... Closes #53813
|
We still link LLVM tools statically on Windows, yes. That's just because I haven't tried to link them dynamically though. It might just work. Has this change caused any visible problems on CI yet? |
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Sep 5, 2018
This commit tweaks the layout of a few components that we distribute to hopefully fix across all platforms the recent issues with LLD being unable to find the LLVM shared object. In rust-lang#53245 we switched to building LLVM as a dynamic library, which means that LLVM tools by default link to LLVM dynamically rather than statically. This in turn means that the tools, at runtime, need to find the LLVM shared library. LLVM's shared library is currently distributed as part of the rustc component. This library is located, however, at `$sysroot/lib`. The LLVM tools we ship are in two locations: * LLD is shipped at `$sysroot/lib/rustlib/$host/bin/rust-lld` * Other LLVM tools are shipped at `$sysroot/bin` Each LLVM tool has an embedded rpath directive indicating where it will search for dynamic libraries. This currently points to `../lib` and is presumably inserted by LLVM's build system. Unfortunately, though, this directive is only correct for the LLVM tools at `$sysroot/bin`, not LLD! This commit is targeted at fixing this situation by making two changes: * LLVM tools other than LLD are moved in the distribution to `$sysroot/lib/rustlib/$host/bin`. This moves them next to LLD and should position them for... * The LLVM shared object is moved to `$sysroot/lib/rustlib/$host/lib` Together this means that all tools should natively be able to find the shared object and the shared object should be installed all the time for the various tools. Overall this should... Closes rust-lang#53813
bors
added a commit
that referenced
this pull request
Sep 6, 2018
rustbuild: Tweak LLVM distribution layout This commit tweaks the layout of a few components that we distribute to hopefully fix across all platforms the recent issues with LLD being unable to find the LLVM shared object. In #53245 we switched to building LLVM as a dynamic library, which means that LLVM tools by default link to LLVM dynamically rather than statically. This in turn means that the tools, at runtime, need to find the LLVM shared library. LLVM's shared library is currently distributed as part of the rustc component. This library is located, however, at `$sysroot/lib`. The LLVM tools we ship are in two locations: * LLD is shipped at `$sysroot/lib/rustlib/$host/bin/rust-lld` * Other LLVM tools are shipped at `$sysroot/bin` Each LLVM tool has an embedded rpath directive indicating where it will search for dynamic libraries. This currently points to `../lib` and is presumably inserted by LLVM's build system. Unfortunately, though, this directive is only correct for the LLVM tools at `$sysroot/bin`, not LLD! This commit is targeted at fixing this situation by making two changes: * LLVM tools other than LLD are moved in the distribution to `$sysroot/lib/rustlib/$host/bin`. This moves them next to LLD and should position them for... * The LLVM shared object is moved to `$sysroot/lib/rustlib/$host/lib` Together this means that all tools should natively be able to find the shared object and the shared object should be installed all the time for the various tools. Overall this should... Closes #53813
|
@alexcrichton So, cross-language LTO does not seem to be worth the trouble: #53855 (comment). We can thus experiment with linking |
|
Bummer! That seems reasonable to me though, we've got solidly measured wins from this approach which is good! We may have shipped a dynamic version once yeah but tbh it's been so long I can't remember such a time! |
|
It seems that dynamically linking to LLVM causes trouble for LLDB on macOS (#54126). It would be best if we could do ThinLTO+codegen while assembling the static library (and not build the dynamic library at all). I don't know if that's possible though. |
|
Heh it's ironic because that's exactly how rustc does ThinLTO! We could maybe play around with LLD's |
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Dec 18, 2018
When building a distributed compiler on Linux where we use ThinLTO to create the LLVM shared object this commit switches the compiler to dynamically linking that LLVM artifact instead of statically linking to LLVM. The primary goal here is to reduce CI compile times, avoiding two+ ThinLTO builds of all of LLVM. By linking dynamically to LLVM we'll reuse the one ThinLTO step done by LLVM's build itself. Lots of discussion about this change can be found [here] and down. A perf run will show whether this is worth it or not! [here]: rust-lang#53245 (comment)
bors
added a commit
that referenced
this pull request
Dec 18, 2018
bootstrap: Link LLVM as a dylib with ThinLTO When building a distributed compiler on Linux where we use ThinLTO to create the LLVM shared object this commit switches the compiler to dynamically linking that LLVM artifact instead of statically linking to LLVM. The primary goal here is to reduce CI compile times, avoiding two+ ThinLTO builds of all of LLVM. By linking dynamically to LLVM we'll reuse the one ThinLTO step done by LLVM's build itself. Lots of discussion about this change can be found [here] and down. A perf run will show whether this is worth it or not! [here]: #53245 (comment)
bors
added a commit
that referenced
this pull request
Dec 18, 2018
bootstrap: Link LLVM as a dylib with ThinLTO When building a distributed compiler on Linux where we use ThinLTO to create the LLVM shared object this commit switches the compiler to dynamically linking that LLVM artifact instead of statically linking to LLVM. The primary goal here is to reduce CI compile times, avoiding two+ ThinLTO builds of all of LLVM. By linking dynamically to LLVM we'll reuse the one ThinLTO step done by LLVM's build itself. Lots of discussion about this change can be found [here] and down. A perf run will show whether this is worth it or not! [here]: #53245 (comment)
Centril
added a commit
to Centril/rust
that referenced
this pull request
Dec 24, 2018
…oerister bootstrap: Link LLVM as a dylib with ThinLTO When building a distributed compiler on Linux where we use ThinLTO to create the LLVM shared object this commit switches the compiler to dynamically linking that LLVM artifact instead of statically linking to LLVM. The primary goal here is to reduce CI compile times, avoiding two+ ThinLTO builds of all of LLVM. By linking dynamically to LLVM we'll reuse the one ThinLTO step done by LLVM's build itself. Lots of discussion about this change can be found [here] and down. A perf run will show whether this is worth it or not! [here]: rust-lang#53245 (comment)
alexcrichton
added a commit
to alexcrichton/rust
that referenced
this pull request
Jan 2, 2019
When building a distributed compiler on Linux where we use ThinLTO to create the LLVM shared object this commit switches the compiler to dynamically linking that LLVM artifact instead of statically linking to LLVM. The primary goal here is to reduce CI compile times, avoiding two+ ThinLTO builds of all of LLVM. By linking dynamically to LLVM we'll reuse the one ThinLTO step done by LLVM's build itself. Lots of discussion about this change can be found [here] and down. A perf run will show whether this is worth it or not! [here]: rust-lang#53245 (comment)
Mark-Simulacrum
added a commit
to Mark-Simulacrum/rust
that referenced
this pull request
Jan 3, 2019
…atsakis bootstrap: Link LLVM as a dylib with ThinLTO (take 2) When building a distributed compiler on Linux where we use ThinLTO to create the LLVM shared object this commit switches the compiler to dynamically linking that LLVM artifact instead of statically linking to LLVM. The primary goal here is to reduce CI compile times, avoiding two+ ThinLTO builds of all of LLVM. By linking dynamically to LLVM we'll reuse the one ThinLTO step done by LLVM's build itself. Lots of discussion about this change can be found [here] and down. A perf run will show whether this is worth it or not! [here]: rust-lang#53245 (comment) --- This PR previously landed in rust-lang#56944, caused rust-lang#57111, and was reverted in rust-lang#57116. I've added one more commit here which should fix the breakage that we saw.
bors
added a commit
that referenced
this pull request
Jan 6, 2019
bootstrap: Link LLVM as a dylib with ThinLTO (take 2) When building a distributed compiler on Linux where we use ThinLTO to create the LLVM shared object this commit switches the compiler to dynamically linking that LLVM artifact instead of statically linking to LLVM. The primary goal here is to reduce CI compile times, avoiding two+ ThinLTO builds of all of LLVM. By linking dynamically to LLVM we'll reuse the one ThinLTO step done by LLVM's build itself. Lots of discussion about this change can be found [here] and down. A perf run will show whether this is worth it or not! [here]: #53245 (comment) --- This PR previously landed in #56944, caused #57111, and was reverted in #57116. I've added one more commit here which should fix the breakage that we saw.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is #51207 revived. This time, I'd like to run actual performance tests to see if it improves compile times.