From 8371a036ea485ef8a830349c779e5acc116cfd8d Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 4 Jul 2022 14:38:42 +0100 Subject: [PATCH 1/3] incr: cache dwarf objects in work products Cache DWARF objects alongside object files in work products when those exist so that DWARF object files are available for thorin in packed mode in incremental scenarios. Signed-off-by: David Wood --- .../rustc_codegen_cranelift/src/driver/aot.rs | 11 ++- compiler/rustc_codegen_ssa/src/back/write.rs | 75 +++++++++++++------ .../rustc_incremental/src/persist/load.rs | 18 ++--- .../rustc_incremental/src/persist/save.rs | 11 +-- .../src/persist/work_product.rs | 46 ++++++------ .../rustc_query_system/src/dep_graph/graph.rs | 8 +- .../incremental/split_debuginfo_cached.rs | 25 +++++++ 7 files changed, 128 insertions(+), 66 deletions(-) create mode 100644 src/test/incremental/split_debuginfo_cached.rs diff --git a/compiler/rustc_codegen_cranelift/src/driver/aot.rs b/compiler/rustc_codegen_cranelift/src/driver/aot.rs index 05457ce15e9..50d8fc30d7d 100644 --- a/compiler/rustc_codegen_cranelift/src/driver/aot.rs +++ b/compiler/rustc_codegen_cranelift/src/driver/aot.rs @@ -66,7 +66,11 @@ fn emit_module( let work_product = if backend_config.disable_incr_cache { None } else { - rustc_incremental::copy_cgu_workproduct_to_incr_comp_cache_dir(tcx.sess, &name, &tmp_file) + rustc_incremental::copy_cgu_workproduct_to_incr_comp_cache_dir( + tcx.sess, + &name, + &[("o", &tmp_file)], + ) }; ModuleCodegenResult( @@ -82,7 +86,10 @@ fn reuse_workproduct_for_cgu( ) -> CompiledModule { let work_product = cgu.previous_work_product(tcx); let obj_out = tcx.output_filenames(()).temp_path(OutputType::Object, Some(cgu.name().as_str())); - let source_file = rustc_incremental::in_incr_comp_dir_sess(&tcx.sess, &work_product.saved_file); + let source_file = rustc_incremental::in_incr_comp_dir_sess( + &tcx.sess, + &work_product.saved_files.get("o").expect("no saved object file in work product"), + ); if let Err(err) = rustc_fs_util::link_or_copy(&source_file, &obj_out) { tcx.sess.err(&format!( "unable to copy {} to {}: {}", diff --git a/compiler/rustc_codegen_ssa/src/back/write.rs b/compiler/rustc_codegen_ssa/src/back/write.rs index 632f07c5c2d..f4a5cac872e 100644 --- a/compiler/rustc_codegen_ssa/src/back/write.rs +++ b/compiler/rustc_codegen_ssa/src/back/write.rs @@ -494,12 +494,18 @@ fn copy_all_cgu_workproducts_to_incr_comp_cache_dir( let _timer = sess.timer("copy_all_cgu_workproducts_to_incr_comp_cache_dir"); for module in compiled_modules.modules.iter().filter(|m| m.kind == ModuleKind::Regular) { - if let Some(path) = &module.object { - if let Some((id, product)) = - copy_cgu_workproduct_to_incr_comp_cache_dir(sess, &module.name, path) - { - work_products.insert(id, product); - } + let mut files = Vec::new(); + if let Some(object_file_path) = &module.object { + files.push(("o", object_file_path.as_path())); + } + if let Some(dwarf_object_file_path) = &module.dwarf_object { + files.push(("dwo", dwarf_object_file_path.as_path())); + } + + if let Some((id, product)) = + copy_cgu_workproduct_to_incr_comp_cache_dir(sess, &module.name, files.as_slice()) + { + work_products.insert(id, product); } } @@ -856,29 +862,50 @@ fn execute_copy_from_cache_work_item( assert!(module_config.emit_obj != EmitObj::None); let incr_comp_session_dir = cgcx.incr_comp_session_dir.as_ref().unwrap(); - let obj_out = cgcx.output_filenames.temp_path(OutputType::Object, Some(&module.name)); - let source_file = in_incr_comp_dir(&incr_comp_session_dir, &module.source.saved_file); - debug!( - "copying pre-existing module `{}` from {:?} to {}", - module.name, - source_file, - obj_out.display() + + let load_from_incr_comp_dir = |output_path: PathBuf, saved_path: &str| { + let source_file = in_incr_comp_dir(&incr_comp_session_dir, saved_path); + debug!( + "copying pre-existing module `{}` from {:?} to {}", + module.name, + source_file, + output_path.display() + ); + match link_or_copy(&source_file, &output_path) { + Ok(_) => Some(output_path), + Err(err) => { + let diag_handler = cgcx.create_diag_handler(); + diag_handler.err(&format!( + "unable to copy {} to {}: {}", + source_file.display(), + output_path.display(), + err + )); + None + } + } + }; + + let object = load_from_incr_comp_dir( + cgcx.output_filenames.temp_path(OutputType::Object, Some(&module.name)), + &module.source.saved_files.get("o").expect("no saved object file in work product"), ); - if let Err(err) = link_or_copy(&source_file, &obj_out) { - let diag_handler = cgcx.create_diag_handler(); - diag_handler.err(&format!( - "unable to copy {} to {}: {}", - source_file.display(), - obj_out.display(), - err - )); - } + let dwarf_object = + module.source.saved_files.get("dwo").as_ref().and_then(|saved_dwarf_object_file| { + let dwarf_obj_out = cgcx + .output_filenames + .split_dwarf_path(cgcx.split_debuginfo, cgcx.split_dwarf_kind, Some(&module.name)) + .expect( + "saved dwarf object in work product but `split_dwarf_path` returned `None`", + ); + load_from_incr_comp_dir(dwarf_obj_out, &saved_dwarf_object_file) + }); WorkItemResult::Compiled(CompiledModule { name: module.name, kind: ModuleKind::Regular, - object: Some(obj_out), - dwarf_object: None, + object, + dwarf_object, bytecode: None, }) } diff --git a/compiler/rustc_incremental/src/persist/load.rs b/compiler/rustc_incremental/src/persist/load.rs index 9c325faae80..f59d8d596b9 100644 --- a/compiler/rustc_incremental/src/persist/load.rs +++ b/compiler/rustc_incremental/src/persist/load.rs @@ -161,19 +161,13 @@ pub fn load_dep_graph(sess: &Session) -> DepGraphFuture { Decodable::decode(&mut work_product_decoder); for swp in work_products { - let mut all_files_exist = true; - let path = in_incr_comp_dir_sess(sess, &swp.work_product.saved_file); - if !path.exists() { - all_files_exist = false; - - if sess.opts.debugging_opts.incremental_info { - eprintln!( - "incremental: could not find file for work \ - product: {}", - path.display() - ); + let all_files_exist = swp.work_product.saved_files.iter().all(|(_, path)| { + let exists = in_incr_comp_dir_sess(sess, path).exists(); + if !exists && sess.opts.debugging_opts.incremental_info { + eprintln!("incremental: could not find file for work product: {path}",); } - } + exists + }); if all_files_exist { debug!("reconcile_work_products: all files for {:?} exist", swp); diff --git a/compiler/rustc_incremental/src/persist/save.rs b/compiler/rustc_incremental/src/persist/save.rs index 79836d66011..4059b7cfc8e 100644 --- a/compiler/rustc_incremental/src/persist/save.rs +++ b/compiler/rustc_incremental/src/persist/save.rs @@ -108,16 +108,17 @@ pub fn save_work_product_index( for (id, wp) in previous_work_products.iter() { if !new_work_products.contains_key(id) { work_product::delete_workproduct_files(sess, wp); - debug_assert!(!in_incr_comp_dir_sess(sess, &wp.saved_file).exists()); + debug_assert!( + !wp.saved_files.iter().all(|(_, path)| in_incr_comp_dir_sess(sess, path).exists()) + ); } } // Check that we did not delete one of the current work-products: debug_assert!({ - new_work_products - .iter() - .map(|(_, wp)| in_incr_comp_dir_sess(sess, &wp.saved_file)) - .all(|path| path.exists()) + new_work_products.iter().all(|(_, wp)| { + wp.saved_files.iter().all(|(_, path)| in_incr_comp_dir_sess(sess, path).exists()) + }) }); } diff --git a/compiler/rustc_incremental/src/persist/work_product.rs b/compiler/rustc_incremental/src/persist/work_product.rs index 4789c0f581f..1b184eca964 100644 --- a/compiler/rustc_incremental/src/persist/work_product.rs +++ b/compiler/rustc_incremental/src/persist/work_product.rs @@ -3,6 +3,7 @@ //! [work products]: WorkProduct use crate::persist::fs::*; +use rustc_data_structures::stable_map::FxHashMap; use rustc_fs_util::link_or_copy; use rustc_middle::dep_graph::{WorkProduct, WorkProductId}; use rustc_session::Session; @@ -13,38 +14,41 @@ use std::path::Path; pub fn copy_cgu_workproduct_to_incr_comp_cache_dir( sess: &Session, cgu_name: &str, - path: &Path, + files: &[(&'static str, &Path)], ) -> Option<(WorkProductId, WorkProduct)> { - debug!("copy_cgu_workproduct_to_incr_comp_cache_dir({:?},{:?})", cgu_name, path); + debug!(?cgu_name, ?files); sess.opts.incremental.as_ref()?; - let file_name = format!("{}.o", cgu_name); - let path_in_incr_dir = in_incr_comp_dir_sess(sess, &file_name); - let saved_file = match link_or_copy(path, &path_in_incr_dir) { - Ok(_) => file_name, - Err(err) => { - sess.warn(&format!( - "error copying object file `{}` to incremental directory as `{}`: {}", - path.display(), - path_in_incr_dir.display(), - err - )); - return None; + let mut saved_files = FxHashMap::default(); + for (ext, path) in files { + let file_name = format!("{cgu_name}.{ext}"); + let path_in_incr_dir = in_incr_comp_dir_sess(sess, &file_name); + match link_or_copy(path, &path_in_incr_dir) { + Ok(_) => { + let _ = saved_files.insert(ext.to_string(), file_name); + } + Err(err) => { + sess.warn(&format!( + "error copying object file `{}` to incremental directory as `{}`: {}", + path.display(), + path_in_incr_dir.display(), + err + )); + } } - }; - - let work_product = WorkProduct { cgu_name: cgu_name.to_string(), saved_file }; + } + let work_product = WorkProduct { cgu_name: cgu_name.to_string(), saved_files }; + debug!(?work_product); let work_product_id = WorkProductId::from_cgu_name(cgu_name); Some((work_product_id, work_product)) } /// Removes files for a given work product. pub fn delete_workproduct_files(sess: &Session, work_product: &WorkProduct) { - let path = in_incr_comp_dir_sess(sess, &work_product.saved_file); - match std_fs::remove_file(&path) { - Ok(()) => {} - Err(err) => { + for (_, path) in &work_product.saved_files { + let path = in_incr_comp_dir_sess(sess, path); + if let Err(err) = std_fs::remove_file(&path) { sess.warn(&format!( "file-system error deleting outdated file `{}`: {}", path.display(), diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index 341cf8f827b..3291717c550 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -886,8 +886,12 @@ impl DepGraph { #[derive(Clone, Debug, Encodable, Decodable)] pub struct WorkProduct { pub cgu_name: String, - /// Saved file associated with this CGU. - pub saved_file: String, + /// Saved files associated with this CGU. In each key/value pair, the value is the path to the + /// saved file and the key is some identifier for the type of file being saved. + /// + /// By convention, file extensions are currently used as identifiers, i.e. the key "o" maps to + /// the object file's path, and "dwo" to the dwarf object file's path. + pub saved_files: FxHashMap, } // Index type for `DepNodeData`'s edges. diff --git a/src/test/incremental/split_debuginfo_cached.rs b/src/test/incremental/split_debuginfo_cached.rs new file mode 100644 index 00000000000..25c802d5a1d --- /dev/null +++ b/src/test/incremental/split_debuginfo_cached.rs @@ -0,0 +1,25 @@ +// Check that compiling with packed Split DWARF twice succeeds. This should confirm that DWARF +// objects are cached as work products and available to the incremental compilation for `thorin` to +// pack into a DWARF package. + +// ignore-tidy-linelength +// only-x86_64-unknown-linux-gnu +// revisions:rpass1 rpass2 + +// [rpass1]compile-flags: -g -Zquery-dep-graph -Zunstable-options -Csplit-debuginfo=packed -Zsplit-dwarf-kind=split +// [rpass2]compile-flags: -g -Zquery-dep-graph -Zunstable-options -Csplit-debuginfo=packed -Zsplit-dwarf-kind=split + +#![feature(rustc_attrs)] +// For `rpass2`, nothing has changed so everything should re-used. +#![rustc_partition_reused(module = "split_debuginfo_cached", cfg = "rpass2")] +#![rustc_partition_reused(module = "split_debuginfo_cached-another_module", cfg = "rpass2")] + +mod another_module { + pub fn foo() -> &'static str { + "hello world" + } +} + +pub fn main() { + println!("{}", another_module::foo()); +} From fc641f21c2f1ba0dcf7bc7750844f097e25b3029 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 6 Jul 2022 10:15:45 +0100 Subject: [PATCH 2/3] ssa: remove dwo of metadata and allocator module Compiling with `-Csplit-debuginfo=packed` was leaving behind `.dwo` files because either the metadata or allocator module contained a DWARF object which was not being removed by the `maybe_remove_temps_from_module` closure. --- compiler/rustc_codegen_ssa/src/back/link.rs | 32 ++++++++++++--------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index 72aa790c363..c85c3bf5710 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -151,11 +151,23 @@ pub fn link_binary<'a, B: ArchiveBuilder<'a>>( return; } - let remove_temps_from_module = |module: &CompiledModule| { - if let Some(ref obj) = module.object { - ensure_removed(sess.diagnostic(), obj); - } - }; + let maybe_remove_temps_from_module = + |preserve_objects: bool, preserve_dwarf_objects: bool, module: &CompiledModule| { + if !preserve_objects { + if let Some(ref obj) = module.object { + ensure_removed(sess.diagnostic(), obj); + } + } + + if !preserve_dwarf_objects { + if let Some(ref dwo_obj) = module.dwarf_object { + ensure_removed(sess.diagnostic(), dwo_obj); + } + } + }; + + let remove_temps_from_module = + |module: &CompiledModule| maybe_remove_temps_from_module(false, false, module); // Otherwise, always remove the metadata and allocator module temporaries. if let Some(ref metadata_module) = codegen_results.metadata_module { @@ -177,15 +189,7 @@ pub fn link_binary<'a, B: ArchiveBuilder<'a>>( debug!(?preserve_objects, ?preserve_dwarf_objects); for module in &codegen_results.modules { - if !preserve_objects { - remove_temps_from_module(module); - } - - if !preserve_dwarf_objects { - if let Some(ref obj) = module.dwarf_object { - ensure_removed(sess.diagnostic(), obj); - } - } + maybe_remove_temps_from_module(preserve_objects, preserve_dwarf_objects, module); } }); From e1065239fe768dbad7ecbe37be39cb3204476a87 Mon Sep 17 00:00:00 2001 From: David Wood Date: Wed, 6 Jul 2022 10:56:08 +0100 Subject: [PATCH 3/3] ssa: abort if dwarf packaging fails This should have been here from the start... oops. When `thorin` fails to package a DWARF package, that should fail compilation. --- compiler/rustc_codegen_ssa/src/back/link.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/rustc_codegen_ssa/src/back/link.rs b/compiler/rustc_codegen_ssa/src/back/link.rs index c85c3bf5710..960e98243ac 100644 --- a/compiler/rustc_codegen_ssa/src/back/link.rs +++ b/compiler/rustc_codegen_ssa/src/back/link.rs @@ -653,6 +653,7 @@ fn link_dwarf_object<'a>( sess.struct_err("linking dwarf objects with thorin failed") .note(&format!("{:?}", e)) .emit(); + sess.abort_if_errors(); } } }