Skip to content

Commit 882b508

Browse files
committed
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
1 parent a51e004 commit 882b508

File tree

4 files changed

+70
-35
lines changed

4 files changed

+70
-35
lines changed

src/librustc_codegen_ssa/back/write.rs

+39-30
Original file line numberDiff line numberDiff line change
@@ -715,38 +715,34 @@ fn execute_work_item<B: ExtraBackendMethods>(
715715
}
716716

717717
// Actual LTO type we end up choosing based on multiple factors.
718-
enum ComputedLtoType {
718+
pub enum ComputedLtoType {
719719
No,
720720
Thin,
721721
Fat,
722722
}
723723

724-
fn execute_optimize_work_item<B: ExtraBackendMethods>(
725-
cgcx: &CodegenContext<B>,
726-
module: ModuleCodegen<B::Module>,
727-
module_config: &ModuleConfig,
728-
) -> Result<WorkItemResult<B>, FatalError> {
729-
let diag_handler = cgcx.create_diag_handler();
730-
731-
unsafe {
732-
B::optimize(cgcx, &diag_handler, &module, module_config)?;
724+
pub fn compute_per_cgu_lto_type(
725+
sess_lto: &Lto,
726+
opts: &config::Options,
727+
sess_crate_types: &[CrateType],
728+
module_kind: ModuleKind,
729+
) -> ComputedLtoType {
730+
// Metadata modules never participate in LTO regardless of the lto
731+
// settings.
732+
if module_kind == ModuleKind::Metadata {
733+
return ComputedLtoType::No;
733734
}
734735

735-
// After we've done the initial round of optimizations we need to
736-
// decide whether to synchronously codegen this module or ship it
737-
// back to the coordinator thread for further LTO processing (which
738-
// has to wait for all the initial modules to be optimized).
739-
740736
// If the linker does LTO, we don't have to do it. Note that we
741737
// keep doing full LTO, if it is requested, as not to break the
742738
// assumption that the output will be a single module.
743-
let linker_does_lto = cgcx.opts.cg.linker_plugin_lto.enabled();
739+
let linker_does_lto = opts.cg.linker_plugin_lto.enabled();
744740

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

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

761-
// Metadata modules never participate in LTO regardless of the lto
762-
// settings.
763-
let lto_type = if module.kind == ModuleKind::Metadata {
764-
ComputedLtoType::No
765-
} else {
766-
match cgcx.lto {
767-
Lto::ThinLocal if !linker_does_lto && !is_allocator => ComputedLtoType::Thin,
768-
Lto::Thin if !linker_does_lto && !is_rlib => ComputedLtoType::Thin,
769-
Lto::Fat if !is_rlib => ComputedLtoType::Fat,
770-
_ => ComputedLtoType::No,
771-
}
772-
};
757+
match sess_lto {
758+
Lto::ThinLocal if !linker_does_lto && !is_allocator => ComputedLtoType::Thin,
759+
Lto::Thin if !linker_does_lto && !is_rlib => ComputedLtoType::Thin,
760+
Lto::Fat if !is_rlib => ComputedLtoType::Fat,
761+
_ => ComputedLtoType::No,
762+
}
763+
}
764+
765+
fn execute_optimize_work_item<B: ExtraBackendMethods>(
766+
cgcx: &CodegenContext<B>,
767+
module: ModuleCodegen<B::Module>,
768+
module_config: &ModuleConfig,
769+
) -> Result<WorkItemResult<B>, FatalError> {
770+
let diag_handler = cgcx.create_diag_handler();
771+
772+
unsafe {
773+
B::optimize(cgcx, &diag_handler, &module, module_config)?;
774+
}
775+
776+
// After we've done the initial round of optimizations we need to
777+
// decide whether to synchronously codegen this module or ship it
778+
// back to the coordinator thread for further LTO processing (which
779+
// has to wait for all the initial modules to be optimized).
780+
781+
let lto_type = compute_per_cgu_lto_type(&cgcx.lto, &cgcx.opts, &cgcx.crate_types, module.kind);
773782

774783
// If we're doing some form of incremental LTO then we need to be sure to
775784
// save our module to disk first.

src/librustc_codegen_ssa/base.rs

+15-5
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@
1414
//! int)` and `rec(x=int, y=int, z=int)` will have the same `llvm::Type`.
1515
1616
use crate::back::write::{
17-
start_async_codegen, submit_codegened_module_to_llvm, submit_post_lto_module_to_llvm,
18-
submit_pre_lto_module_to_llvm, OngoingCodegen,
17+
compute_per_cgu_lto_type, start_async_codegen, submit_codegened_module_to_llvm,
18+
submit_post_lto_module_to_llvm, submit_pre_lto_module_to_llvm, ComputedLtoType, OngoingCodegen,
1919
};
2020
use crate::common::{IntPredicate, RealPredicate, TypeKind};
2121
use crate::meth;
@@ -43,7 +43,7 @@ use rustc_middle::ty::layout::{FAT_PTR_ADDR, FAT_PTR_EXTRA};
4343
use rustc_middle::ty::query::Providers;
4444
use rustc_middle::ty::{self, Instance, Ty, TyCtxt};
4545
use rustc_session::cgu_reuse_tracker::CguReuse;
46-
use rustc_session::config::{self, EntryFnType, Lto};
46+
use rustc_session::config::{self, EntryFnType};
4747
use rustc_session::Session;
4848
use rustc_span::Span;
4949
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
941941
);
942942

943943
if tcx.dep_graph.try_mark_green(tcx, &dep_node).is_some() {
944-
// We can re-use either the pre- or the post-thinlto state
945-
if tcx.sess.lto() != Lto::No { CguReuse::PreLto } else { CguReuse::PostLto }
944+
// We can re-use either the pre- or the post-thinlto state. If no LTO is
945+
// being performed then we can use post-LTO artifacts, otherwise we must
946+
// reuse pre-LTO artifacts
947+
match compute_per_cgu_lto_type(
948+
&tcx.sess.lto(),
949+
&tcx.sess.opts,
950+
&tcx.sess.crate_types.borrow(),
951+
ModuleKind::Regular,
952+
) {
953+
ComputedLtoType::No => CguReuse::PostLto,
954+
_ => CguReuse::PreLto,
955+
}
946956
} else {
947957
CguReuse::No
948958
}

src/test/incremental/lto-in-linker.rs

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// revisions:cfail1 cfail2
2+
// compile-flags: -Z query-dep-graph --crate-type rlib -C linker-plugin-lto -O
3+
// build-pass
4+
5+
#![feature(rustc_attrs)]
6+
#![rustc_partition_reused(module = "lto_in_linker", cfg = "cfail2")]
7+
8+
pub fn foo() {}

src/test/incremental/rlib-lto.rs

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// revisions:cfail1 cfail2
2+
// compile-flags: -Z query-dep-graph --crate-type rlib -C lto
3+
// build-pass
4+
5+
#![feature(rustc_attrs)]
6+
#![rustc_partition_reused(module = "rlib_lto", cfg = "cfail2")]
7+
8+
pub fn foo() {}

0 commit comments

Comments
 (0)