-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
rustbuild: Update LLVM and enable ThinLTO #46008
Conversation
@bors r+ |
📌 Commit bf3320f has been approved by |
☔ The latest upstream changes (presumably #45897) made this pull request unmergeable. Please resolve the merge conflicts. |
bf3320f
to
e18dbaa
Compare
@bors: r=Mark-Simulacrum |
📌 Commit e18dbaa has been approved by |
⌛ Testing commit e18dbaa64dbc63737a0abc9432cac886b89c286c with merge 557e2ccf38ae65cb1522a7c755809104cb8b18a5... |
💔 Test failed - status-travis |
|
cc #44899 — This PR contains the fix for that issue as well. |
Looks like that was accidentally caused by rust-lang/compiler-builtins#206 |
in that we forgot this commit -- rust-lang/compiler-builtins@02b3734 |
e18dbaa
to
78e13ba
Compare
@bors: r=Mark-Simulacrum |
📌 Commit 78e13ba has been approved by |
⌛ Testing commit 78e13ba6b969cf81a331869b891abb286346d453 with merge a25d81b2707b712d6b3f6b16ad7e09bfcf995be2... |
💔 Test failed - status-travis |
@alexcrichton Given that ThinLTO also affects sanitizers (#45220), I'm not so sure it is spurious or not. Let's see what bors says. |
Turns out using LTO on Windows means you currently don't run any thread local destructors at all. Using ThinLTO just exposed this. Let's try again! @bors: r=Mark-Simulacrum |
📌 Commit 7bed281 has been approved by |
⌛ Testing commit 7bed281dc022806efd1104e91a727fb66470d916 with merge c7b11f98b67ce0761fcda5f77ce7b33eadddf716... |
💔 Test failed - status-travis |
The Perhaps add a
|
7bed281
to
9714458
Compare
@bors: r=Mark-Simulacrum |
📌 Commit 9714458 has been approved by |
⌛ Testing commit 9714458100b3b3e883d0814cc66b3bb53df323bf with merge 8b2526dd476884c6f2422fb3d58a2a19b0c46e1e... |
💔 Test failed - status-travis |
Turns out ThinLTO was internalizing this symbol and eliminating it. Worse yet if you compiled with LTO turns out no TLS destructors would run on Windows! The `#[used]` annotation should be a more bulletproof implementation (in the face of LTO) of preserving this symbol all the way through in LLVM and ensuring it makes it all the way to the linker which will take care of it.
9714458
to
95e9609
Compare
@bors: r=Mark-Simulacrum Woe unto thee who decides to add a test |
📌 Commit 95e9609 has been approved by |
rustbuild: Update LLVM and enable ThinLTO This commit updates LLVM to fix #45511 (https://reviews.llvm.org/D39981) and also reenables ThinLTO for libtest now that we shouldn't hit #45768. This also opportunistically enables ThinLTO for libstd which was previously blocked (#45661) on test failures related to debuginfo with a presumed cause of #45511. Closes #45511
☀️ Test successful - status-appveyor, status-travis |
This commit updates LLVM to fix #45511 (https://reviews.llvm.org/D39981) and
also reenables ThinLTO for libtest now that we shouldn't hit #45768. This also
opportunistically enables ThinLTO for libstd which was previously blocked
(#45661) on test failures related to debuginfo with a presumed cause of #45511.
Closes #45511