From 882b50854cfdac67bd743ec9fd02464eca6edf15 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 8 May 2020 09:27:59 -0700 Subject: [PATCH] Fix disagreeement about CGU reuse and LTO 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 --- src/librustc_codegen_ssa/back/write.rs | 69 +++++++++++++++----------- src/librustc_codegen_ssa/base.rs | 20 ++++++-- src/test/incremental/lto-in-linker.rs | 8 +++ src/test/incremental/rlib-lto.rs | 8 +++ 4 files changed, 70 insertions(+), 35 deletions(-) create mode 100644 src/test/incremental/lto-in-linker.rs create mode 100644 src/test/incremental/rlib-lto.rs diff --git a/src/librustc_codegen_ssa/back/write.rs b/src/librustc_codegen_ssa/back/write.rs index 5c3444eff0a11..e5bd18ed680de 100644 --- a/src/librustc_codegen_ssa/back/write.rs +++ b/src/librustc_codegen_ssa/back/write.rs @@ -715,38 +715,34 @@ fn execute_work_item( } // Actual LTO type we end up choosing based on multiple factors. -enum ComputedLtoType { +pub enum ComputedLtoType { No, Thin, Fat, } -fn execute_optimize_work_item( - cgcx: &CodegenContext, - module: ModuleCodegen, - module_config: &ModuleConfig, -) -> Result, 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, @@ -756,20 +752,33 @@ fn execute_optimize_work_item( // 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( + cgcx: &CodegenContext, + module: ModuleCodegen, + module_config: &ModuleConfig, +) -> Result, 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. diff --git a/src/librustc_codegen_ssa/base.rs b/src/librustc_codegen_ssa/base.rs index c5b95905ea012..29398db6ff8a9 100644 --- a/src/librustc_codegen_ssa/base.rs +++ b/src/librustc_codegen_ssa/base.rs @@ -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; @@ -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; @@ -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 } diff --git a/src/test/incremental/lto-in-linker.rs b/src/test/incremental/lto-in-linker.rs new file mode 100644 index 0000000000000..6d21f267ace05 --- /dev/null +++ b/src/test/incremental/lto-in-linker.rs @@ -0,0 +1,8 @@ +// revisions:cfail1 cfail2 +// compile-flags: -Z query-dep-graph --crate-type rlib -C linker-plugin-lto -O +// build-pass + +#![feature(rustc_attrs)] +#![rustc_partition_reused(module = "lto_in_linker", cfg = "cfail2")] + +pub fn foo() {} diff --git a/src/test/incremental/rlib-lto.rs b/src/test/incremental/rlib-lto.rs new file mode 100644 index 0000000000000..752fee5a0d5b6 --- /dev/null +++ b/src/test/incremental/rlib-lto.rs @@ -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() {}