-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Handle LTO with an rlib/cdylib crate type #8254
Conversation
r? @ehuss (rust_highfive has picked a reviewer for you, use r? to override) |
bar = { path = 'bar' } | ||
registry-shared = "*" | ||
|
||
[profile.release] |
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.
The release here isn't necessary. You could use profile.dev
and omit the --release
further down
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.
Ah that's ok, these are just internal tests.
src/cargo/core/compiler/lto.rs
Outdated
// to fully embed bitcode rather than simply generating just bitcode. | ||
match lto_for_deps { | ||
Lto::None => (Lto::None, Lto::None), | ||
_ if unit.target.is_cdylib() => (Lto::EmbedBitcode, Lto::EmbedBitcode), |
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.
Should this also check for staticlib? I did a quick test, and trying to link a .a
file generated with -Clinker-plugin-lto
causes the linker to seg fault.
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.
Whoops yes indeed! I've tweaked this a bit to handle both in one go.
In the case that LTO is happening but we're also generating a cdylib/rlib simultatneously that means that the final artifact will use the rlib but the cdylib still needs to be produced. To get this to work the cdylib's dependency tree needs to be compiled with embedded bitcode. The cdylib itself will be linked with the linker because we can't LTO an rlib+cdylib compilation, but the final executable will need to load bitcode from all its deps. This is meant to address rust-lang/rust#72268
5fd347c
to
b45fd8f
Compare
👍 |
📌 Commit b45fd8f has been approved by |
☀️ Test successful - checks-azure |
Update cargo 9 commits in cb06cb2696df2567ce06d1a39b1b40612a29f853..500b2bd01c958f5a33b6aa3f080bea015877b83c 2020-05-08 21:57:44 +0000 to 2020-05-18 17:12:54 +0000 - Handle LTO with an rlib/cdylib crate type (rust-lang/cargo#8254) - Gracefully handle errors during a build. (rust-lang/cargo#8247) - Update `im-rc` to 15.0.0 (rust-lang/cargo#8255) - Fix `cargo update` with unused patch. (rust-lang/cargo#8243) - Rephrased error message for disallowed sections in virtual workspace (rust-lang/cargo#8200) - Ignore broken console output in some situations. (rust-lang/cargo#8236) - Expand error message to explain that a string was found (rust-lang/cargo#8235) - Add context to some fs errors. (rust-lang/cargo#8232) - Move SipHasher to an isolated module. (rust-lang/cargo#8233)
In the case that LTO is happening but we're also generating a
cdylib/rlib simultatneously that means that the final artifact will use
the rlib but the cdylib still needs to be produced. To get this to work
the cdylib's dependency tree needs to be compiled with embedded bitcode.
The cdylib itself will be linked with the linker because we can't LTO an
rlib+cdylib compilation, but the final executable will need to load
bitcode from all its deps.
This is meant to address rust-lang/rust#72268