Skip to content
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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bootstrap: Link LLVM as a dylib with ThinLTO #56944

Merged
merged 4 commits into from
Dec 24, 2018

Conversation

alexcrichton
Copy link
Member

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!

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)
@rust-highfive
Copy link
Collaborator

r? @nikomatsakis

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

@alexcrichton
Copy link
Member Author

@bors: try

r? @michaelwoerister

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 18, 2018
@bors
Copy link
Contributor

bors commented Dec 18, 2018

⌛ Trying commit bd18a92 with merge 8c1161d...

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
Copy link
Contributor

bors commented Dec 18, 2018

💔 Test failed - status-travis

@bors 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-review Status: Awaiting review from the assignee but also interested parties. labels Dec 18, 2018
@rust-highfive
Copy link
Collaborator

The job dist-x86_64-linux-alt of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_fold:end:services

travis_fold:start:git.checkout
travis_time:start:08a1d7d3
$ git clone --depth=2 --branch=try https://github.com/rust-lang/rust.git rust-lang/rust
---
[01:25:10]   curl error: Could not resolve host: github.com
[01:25:10] ; class=Net (12)
[01:25:10] 
[01:25:10] 
[01:25:10] command did not execute successfully: "/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/bin/cargo" "install" "-j" "4" "--locked" "--color" "always" "--force" "--debug" "--vers" "0.1.22" "cargo-vendor"
[01:25:10] 
[01:25:10] 
[01:25:10] failed to run: /checkout/obj/build/bootstrap/debug/bootstrap dist --host x86_64-unknown-linux-gnu --target x86_64-unknown-linux-gnu
[01:25:10] Build completed unsuccessfully in 1:20:37
---
travis_time:end:01d69427:start=1545115281612526283,finish=1545115281624430147,duration=11903864
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:0fc8be9c
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_fold":start:crashlog\n\033[31;1m%s\033[0m\n" "$CORE"; gdb --batch -q -c "$CORE" "$EXE" -iex 'set auto-load off' -iex 'dir src/' -iex 'set sysroot .' -ex bt -ex q; echo travis_fold":"end:crashlog; fi; done || true
travis_fold:end:after_failure.4
travis_fold:start:after_failure.5
travis_time:start:12446463
travis_time:start:12446463
$ cat ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers || true
cat: ./obj/build/x86_64-unknown-linux-gnu/native/asan/build/lib/asan/clang_rt.asan-dynamic-i386.vers: No such file or directory
travis_fold:end:after_failure.5
travis_fold:start:after_failure.6
travis_time:start:2182a530
$ dmesg | grep -i kill

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 @TimNN. (Feature Requests)

@alexcrichton
Copy link
Member Author

@bors: retry

@bors
Copy link
Contributor

bors commented Dec 18, 2018

⌛ Trying commit bd18a92 with merge a454167...

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
Copy link
Contributor

bors commented Dec 18, 2018

☀️ Test successful - status-travis
State: approved= try=True

@alexcrichton
Copy link
Member Author

@rust-timer build a454167

@rust-timer
Copy link
Collaborator

Success: Queued a454167 with parent 041254b, comparison URL.

@rust-timer
Copy link
Collaborator

Finished benchmarking try commit a454167

@alexcrichton
Copy link
Member Author

@bors: try

@bors
Copy link
Contributor

bors commented Dec 18, 2018

⌛ Trying commit e31ba470cc9e6666cbbb8a08f24515478c94c56c with merge 06113fddee6ca05a38fc2a7bf288f715d0b72f5e...

@alexcrichton
Copy link
Member Author

@bors: try

@bors
Copy link
Contributor

bors commented Dec 18, 2018

⌛ Trying commit c383d38 with merge a8b3bd70936e9ce4e4db4aaf7b4a4cae6b8f3d39...

@rust-highfive
Copy link
Collaborator

The job x86_64-gnu-llvm-6.0 of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log.
travis_time:end:0bc29572:start=1545167080480896013,finish=1545167189172278866,duration=108691382853
$ git checkout -qf FETCH_HEAD
travis_fold:end:git.checkout

Encrypted environment variables have been removed for security reasons.
See https://docs.travis-ci.com/user/pull-requests/#pull-requests-and-security-restrictions
$ export SCCACHE_BUCKET=rust-lang-ci-sccache2
$ export SCCACHE_REGION=us-west-1
Setting environment variables from .travis.yml
$ export IMAGE=x86_64-gnu-llvm-6.0
---
travis_time:start:test_debuginfo
Check compiletest suite=debuginfo mode=debuginfo-both (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[00:54:48] 
[00:54:48] running 119 tests
[00:55:11] .iiiii...i.....i..i...i..i.i..i.ii..i.....i..i....i..........iiii.........i.i...ii...i.......ii.i.i. 100/119
[00:55:15] i......iii.i.....ii
[00:55:15] 
[00:55:15]  finished in 26.905
[00:55:15] travis_fold:end:test_debuginfo

---
Check compiletest suite=run-make-fulldeps mode=run-make (x86_64-unknown-linux-gnu -> x86_64-unknown-linux-gnu)
[01:21:45] 
[01:21:45] running 193 tests
[01:22:10] .................................................................................................... 100/193
[01:23:08] ..F.........................................................................................test [run-make] run-make-fulldeps/long-linker-command-lines has been running for over 60 seconds
d-initializers -pedantic -Wno-long-long -Wno-maybe-uninitialized -Wdelete-non-virtual-dtor -Wno-comment -ffunction-sections -fdata-sections -O2 -DNDEBUG  -fno-exceptions -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -c llvm-module-pass.so.cc -o /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/llvm-pass/llvm-pass/libllvm-module-pass.o
[01:25:48] ar crus /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/llvm-pass/llvm-pass/libllvm-module-pass.a /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/llvm-pass/llvm-pass/libllvm-module-pass.o
[01:25:48] LD_LIBRARY_PATH="/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/llvm-pass/llvm-pass:/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-bootstrap-tools/x86_64-unknown-linux-gnu/release/deps:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/lib:" '/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc' --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/llvm-pass/llvm-pass -L /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/llvm-pass/llvm-pass  plugin.rs -C prefer-dynamic 
[01:25:48] LD_LIBRARY_PATH="/checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/llvm-pass/llvm-pass:/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/lib:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0-bootstrap-tools/x86_64-unknown-linux-gnu/release/deps:/checkout/obj/build/x86_64-unknown-linux-gnu/stage0/lib:" '/checkout/obj/build/x86_64-unknown-linux-gnu/stage2/bin/rustc' --out-dir /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/llvm-pass/llvm-pass -L /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/llvm-pass/llvm-pass  main.rs
[01:25:48] Makefile:18: recipe for target 'all' failed
[01:25:48] 
[01:25:48] ------------------------------------------
[01:25:48] stderr:
[01:25:48] ------------------------------------------
[01:25:48] ------------------------------------------
[01:25:48] ar: `u' modifier ignored since `D' is the default (see `U')
[01:25:48] ar: `u' modifier ignored since `D' is the default (see `U')
[01:25:48] error: /checkout/obj/build/x86_64-unknown-linux-gnu/test/run-make-fulldeps/llvm-pass/llvm-pass/libsome_plugin.so: undefined symbol: _ZTVN10__cxxabiv120__si_class_type_infoE
[01:25:48]   --> main.rs:12:11
[01:25:48]    |
[01:25:48] 12 | #![plugin(some_plugin)]
[01:25:48] 
[01:25:48] error: aborting due to previous error
[01:25:48] 
[01:25:48] 
[01:25:48] make[1]: *** [all] Error 1
[01:25:48] ------------------------------------------
[01:25:48] 
[01:25:48] thread '[run-make] run-make-fulldeps/llvm-pass' panicked at 'explicit panic', src/tools/compiletest/src/runtest.rs:3252:9
[01:25:48] note: Run with `RUST_BACKTRACE=1` for a backtrace.
---
travis_time:end:0a174afd:start=1545172349018592907,finish=1545172349026639655,duration=8046748
travis_fold:end:after_failure.3
travis_fold:start:after_failure.4
travis_time:start:04d5f55d
$ ln -s . checkout && for CORE in obj/cores/core.*; do EXE=$(echo $CORE | sed 's|obj/cores/core\.[0-9]*\.!checkout!\(.*\)|\1|;y|!|/|'); if [ -f "$EXE" ]; then printf travis_f

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 @TimNN. (Feature Requests)

@bors
Copy link
Contributor

bors commented Dec 18, 2018

☀️ Test successful - status-travis
State: approved= try=True

@bors
Copy link
Contributor

bors commented Dec 20, 2018

📌 Commit 0feb680 has been approved by michaelwoerister

@bors 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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 20, 2018
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)
@bors
Copy link
Contributor

bors commented Dec 24, 2018

⌛ Testing commit 0feb680 with merge c17f55dea4162d8509f40e7765b0df7620e14f7c...

@Centril
Copy link
Contributor

Centril commented Dec 24, 2018

@bors retry

(Included in #57094)

bors added a commit that referenced this pull request Dec 24, 2018
Rollup of 10 pull requests

Successful merges:

 - #55470 (box: Add documentation for `From` impls)
 - #56242 (Add missing link in docs)
 - #56944 (bootstrap: Link LLVM as a dylib with ThinLTO)
 - #56978 (Add `std::os::fortanix_sgx` module)
 - #56985 (Allow testing pointers for inboundedness while forbidding dangling pointers)
 - #56986 (rustc: Move jemalloc from rustc_driver to rustc)
 - #57010 (Actually run compiletest tests on CI)
 - #57021 (Enable emission of alignment attrs for pointer params)
 - #57074 (Fix recursion limits)
 - #57085 (librustc_codegen_llvm: Don't eliminate empty structs in C ABI on linux-sparc64)

Failed merges:

r? @ghost
@rust-highfive

This comment has been minimized.

@bors
Copy link
Contributor

bors commented Dec 24, 2018

⌛ Testing commit 0feb680 with merge 50f3d6e...

@bors bors merged commit 0feb680 into rust-lang:master Dec 24, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Dec 25, 2018
…michaelwoerister"

This reverts commit f1051b5, reversing
changes made to 833e0b3.
@kennytm kennytm mentioned this pull request Dec 25, 2018
bors added a commit that referenced this pull request Dec 25, 2018
Revert #56944.

This should fix #57111, since #56944 is the only PR involving LLVM.

#57111 is caused by both the rustc and rust-std tarballs providing libLLVM.

r? @alexcrichton
@alexcrichton alexcrichton deleted the less-thin2 branch January 2, 2019 19:31
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
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants