Skip to content

Commit 2a83379

Browse files
committed
Switch to simpler strategy devised after discussing with mw.
Instead of accumulating (and acting upon) LTO import information over an unbounded number of prior compilations, just see if the curent import set matches the previous import set. if they don't match, then you cannot reuse the PostLTO build product for that module.
1 parent f29e4bd commit 2a83379

File tree

1 file changed

+33
-24
lines changed
  • src/librustc_codegen_llvm/back

1 file changed

+33
-24
lines changed

src/librustc_codegen_llvm/back/lto.rs

+33-24
Original file line numberDiff line numberDiff line change
@@ -479,7 +479,7 @@ fn thin_lto(cgcx: &CodegenContext<LlvmCodegenBackend>,
479479

480480
info!("thin LTO data created");
481481

482-
let (import_map_path, prev_import_map, mut curr_import_map) =
482+
let (import_map_path, prev_import_map, curr_import_map) =
483483
if let Some(ref incr_comp_session_dir) = cgcx.incr_comp_session_dir
484484
{
485485
let path = incr_comp_session_dir.join(THIN_LTO_IMPORTS_INCR_COMP_FILE_NAME);
@@ -522,35 +522,39 @@ fn thin_lto(cgcx: &CodegenContext<LlvmCodegenBackend>,
522522
for (module_index, module_name) in shared.module_names.iter().enumerate() {
523523
let module_name = module_name_to_str(module_name);
524524

525-
// If the module hasn't changed, and none of the modules it imported
526-
// from (*) has changed, then we can (1.) re-use the post-ThinLTO
527-
// version of the module, and thus (2.) save those previous ThinLTO
528-
// imports as its current set of imports.
525+
// If (1.) the module hasn't changed, and (2.) none of the modules
526+
// it imports from has changed, *and* (3.) the import-set itself has
527+
// not changed from the previous compile when it was last
528+
// ThinLTO'ed, then we can re-use the post-ThinLTO version of the
529+
// module. Otherwise, freshly perform LTO optimization.
529530
//
530-
// (*): that is, "imported from" at the time it was previously ThinLTO'ed;
531-
// see rust-lang/rust#59535.
531+
// This strategy means we can always save the computed imports as
532+
// canon: when we reuse the post-ThinLTO version, condition (3.)
533+
// ensures that the curent import set is the same as the previous
534+
// one. (And of course, when we don't reuse the post-ThinLTO
535+
// version, the current import set *is* the correct one, since we
536+
// are doing the ThinLTO in this current compilation cycle.)
532537
//
533-
// If we do *not* re-use the post-ThinLTO version of the module,
534-
// then we should save the computed imports that LLVM reported to us
535-
// during this cycle.
538+
// See rust-lang/rust#59535.
536539
if green_modules.contains_key(module_name) {
537540
assert!(cgcx.incr_comp_session_dir.is_some());
538541
assert!(prev_import_map.is_some());
539542

540543
let prev_import_map = prev_import_map.as_ref().unwrap();
544+
541545
let prev_imports = prev_import_map.modules_imported_by(module_name);
542-
let imports_all_green = prev_imports
546+
let curr_imports = curr_import_map.modules_imported_by(module_name);
547+
let imports_all_green = curr_imports
543548
.iter()
544549
.all(|imported_module| green_modules.contains_key(imported_module));
545550

546-
if imports_all_green {
551+
if imports_all_green && equivalent_as_sets(prev_imports, curr_imports) {
547552
let work_product = green_modules[module_name].clone();
548553
copy_jobs.push(work_product);
549554
info!(" - {}: re-used", module_name);
550555
assert!(cgcx.incr_comp_session_dir.is_some());
551556
cgcx.cgu_reuse_tracker.set_actual_reuse(module_name,
552557
CguReuse::PostLto);
553-
curr_import_map.overwrite_with_past_import_state(module_name, prev_imports);
554558
continue
555559
}
556560
}
@@ -574,6 +578,22 @@ fn thin_lto(cgcx: &CodegenContext<LlvmCodegenBackend>,
574578
}
575579
}
576580

581+
/// Given two slices, each with no repeat elements. returns true if and only if
582+
/// the two slices have the same contents when considered as sets (i.e. when
583+
/// element order is disregarded).
584+
fn equivalent_as_sets(a: &[String], b: &[String]) -> bool {
585+
// cheap path: unequal lengths means cannot possibly be set equivalent.
586+
if a.len() != b.len() { return false; }
587+
// fast path: before taking time to build up sorted clones, just see if the given inputs are equivant as is.
588+
if a == b { return true; }
589+
// slow path: compare sorted contents
590+
let mut a: Vec<&str> = a.iter().map(|s| s.as_str()).collect();
591+
let mut b: Vec<&str> = b.iter().map(|s| s.as_str()).collect();
592+
a.sort();
593+
b.sort();
594+
a == b
595+
}
596+
577597
pub(crate) fn run_pass_manager(cgcx: &CodegenContext<LlvmCodegenBackend>,
578598
module: &ModuleCodegen<ModuleLlvm>,
579599
config: &ModuleConfig,
@@ -871,17 +891,6 @@ pub struct ThinLTOImports {
871891
}
872892

873893
impl ThinLTOImports {
874-
/// Records `llvm_module_name` as importing `prev_imports` rather than what
875-
/// we have currently computed.
876-
fn overwrite_with_past_import_state(&mut self,
877-
llvm_module_name: &str,
878-
prev_imports: &[String]) {
879-
debug!("ThinLTOImports::overwrite_with_past_import_state \
880-
mod: {:?} replacing curr state: {:?} with prev state: {:?}",
881-
llvm_module_name, self.modules_imported_by(llvm_module_name), prev_imports);
882-
self.imports.insert(llvm_module_name.to_owned(), prev_imports.to_vec());
883-
}
884-
885894
fn modules_imported_by(&self, llvm_module_name: &str) -> &[String] {
886895
self.imports.get(llvm_module_name).map(|v| &v[..]).unwrap_or(&[])
887896
}

0 commit comments

Comments
 (0)