Skip to content

Commit

Permalink
Fix disagreeement about CGU reuse and LTO
Browse files Browse the repository at this point in the history
This commit fixes an issue where the codegen backend's selection of LTO
disagreed with what the codegen later thought was being done. Discovered
in #72006 we have a longstanding issue where if `-Clinker-plugin-lto` in
optimized mode is compiled incrementally it will always panic on the
second compilation. The underlying issue turned out to be that the
production of the original artifact determined that LTO should not be
done (because it's being postponed to the linker) but the CGU reuse
selection thought that LTO was done so it was trying to load pre-LTO
artifacts which were never generated.

The fix here is to ensure that the logic when generating code which
determines what kind of LTO is being done is shared amongst the CGU
reuse decision and the backend actually doing LTO. This means that
they'll both be in agreement about whether the previous compilation did
indeed produce incremental pre-LTO artifacts.

Closes #72006
  • Loading branch information
alexcrichton committed May 10, 2020
1 parent a51e004 commit c7bd5a6
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 35 deletions.
69 changes: 39 additions & 30 deletions src/librustc_codegen_ssa/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -715,38 +715,34 @@ fn execute_work_item<B: ExtraBackendMethods>(
}

// Actual LTO type we end up choosing based on multiple factors.
enum ComputedLtoType {
pub enum ComputedLtoType {
No,
Thin,
Fat,
}

fn execute_optimize_work_item<B: ExtraBackendMethods>(
cgcx: &CodegenContext<B>,
module: ModuleCodegen<B::Module>,
module_config: &ModuleConfig,
) -> Result<WorkItemResult<B>, FatalError> {
let diag_handler = cgcx.create_diag_handler();

unsafe {
B::optimize(cgcx, &diag_handler, &module, module_config)?;
pub fn compute_per_cgu_lto_type(
sess_lto: &Lto,
opts: &config::Options,
sess_crate_types: &[CrateType],
module_kind: ModuleKind,
) -> ComputedLtoType {
// Metadata modules never participate in LTO regardless of the lto
// settings.
if module_kind == ModuleKind::Metadata {
return ComputedLtoType::No;
}

// After we've done the initial round of optimizations we need to
// decide whether to synchronously codegen this module or ship it
// back to the coordinator thread for further LTO processing (which
// has to wait for all the initial modules to be optimized).

// If the linker does LTO, we don't have to do it. Note that we
// keep doing full LTO, if it is requested, as not to break the
// assumption that the output will be a single module.
let linker_does_lto = cgcx.opts.cg.linker_plugin_lto.enabled();
let linker_does_lto = opts.cg.linker_plugin_lto.enabled();

// When we're automatically doing ThinLTO for multi-codegen-unit
// builds we don't actually want to LTO the allocator modules if
// it shows up. This is due to various linker shenanigans that
// we'll encounter later.
let is_allocator = module.kind == ModuleKind::Allocator;
let is_allocator = module_kind == ModuleKind::Allocator;

// We ignore a request for full crate grath LTO if the cate type
// is only an rlib, as there is no full crate graph to process,
Expand All @@ -756,20 +752,33 @@ fn execute_optimize_work_item<B: ExtraBackendMethods>(
// require LTO so the request for LTO is always unconditionally
// passed down to the backend, but we don't actually want to do
// anything about it yet until we've got a final product.
let is_rlib = cgcx.crate_types.len() == 1 && cgcx.crate_types[0] == CrateType::Rlib;
let is_rlib = sess_crate_types.len() == 1 && sess_crate_types[0] == CrateType::Rlib;

// Metadata modules never participate in LTO regardless of the lto
// settings.
let lto_type = if module.kind == ModuleKind::Metadata {
ComputedLtoType::No
} else {
match cgcx.lto {
Lto::ThinLocal if !linker_does_lto && !is_allocator => ComputedLtoType::Thin,
Lto::Thin if !linker_does_lto && !is_rlib => ComputedLtoType::Thin,
Lto::Fat if !is_rlib => ComputedLtoType::Fat,
_ => ComputedLtoType::No,
}
};
match sess_lto {
Lto::ThinLocal if !linker_does_lto && !is_allocator => ComputedLtoType::Thin,
Lto::Thin if !linker_does_lto && !is_rlib => ComputedLtoType::Thin,
Lto::Fat if !is_rlib => ComputedLtoType::Fat,
_ => ComputedLtoType::No,
}
}

fn execute_optimize_work_item<B: ExtraBackendMethods>(
cgcx: &CodegenContext<B>,
module: ModuleCodegen<B::Module>,
module_config: &ModuleConfig,
) -> Result<WorkItemResult<B>, FatalError> {
let diag_handler = cgcx.create_diag_handler();

unsafe {
B::optimize(cgcx, &diag_handler, &module, module_config)?;
}

// After we've done the initial round of optimizations we need to
// decide whether to synchronously codegen this module or ship it
// back to the coordinator thread for further LTO processing (which
// has to wait for all the initial modules to be optimized).

let lto_type = compute_per_cgu_lto_type(&cgcx.lto, &cgcx.opts, &cgcx.crate_types, module.kind);

// If we're doing some form of incremental LTO then we need to be sure to
// save our module to disk first.
Expand Down
20 changes: 15 additions & 5 deletions src/librustc_codegen_ssa/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@
//! int)` and `rec(x=int, y=int, z=int)` will have the same `llvm::Type`.
use crate::back::write::{
start_async_codegen, submit_codegened_module_to_llvm, submit_post_lto_module_to_llvm,
submit_pre_lto_module_to_llvm, OngoingCodegen,
compute_per_cgu_lto_type, start_async_codegen, submit_codegened_module_to_llvm,
submit_post_lto_module_to_llvm, submit_pre_lto_module_to_llvm, ComputedLtoType, OngoingCodegen,
};
use crate::common::{IntPredicate, RealPredicate, TypeKind};
use crate::meth;
Expand Down Expand Up @@ -43,7 +43,7 @@ use rustc_middle::ty::layout::{FAT_PTR_ADDR, FAT_PTR_EXTRA};
use rustc_middle::ty::query::Providers;
use rustc_middle::ty::{self, Instance, Ty, TyCtxt};
use rustc_session::cgu_reuse_tracker::CguReuse;
use rustc_session::config::{self, EntryFnType, Lto};
use rustc_session::config::{self, EntryFnType};
use rustc_session::Session;
use rustc_span::Span;
use rustc_symbol_mangling::test as symbol_names_test;
Expand Down Expand Up @@ -941,8 +941,18 @@ fn determine_cgu_reuse<'tcx>(tcx: TyCtxt<'tcx>, cgu: &CodegenUnit<'tcx>) -> CguR
);

if tcx.dep_graph.try_mark_green(tcx, &dep_node).is_some() {
// We can re-use either the pre- or the post-thinlto state
if tcx.sess.lto() != Lto::No { CguReuse::PreLto } else { CguReuse::PostLto }
// We can re-use either the pre- or the post-thinlto state. If no LTO is
// being performed then we can use post-LTO artifacts, otherwise we must
// reuse pre-LTO artifacts
match compute_per_cgu_lto_type(
&tcx.sess.lto(),
&tcx.sess.opts,
&tcx.sess.crate_types.borrow(),
ModuleKind::Regular,
) {
ComputedLtoType::No => CguReuse::PostLto,
_ => CguReuse::PreLto,
}
} else {
CguReuse::No
}
Expand Down
9 changes: 9 additions & 0 deletions src/test/incremental/lto-in-linker.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
// revisions:cfail1 cfail2
// compile-flags: -Z query-dep-graph --crate-type rlib -C linker-plugin-lto -O
// no-prefer-dynamic
// build-pass

#![feature(rustc_attrs)]
#![rustc_partition_reused(module = "lto_in_linker", cfg = "cfail2")]

pub fn foo() {}
8 changes: 8 additions & 0 deletions src/test/incremental/rlib-lto.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// revisions:cfail1 cfail2
// compile-flags: -Z query-dep-graph --crate-type rlib -C lto
// build-pass

#![feature(rustc_attrs)]
#![rustc_partition_reused(module = "rlib_lto", cfg = "cfail2")]

pub fn foo() {}

0 comments on commit c7bd5a6

Please sign in to comment.