diff --git a/src/cargo/core/compiler/lto.rs b/src/cargo/core/compiler/lto.rs index c808520d39b..564d6f57fc6 100644 --- a/src/cargo/core/compiler/lto.rs +++ b/src/cargo/core/compiler/lto.rs @@ -1,6 +1,7 @@ use crate::core::compiler::{Context, Unit}; use crate::core::interning::InternedString; use crate::core::profiles; +use crate::core::TargetKind; use crate::util::errors::CargoResult; use std::collections::hash_map::{Entry, HashMap}; @@ -26,7 +27,7 @@ pub enum Lto { pub fn generate(cx: &mut Context<'_, '_>) -> CargoResult<()> { let mut map = HashMap::new(); for unit in cx.bcx.roots.iter() { - calculate(cx, &mut map, unit, false)?; + calculate(cx, &mut map, unit, Lto::None)?; } cx.lto = map; Ok(()) @@ -36,34 +37,49 @@ fn calculate( cx: &Context<'_, '_>, map: &mut HashMap, unit: &Unit, - require_bitcode: bool, + lto_for_deps: Lto, ) -> CargoResult<()> { - let (lto, require_bitcode_for_deps) = if unit.target.for_host() { + let (lto, lto_for_deps) = if unit.target.for_host() { // Disable LTO for host builds since we only really want to perform LTO // for the final binary, and LTO on plugins/build scripts/proc macros is // largely not desired. - (Lto::None, false) - } else if unit.target.can_lto() { - // Otherwise if this target can perform LTO then we're going to read the - // LTO value out of the profile. Note that we ignore `require_bitcode` + (Lto::None, Lto::None) + } else if unit.target.is_linkable() { + // A "linkable" target is one that produces and rlib or dylib in this + // case. In this scenario we cannot pass `-Clto` to the compiler because + // that is an invalid request, this is simply a dependency. What we do, + // however, is respect the request for whatever dependencies need to + // have. + // + // Here if no LTO is requested then we keep it turned off. Otherwise LTO + // is requested in some form, which means ideally we need just what's + // requested, nothing else. It's possible, though, to have libraries + // which are both a cdylib and and rlib, for example, which means that + // object files are getting sent to the linker. That means that we need + // to fully embed bitcode rather than simply generating just bitcode. + let has_non_linkable_lib = match unit.target.kind() { + TargetKind::Lib(kinds) => kinds.iter().any(|k| !k.is_linkable()), + _ => true, + }; + match lto_for_deps { + Lto::None => (Lto::None, Lto::None), + _ if has_non_linkable_lib => (Lto::EmbedBitcode, Lto::EmbedBitcode), + other => (other, other), + } + } else { + // Otherwise this target can perform LTO and we're going to read the + // LTO value out of the profile. Note that we ignore `lto_for_deps` // here because if a unit depends on another unit than can LTO this // isn't a rustc-level dependency but rather a Cargo-level dependency. // For example this is an integration test depending on a binary. match unit.profile.lto { profiles::Lto::Named(s) => match s.as_str() { - "n" | "no" | "off" => (Lto::Run(Some(s)), false), - _ => (Lto::Run(Some(s)), true), + "n" | "no" | "off" => (Lto::Run(Some(s)), Lto::None), + _ => (Lto::Run(Some(s)), Lto::OnlyBitcode), }, - profiles::Lto::Bool(true) => (Lto::Run(None), true), - profiles::Lto::Bool(false) => (Lto::None, false), + profiles::Lto::Bool(true) => (Lto::Run(None), Lto::OnlyBitcode), + profiles::Lto::Bool(false) => (Lto::None, Lto::None), } - } else if require_bitcode { - // Otherwise we're a dependency of something, an rlib. This means that - // if our parent required bitcode of some kind then we need to generate - // bitcode. - (Lto::OnlyBitcode, true) - } else { - (Lto::None, false) }; match map.entry(unit.clone()) { @@ -93,14 +109,12 @@ fn calculate( // either only-objects or only-bitcode we have to embed both in // rlibs (used for different compilations), so we switch to // embedding bitcode. - (Lto::OnlyBitcode, Lto::None) - | (Lto::OnlyBitcode, Lto::EmbedBitcode) - | (Lto::None, Lto::OnlyBitcode) - | (Lto::None, Lto::EmbedBitcode) => Lto::EmbedBitcode, + (Lto::OnlyBitcode, Lto::None) | (Lto::None, Lto::OnlyBitcode) => Lto::EmbedBitcode, - // Currently this variant is never calculated above, so no need - // to handle this case. - (Lto::EmbedBitcode, _) => unreachable!(), + // Once a target has requested bitcode embedding that's the + // maximal amount of work that can be done, so we just keep + // doing that work. + (Lto::EmbedBitcode, _) | (_, Lto::EmbedBitcode) => Lto::EmbedBitcode, }; // No need to recurse if we calculated the same value as before. if result == *v.get() { @@ -111,7 +125,7 @@ fn calculate( } for dep in cx.unit_deps(unit) { - calculate(cx, map, &dep.unit, require_bitcode_for_deps)?; + calculate(cx, map, &dep.unit, lto_for_deps)?; } Ok(()) } diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index 221b769295b..6978081a6fa 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -842,17 +842,6 @@ impl Target { self.kind().rustc_crate_types() } - pub fn can_lto(&self) -> bool { - match self.kind() { - TargetKind::Lib(v) => { - !v.contains(&CrateType::Rlib) - && !v.contains(&CrateType::Dylib) - && !v.contains(&CrateType::Lib) - } - _ => true, - } - } - pub fn set_tested(&mut self, tested: bool) -> &mut Target { Arc::make_mut(&mut self.inner).tested = tested; self diff --git a/tests/testsuite/lto.rs b/tests/testsuite/lto.rs index f5e2fb5bd68..99abbdefce9 100644 --- a/tests/testsuite/lto.rs +++ b/tests/testsuite/lto.rs @@ -336,3 +336,78 @@ fn test_all_and_bench() { .with_stderr_contains("[RUNNING] `rustc[..]--crate-name foo[..]-C lto[..]") .run(); } + +#[cargo_test] +fn cdylib_and_rlib() { + if !cargo_test_support::is_nightly() { + return; + } + + Package::new("registry", "0.0.1") + .file("src/lib.rs", "pub fn foo() {}") + .publish(); + Package::new("registry-shared", "0.0.1") + .file("src/lib.rs", "pub fn foo() {}") + .publish(); + + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "0.0.0" + + [workspace] + + [dependencies] + bar = { path = 'bar' } + registry-shared = "*" + + [profile.release] + lto = true + "#, + ) + .file( + "src/main.rs", + " + fn main() { + bar::foo(); + registry_shared::foo(); + } + ", + ) + .file( + "bar/Cargo.toml", + r#" + [package] + name = "bar" + version = "0.0.0" + + [dependencies] + registry = "*" + registry-shared = "*" + + [lib] + crate-type = ['cdylib', 'rlib'] + "#, + ) + .file( + "bar/src/lib.rs", + " + pub fn foo() { + registry::foo(); + registry_shared::foo(); + } + ", + ) + .file("tests/a.rs", "") + .file("bar/tests/b.rs", "") + .build(); + p.cargo("build --release -v").run(); + p.cargo("test --release -v").run(); + p.cargo("build --release -v --manifest-path bar/Cargo.toml") + .run(); + p.cargo("test --release -v --manifest-path bar/Cargo.toml") + .run(); +}