-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Support static linking LLVM with ThinLTO #69081
Conversation
Instead of the conditional-default behavior, we could just update every builder which sets |
Yes, I would prefer that we update the CI builders instead of having a conditional here. Also marking as relnotes as this is a user (well, distro)-visible change. This PR does not intend to change our CI configuration, right? Just make it possible to have statically linked ThinLTOd LLVM? (And our default on CI is shared linked ThinLTO?) |
fe15200
to
36024f6
Compare
Done.
Right.
Well, it looks like ThinLTO is only done on dist-x86_64-linux. (EDIT: But yes, anytime we did ThinLTO it was with shared linkage for LLVM.) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally changes seem good to go presuming the clarification I asked for in a comment is added and doesn't raise any new questions :)
36024f6
to
7222038
Compare
@bors r+ |
📌 Commit 7222038 has been approved by |
⌛ Testing commit 7222038 with merge aae3e134d90ce7c84a79dbd9259daa209816ce03... |
💥 Test timed out |
@bors retry |
⌛ Testing commit 7222038 with merge 9cbd89798f4232e5eebce6f3bcc8aaa7815cf963... |
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 |
💔 Test failed - checks-azure |
Triaged |
This is in the queue accidentally. |
@tmandry waiting on you to fix the test failure
|
☔ The latest upstream changes (presumably #71147) made this pull request unmergeable. Please resolve the merge conflicts. |
Closing this due to inactivity |
As discussed here, we'd like to support static linking LLVM with ThinLTO, even if that isn't what happens on our CI bots. This changes the
llvm.link-shared
config flag to default totrue
ifllvm.thinlto
is enabled, but allows the user to override that default instead of forcing shared linking.r? @Mark-Simulacrum
cc @alexcrichton @petrhosek