diff options
3 files changed, 121 insertions, 22 deletions
diff --git a/src/librustc_codegen_llvm/back/lto.rs b/src/librustc_codegen_llvm/back/lto.rs index 816329e06c7..e21cdee961d 100644 --- a/src/librustc_codegen_llvm/back/lto.rs +++ b/src/librustc_codegen_llvm/back/lto.rs @@ -463,15 +463,18 @@ fn thin_lto( // If previous imports have been deleted, or we get an IO error // reading the file storing them, then we'll just use `None` as the // prev_import_map, which will force the code to be recompiled. - let prev = - if path.exists() { ThinLTOImports::load_from_file(&path).ok() } else { None }; - let curr = ThinLTOImports::from_thin_lto_data(data); + let prev = if path.exists() { + ThinLTOImportMaps::load_from_file(&path).ok() + } else { + None + }; + let curr = ThinLTOImportMaps::from_thin_lto_data(data); (Some(path), prev, curr) } else { // If we don't compile incrementally, we don't need to load the // import data from LLVM. assert!(green_modules.is_empty()); - let curr = ThinLTOImports::default(); + let curr = ThinLTOImportMaps::default(); (None, None, curr) }; info!("thin LTO import map loaded"); @@ -497,10 +500,14 @@ fn thin_lto( let module_name = module_name_to_str(module_name); // If (1.) the module hasn't changed, and (2.) none of the modules - // it imports from has changed, *and* (3.) the import-set itself has - // not changed from the previous compile when it was last - // ThinLTO'ed, then we can re-use the post-ThinLTO version of the - // module. Otherwise, freshly perform LTO optimization. + // it imports from nor exports to have changed, *and* (3.) the + // import and export sets themselves have not changed from the + // previous compile when it was last ThinLTO'ed, then we can re-use + // the post-ThinLTO version of the module. Otherwise, freshly + // perform LTO optimization. + // + // (Note that globally, the export set is just the inverse of the + // import set.) // // This strategy means we can always save the computed imports as // canon: when we reuse the post-ThinLTO version, condition (3.) @@ -509,19 +516,30 @@ fn thin_lto( // version, the current import set *is* the correct one, since we // are doing the ThinLTO in this current compilation cycle.) // - // See rust-lang/rust#59535. + // For more discussion, see rust-lang/rust#59535 (where the import + // issue was discovered) and rust-lang/rust#69798 (where the + // analogous export issue was discovered). if let (Some(prev_import_map), true) = (prev_import_map.as_ref(), green_modules.contains_key(module_name)) { assert!(cgcx.incr_comp_session_dir.is_some()); - let prev_imports = prev_import_map.modules_imported_by(module_name); - let curr_imports = curr_import_map.modules_imported_by(module_name); + let prev_imports = prev_import_map.imports_of(module_name); + let curr_imports = curr_import_map.imports_of(module_name); + let prev_exports = prev_import_map.exports_of(module_name); + let curr_exports = curr_import_map.exports_of(module_name); let imports_all_green = curr_imports .iter() .all(|imported_module| green_modules.contains_key(imported_module)); + let exports_all_green = curr_exports + .iter() + .all(|exported_module| green_modules.contains_key(exported_module)); - if imports_all_green && equivalent_as_sets(prev_imports, curr_imports) { + if imports_all_green + && equivalent_as_sets(prev_imports, curr_imports) + && exports_all_green + && equivalent_as_sets(prev_exports, curr_exports) + { let work_product = green_modules[module_name].clone(); copy_jobs.push(work_product); info!(" - {}: re-used", module_name); @@ -881,17 +899,32 @@ pub unsafe fn optimize_thin_module( Ok(module) } +/// Summarizes module import/export relationships used by LLVM's ThinLTO pass. +/// +/// Note that we tend to have two such instances of `ThinLTOImportMaps` in use: +/// one loaded from a file that represents the relationships used during the +/// compilation associated with the incremetnal build artifacts we are +/// attempting to reuse, and another constructed via `from_thin_lto_data`, which +/// captures the relationships of ThinLTO in the current compilation. #[derive(Debug, Default)] -pub struct ThinLTOImports { +pub struct ThinLTOImportMaps { // key = llvm name of importing module, value = list of modules it imports from imports: FxHashMap<String, Vec<String>>, + // key = llvm name of exporting module, value = list of modules it exports to + exports: FxHashMap<String, Vec<String>>, } -impl ThinLTOImports { - fn modules_imported_by(&self, llvm_module_name: &str) -> &[String] { +impl ThinLTOImportMaps { + /// Returns modules imported by `llvm_module_name` during some ThinLTO pass. + fn imports_of(&self, llvm_module_name: &str) -> &[String] { self.imports.get(llvm_module_name).map(|v| &v[..]).unwrap_or(&[]) } + /// Returns modules exported by `llvm_module_name` during some ThinLTO pass. + fn exports_of(&self, llvm_module_name: &str) -> &[String] { + self.exports.get(llvm_module_name).map(|v| &v[..]).unwrap_or(&[]) + } + fn save_to_file(&self, path: &Path) -> io::Result<()> { use std::io::Write; let file = File::create(path)?; @@ -906,16 +939,20 @@ impl ThinLTOImports { Ok(()) } - fn load_from_file(path: &Path) -> io::Result<ThinLTOImports> { + fn load_from_file(path: &Path) -> io::Result<ThinLTOImportMaps> { use std::io::BufRead; let mut imports = FxHashMap::default(); - let mut current_module = None; - let mut current_imports = vec![]; + let mut exports: FxHashMap<_, Vec<_>> = FxHashMap::default(); + let mut current_module: Option<String> = None; + let mut current_imports: Vec<String> = vec![]; let file = File::open(path)?; for line in io::BufReader::new(file).lines() { let line = line?; if line.is_empty() { let importing_module = current_module.take().expect("Importing module not set"); + for imported in ¤t_imports { + exports.entry(imported.clone()).or_default().push(importing_module.clone()); + } imports.insert(importing_module, mem::replace(&mut current_imports, vec![])); } else if line.starts_with(' ') { // Space marks an imported module @@ -927,17 +964,17 @@ impl ThinLTOImports { current_module = Some(line.trim().to_string()); } } - Ok(ThinLTOImports { imports }) + Ok(ThinLTOImportMaps { imports, exports }) } /// Loads the ThinLTO import map from ThinLTOData. - unsafe fn from_thin_lto_data(data: *const llvm::ThinLTOData) -> ThinLTOImports { + unsafe fn from_thin_lto_data(data: *const llvm::ThinLTOData) -> ThinLTOImportMaps { unsafe extern "C" fn imported_module_callback( payload: *mut libc::c_void, importing_module_name: *const libc::c_char, imported_module_name: *const libc::c_char, ) { - let map = &mut *(payload as *mut ThinLTOImports); + let map = &mut *(payload as *mut ThinLTOImportMaps); let importing_module_name = CStr::from_ptr(importing_module_name); let importing_module_name = module_name_to_str(&importing_module_name); let imported_module_name = CStr::from_ptr(imported_module_name); @@ -951,8 +988,18 @@ impl ThinLTOImports { .get_mut(importing_module_name) .unwrap() .push(imported_module_name.to_owned()); + + if !map.exports.contains_key(imported_module_name) { + map.exports.insert(imported_module_name.to_owned(), vec![]); + } + + map.exports + .get_mut(imported_module_name) + .unwrap() + .push(importing_module_name.to_owned()); } - let mut map = ThinLTOImports::default(); + + let mut map = ThinLTOImportMaps::default(); llvm::LLVMRustGetThinLTOModuleImports( data, imported_module_callback, diff --git a/src/test/incremental/thinlto/cgu_invalidated_when_export_added.rs b/src/test/incremental/thinlto/cgu_invalidated_when_export_added.rs new file mode 100644 index 00000000000..4d48a5f0ac5 --- /dev/null +++ b/src/test/incremental/thinlto/cgu_invalidated_when_export_added.rs @@ -0,0 +1,26 @@ +// revisions: cfail1 cfail2 +// build-pass + +// rust-lang/rust#69798: +// +// This is analgous to cgu_invalidated_when_import_added, but it covers a +// problem uncovered where a change to the *export* set caused a link failure +// when reusing post-LTO optimized object code. + +pub struct Foo {} +impl Drop for Foo { + fn drop(&mut self) { + println!("Dropping Foo"); + } +} +#[no_mangle] +pub extern "C" fn run() { + thread_local! { pub static FOO : Foo = Foo { } ; } + + #[cfg(cfail2)] + { + FOO.with(|_f| ()) + } +} + +pub fn main() { run() } diff --git a/src/test/incremental/thinlto/cgu_invalidated_when_export_removed.rs b/src/test/incremental/thinlto/cgu_invalidated_when_export_removed.rs new file mode 100644 index 00000000000..e85b4856f3a --- /dev/null +++ b/src/test/incremental/thinlto/cgu_invalidated_when_export_removed.rs @@ -0,0 +1,26 @@ +// revisions: cfail1 cfail2 +// build-pass + +// rust-lang/rust#69798: +// +// This is analgous to cgu_invalidated_when_export_added, but it covers the +// other direction. This is analogous to cgu_invalidated_when_import_added: we +// include it, because it may uncover bugs in variant implementation strategies. + +pub struct Foo {} +impl Drop for Foo { + fn drop(&mut self) { + println!("Dropping Foo"); + } +} +#[no_mangle] +pub extern "C" fn run() { + thread_local! { pub static FOO : Foo = Foo { } ; } + + #[cfg(cfail1)] + { + FOO.with(|_f| ()) + } +} + +pub fn main() { run() } |