From 2f9fff21911a3e419b21e56dba145bf0deab6f81 Mon Sep 17 00:00:00 2001 From: Niko Matsakis Date: Mon, 25 Jul 2016 10:51:14 -0400 Subject: [PATCH] Keep multiple files per work-product In the older version, a `.o` and ` .bc` file were separate work-products. This newer version keeps, for each codegen-unit, a set of files of different kinds. We assume that if any kinds are available then all the kinds we need are available, since the precise set of switches will depend on attributes and command-line switches. Should probably test this: the effect of changing attributes in particular might not be successfully tracked? --- src/librustc/dep_graph/dep_node.rs | 4 +- src/librustc/dep_graph/graph.rs | 7 +- src/librustc/session/config.rs | 23 ++++--- src/librustc_incremental/persist/load.rs | 31 ++++++--- .../persist/work_product.rs | 68 ++++++++++--------- src/librustc_trans/back/write.rs | 56 ++++++++++----- src/librustc_trans/base.rs | 8 +-- src/librustc_trans/context.rs | 15 ++-- src/librustc_trans/lib.rs | 12 +++- src/librustc_trans/partitioning.rs | 7 +- .../rlib_cross_crate/auxiliary/a.rs | 25 +++++++ src/test/incremental/rlib_cross_crate/b.rs | 38 +++++++++++ 12 files changed, 202 insertions(+), 92 deletions(-) create mode 100644 src/test/incremental/rlib_cross_crate/auxiliary/a.rs create mode 100644 src/test/incremental/rlib_cross_crate/b.rs diff --git a/src/librustc/dep_graph/dep_node.rs b/src/librustc/dep_graph/dep_node.rs index dd7f0286574..c9247539990 100644 --- a/src/librustc/dep_graph/dep_node.rs +++ b/src/librustc/dep_graph/dep_node.rs @@ -246,7 +246,5 @@ impl DepNode { /// the need to be mapped or unmapped. (This ensures we can serialize /// them even in the absence of a tcx.) #[derive(Clone, Debug, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)] -pub enum WorkProductId { - PartitionObjectFile(String), // see (*TransPartition) below -} +pub struct WorkProductId(pub String); diff --git a/src/librustc/dep_graph/graph.rs b/src/librustc/dep_graph/graph.rs index 8691894b325..bb027b11b45 100644 --- a/src/librustc/dep_graph/graph.rs +++ b/src/librustc/dep_graph/graph.rs @@ -10,6 +10,7 @@ use hir::def_id::DefId; use rustc_data_structures::fnv::FnvHashMap; +use session::config::OutputType; use std::cell::{Ref, RefCell}; use std::rc::Rc; use std::sync::Arc; @@ -157,11 +158,11 @@ impl DepGraph { /// previous hash. If it matches up, we can reuse the object file. #[derive(Clone, Debug, RustcEncodable, RustcDecodable)] pub struct WorkProduct { - /// extra hash used to decide if work-product is still suitable; + /// Extra hash used to decide if work-product is still suitable; /// note that this is *not* a hash of the work-product itself. /// See documentation on `WorkProduct` type for an example. pub input_hash: u64, - /// filename storing this work-product (found in the incr. comp. directory) - pub file_name: String, + /// Saved files associated with this CGU + pub saved_files: Vec<(OutputType, String)>, } diff --git a/src/librustc/session/config.rs b/src/librustc/session/config.rs index a0c2416d24c..690395399ef 100644 --- a/src/librustc/session/config.rs +++ b/src/librustc/session/config.rs @@ -61,7 +61,7 @@ pub enum DebugInfoLevel { FullDebugInfo, } -#[derive(Clone, Copy, PartialEq, Eq, Hash)] +#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, RustcEncodable, RustcDecodable)] pub enum OutputType { Bitcode, Assembly, @@ -105,6 +105,17 @@ impl OutputType { OutputType::DepInfo => "dep-info", } } + + pub fn extension(&self) -> &'static str { + match *self { + OutputType::Bitcode => "bc", + OutputType::Assembly => "s", + OutputType::LlvmAssembly => "ll", + OutputType::Object => "o", + OutputType::DepInfo => "d", + OutputType::Exe => "", + } + } } #[derive(Clone)] @@ -215,15 +226,7 @@ impl OutputFilenames { flavor: OutputType, codegen_unit_name: Option<&str>) -> PathBuf { - let extension = match flavor { - OutputType::Bitcode => "bc", - OutputType::Assembly => "s", - OutputType::LlvmAssembly => "ll", - OutputType::Object => "o", - OutputType::DepInfo => "d", - OutputType::Exe => "", - }; - + let extension = flavor.extension(); self.temp_path_ext(extension, codegen_unit_name) } diff --git a/src/librustc_incremental/persist/load.rs b/src/librustc_incremental/persist/load.rs index 6b856459aab..36b6c79c40f 100644 --- a/src/librustc_incremental/persist/load.rs +++ b/src/librustc_incremental/persist/load.rs @@ -260,11 +260,20 @@ fn reconcile_work_products<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, debug!("reconcile_work_products: dep-node for {:?} is dirty", swp); delete_dirty_work_product(tcx, swp); } else { - let path = in_incr_comp_dir(tcx.sess, &swp.work_product.file_name).unwrap(); - if path.exists() { + let all_files_exist = + swp.work_product + .saved_files + .iter() + .all(|&(_, ref file_name)| { + let path = in_incr_comp_dir(tcx.sess, &file_name).unwrap(); + path.exists() + }); + if all_files_exist { + debug!("reconcile_work_products: all files for {:?} exist", swp); tcx.dep_graph.insert_previous_work_product(&swp.id, swp.work_product); } else { - debug!("reconcile_work_products: file for {:?} does not exist", swp); + debug!("reconcile_work_products: some file for {:?} does not exist", swp); + delete_dirty_work_product(tcx, swp); } } } @@ -273,13 +282,15 @@ fn reconcile_work_products<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, fn delete_dirty_work_product(tcx: TyCtxt, swp: SerializedWorkProduct) { debug!("delete_dirty_work_product({:?})", swp); - let path = in_incr_comp_dir(tcx.sess, &swp.work_product.file_name).unwrap(); - match fs::remove_file(&path) { - Ok(()) => { } - Err(err) => { - tcx.sess.warn( - &format!("file-system error deleting outdated file `{}`: {}", - path.display(), err)); + for &(_, ref file_name) in &swp.work_product.saved_files { + let path = in_incr_comp_dir(tcx.sess, file_name).unwrap(); + match fs::remove_file(&path) { + Ok(()) => { } + Err(err) => { + tcx.sess.warn( + &format!("file-system error deleting outdated file `{}`: {}", + path.display(), err)); + } } } } diff --git a/src/librustc_incremental/persist/work_product.rs b/src/librustc_incremental/persist/work_product.rs index 7695291bf62..c106ea8f262 100644 --- a/src/librustc_incremental/persist/work_product.rs +++ b/src/librustc_incremental/persist/work_product.rs @@ -13,47 +13,51 @@ use persist::util::*; use rustc::dep_graph::{WorkProduct, WorkProductId}; use rustc::session::Session; +use rustc::session::config::OutputType; use rustc::util::fs::link_or_copy; -use std::fs; -use std::path::Path; +use std::path::PathBuf; use std::sync::Arc; pub fn save_trans_partition(sess: &Session, - partition_name: &str, + cgu_name: &str, partition_hash: u64, - path_to_obj_file: &Path) { - debug!("save_trans_partition({:?},{},{})", - partition_name, + files: &[(OutputType, PathBuf)]) { + debug!("save_trans_partition({:?},{},{:?})", + cgu_name, partition_hash, - path_to_obj_file.display()); + files); if sess.opts.incremental.is_none() { return; } - let id = Arc::new(WorkProductId::PartitionObjectFile(partition_name.to_string())); - let file_name = format!("cgu-{}", partition_name); - let path_in_incr_dir = in_incr_comp_dir(sess, &file_name).unwrap(); + let work_product_id = Arc::new(WorkProductId(cgu_name.to_string())); - // try to delete the file if it already exists - // - // FIXME(#34955) we can be smarter here -- if we are re-using, no need to do anything - if path_in_incr_dir.exists() { - let _ = fs::remove_file(&path_in_incr_dir); - } + let saved_files: Option> = + files.iter() + .map(|&(kind, ref path)| { + let file_name = format!("cgu-{}.{}", cgu_name, kind.extension()); + let path_in_incr_dir = in_incr_comp_dir(sess, &file_name).unwrap(); + match link_or_copy(path, &path_in_incr_dir) { + Ok(_) => Some((kind, file_name)), + Err(err) => { + sess.warn(&format!("error copying object file `{}` \ + to incremental directory as `{}`: {}", + path.display(), + path_in_incr_dir.display(), + err)); + None + } + } + }) + .collect(); + let saved_files = match saved_files { + Some(v) => v, + None => return, + }; - match link_or_copy(path_to_obj_file, &path_in_incr_dir) { - Ok(_) => { - let work_product = WorkProduct { - input_hash: partition_hash, - file_name: file_name, - }; - sess.dep_graph.insert_work_product(&id, work_product); - } - Err(err) => { - sess.warn(&format!("error copying object file `{}` \ - to incremental directory as `{}`: {}", - path_to_obj_file.display(), - path_in_incr_dir.display(), - err)); - } - } + let work_product = WorkProduct { + input_hash: partition_hash, + saved_files: saved_files, + }; + + sess.dep_graph.insert_work_product(&work_product_id, work_product); } diff --git a/src/librustc_trans/back/write.rs b/src/librustc_trans/back/write.rs index 08d7b531c2f..4b9d5dd9e8d 100644 --- a/src/librustc_trans/back/write.rs +++ b/src/librustc_trans/back/write.rs @@ -337,6 +337,8 @@ struct CodegenContext<'a> { remark: Passes, // Worker thread number worker: usize, + // Directory where incremental data is stored (if any) + incremental: Option, } impl<'a> CodegenContext<'a> { @@ -347,6 +349,7 @@ impl<'a> CodegenContext<'a> { plugin_passes: sess.plugin_llvm_passes.borrow().clone(), remark: sess.opts.cg.remark.clone(), worker: 0, + incremental: sess.opts.incremental.clone(), } } } @@ -612,7 +615,7 @@ unsafe fn optimize_and_codegen(cgcx: &CodegenContext, if copy_bc_to_obj { debug!("copying bitcode {:?} to obj {:?}", bc_out, obj_out); - if let Err(e) = fs::copy(&bc_out, &obj_out) { + if let Err(e) = link_or_copy(&bc_out, &obj_out) { cgcx.handler.err(&format!("failed to copy bitcode to object file: {}", e)); } } @@ -754,9 +757,19 @@ pub fn run_passes(sess: &Session, // If in incr. comp. mode, preserve the `.o` files for potential re-use for mtrans in trans.modules.iter() { - let path_to_obj = crate_output.temp_path(OutputType::Object, Some(&mtrans.name)); - debug!("wrote module {:?} to {:?}", mtrans.name, path_to_obj); - save_trans_partition(sess, &mtrans.name, mtrans.symbol_name_hash, &path_to_obj); + let mut files = vec![]; + + if modules_config.emit_obj { + let path = crate_output.temp_path(OutputType::Object, Some(&mtrans.name)); + files.push((OutputType::Object, path)); + } + + if modules_config.emit_bc { + let path = crate_output.temp_path(OutputType::Bitcode, Some(&mtrans.name)); + files.push((OutputType::Bitcode, path)); + } + + save_trans_partition(sess, &mtrans.name, mtrans.symbol_name_hash, &files); } // All codegen is finished. @@ -941,20 +954,24 @@ fn execute_work_item(cgcx: &CodegenContext, work_item.config, work_item.output_names); } - ModuleSource::Preexisting(ref buf) => { - let obj_out = work_item.output_names.temp_path(OutputType::Object, - Some(&work_item.mtrans.name)); - debug!("copying pre-existing module `{}` from {} to {}", - work_item.mtrans.name, - buf.display(), - obj_out.display()); - match link_or_copy(buf, &obj_out) { - Ok(()) => { } - Err(err) => { - cgcx.handler.err(&format!("unable to copy {} to {}: {}", - buf.display(), - obj_out.display(), - err)); + ModuleSource::Preexisting(wp) => { + let incremental = cgcx.incremental.as_ref().unwrap(); + let name = &work_item.mtrans.name; + for (kind, saved_file) in wp.saved_files { + let obj_out = work_item.output_names.temp_path(kind, Some(name)); + let source_file = incremental.join(&saved_file); + debug!("copying pre-existing module `{}` from {:?} to {}", + work_item.mtrans.name, + source_file, + obj_out.display()); + match link_or_copy(&source_file, &obj_out) { + Ok(()) => { } + Err(err) => { + cgcx.handler.err(&format!("unable to copy {} to {}: {}", + source_file.display(), + obj_out.display(), + err)); + } } } } @@ -994,6 +1011,8 @@ fn run_work_multithreaded(sess: &Session, let mut tx = Some(tx); futures.push(rx); + let incremental = sess.opts.incremental.clone(); + thread::Builder::new().name(format!("codegen-{}", i)).spawn(move || { let diag_handler = Handler::with_emitter(true, false, box diag_emitter); @@ -1005,6 +1024,7 @@ fn run_work_multithreaded(sess: &Session, plugin_passes: plugin_passes, remark: remark, worker: i, + incremental: incremental, }; loop { diff --git a/src/librustc_trans/base.rs b/src/librustc_trans/base.rs index e73c4f41c94..5a19ddff746 100644 --- a/src/librustc_trans/base.rs +++ b/src/librustc_trans/base.rs @@ -43,10 +43,9 @@ use rustc::ty::subst::{self, Substs}; use rustc::traits; use rustc::ty::{self, Ty, TyCtxt, TypeFoldable}; use rustc::ty::adjustment::CustomCoerceUnsized; -use rustc::dep_graph::DepNode; +use rustc::dep_graph::{DepNode, WorkProduct}; use rustc::hir::map as hir_map; use rustc::util::common::time; -use rustc_incremental::in_incr_comp_dir; use rustc::mir::mir_map::MirMap; use rustc_data_structures::graph::OUTGOING; use session::config::{self, NoDebugInfo, FullDebugInfo}; @@ -103,7 +102,6 @@ use std::cell::{Cell, RefCell}; use std::collections::HashMap; use std::ptr; use std::rc::Rc; -use std::path::PathBuf; use std::str; use std::{i8, i16, i32, i64}; use syntax_pos::{Span, DUMMY_SP}; @@ -2721,7 +2719,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, fn trans_reuse_previous_work_products(tcx: TyCtxt, codegen_units: &[CodegenUnit], symbol_map: &SymbolMap) - -> Vec> { + -> Vec> { debug!("trans_reuse_previous_work_products()"); codegen_units .iter() @@ -2735,7 +2733,7 @@ fn trans_reuse_previous_work_products(tcx: TyCtxt, if let Some(work_product) = tcx.dep_graph.previous_work_product(&id) { if work_product.input_hash == hash { debug!("trans_reuse_previous_work_products: reusing {:?}", work_product); - return Some(in_incr_comp_dir(tcx.sess, &work_product.file_name).unwrap()); + return Some(work_product); } else { debug!("trans_reuse_previous_work_products: \ not reusing {:?} because hash changed to {:?}", diff --git a/src/librustc_trans/context.rs b/src/librustc_trans/context.rs index bf696298212..a8f8474e940 100644 --- a/src/librustc_trans/context.rs +++ b/src/librustc_trans/context.rs @@ -10,7 +10,7 @@ use llvm; use llvm::{ContextRef, ModuleRef, ValueRef, BuilderRef}; -use rustc::dep_graph::{DepNode, DepTrackingMap, DepTrackingMapConfig}; +use rustc::dep_graph::{DepNode, DepTrackingMap, DepTrackingMapConfig, WorkProduct}; use middle::cstore::LinkMeta; use rustc::hir::def::ExportMap; use rustc::hir::def_id::DefId; @@ -40,7 +40,6 @@ use util::nodemap::{NodeMap, NodeSet, DefIdMap, FnvHashMap, FnvHashSet}; use std::ffi::{CStr, CString}; use std::cell::{Cell, RefCell}; -use std::path::PathBuf; use std::marker::PhantomData; use std::ptr; use std::rc::Rc; @@ -96,7 +95,7 @@ pub struct SharedCrateContext<'a, 'tcx: 'a> { pub struct LocalCrateContext<'tcx> { llmod: ModuleRef, llcx: ContextRef, - previous_work_product: Option, + previous_work_product: Option, tn: TypeNames, // FIXME: This seems to be largely unused. codegen_unit: CodegenUnit<'tcx>, needs_unwind_cleanup_cache: RefCell, bool>>, @@ -202,13 +201,13 @@ pub struct CrateContextList<'a, 'tcx: 'a> { impl<'a, 'tcx: 'a> CrateContextList<'a, 'tcx> { pub fn new(shared_ccx: &'a SharedCrateContext<'a, 'tcx>, codegen_units: Vec>, - previous_work_products: Vec>, + previous_work_products: Vec>, symbol_map: Rc>) -> CrateContextList<'a, 'tcx> { CrateContextList { shared: shared_ccx, - local_ccxs: codegen_units.into_iter().zip(previous_work_products).map(|(cgu, path)| { - LocalCrateContext::new(shared_ccx, cgu, path, symbol_map.clone()) + local_ccxs: codegen_units.into_iter().zip(previous_work_products).map(|(cgu, wp)| { + LocalCrateContext::new(shared_ccx, cgu, wp, symbol_map.clone()) }).collect() } } @@ -541,7 +540,7 @@ impl<'b, 'tcx> SharedCrateContext<'b, 'tcx> { impl<'tcx> LocalCrateContext<'tcx> { fn new<'a>(shared: &SharedCrateContext<'a, 'tcx>, codegen_unit: CodegenUnit<'tcx>, - previous_work_product: Option, + previous_work_product: Option, symbol_map: Rc>) -> LocalCrateContext<'tcx> { unsafe { @@ -727,7 +726,7 @@ impl<'b, 'tcx> CrateContext<'b, 'tcx> { self.local().llcx } - pub fn previous_work_product(&self) -> Option<&PathBuf> { + pub fn previous_work_product(&self) -> Option<&WorkProduct> { self.local().previous_work_product.as_ref() } diff --git a/src/librustc_trans/lib.rs b/src/librustc_trans/lib.rs index 67475081cae..81a1dbeb7fe 100644 --- a/src/librustc_trans/lib.rs +++ b/src/librustc_trans/lib.rs @@ -37,7 +37,7 @@ #![feature(unicode)] #![feature(question_mark)] -use std::path::PathBuf; +use rustc::dep_graph::WorkProduct; extern crate arena; extern crate flate; @@ -135,6 +135,11 @@ mod value; #[derive(Clone)] pub struct ModuleTranslation { + /// The name of the module. When the crate may be saved between + /// compilations, incremental compilation requires that name be + /// unique amongst **all** crates. Therefore, it should contain + /// something unique to this crate (e.g., a module path) as well + /// as the crate name and disambiguator. pub name: String, pub symbol_name_hash: u64, pub source: ModuleSource, @@ -142,7 +147,10 @@ pub struct ModuleTranslation { #[derive(Clone)] pub enum ModuleSource { - Preexisting(PathBuf), + /// Copy the `.o` files or whatever from the incr. comp. directory. + Preexisting(WorkProduct), + + /// Rebuild from this LLVM module. Translated(ModuleLlvm), } diff --git a/src/librustc_trans/partitioning.rs b/src/librustc_trans/partitioning.rs index 32bcbf9f756..ade6e8abeb3 100644 --- a/src/librustc_trans/partitioning.rs +++ b/src/librustc_trans/partitioning.rs @@ -143,7 +143,12 @@ pub enum PartitioningStrategy { } pub struct CodegenUnit<'tcx> { + /// A name for this CGU. Incremental compilation requires that + /// name be unique amongst **all** crates. Therefore, it should + /// contain something unique to this crate (e.g., a module path) + /// as well as the crate name and disambiguator. name: InternedString, + items: FnvHashMap, llvm::Linkage>, } @@ -174,7 +179,7 @@ impl<'tcx> CodegenUnit<'tcx> { } pub fn work_product_id(&self) -> Arc { - Arc::new(WorkProductId::PartitionObjectFile(self.name().to_string())) + Arc::new(WorkProductId(self.name().to_string())) } pub fn work_product_dep_node(&self) -> DepNode { diff --git a/src/test/incremental/rlib_cross_crate/auxiliary/a.rs b/src/test/incremental/rlib_cross_crate/auxiliary/a.rs new file mode 100644 index 00000000000..ff5fd634714 --- /dev/null +++ b/src/test/incremental/rlib_cross_crate/auxiliary/a.rs @@ -0,0 +1,25 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// no-prefer-dynamic + +#![crate_type="rlib"] + +#[cfg(rpass1)] +pub type X = u32; + +#[cfg(rpass2)] +pub type X = i32; + +// this version doesn't actually change anything: +#[cfg(rpass3)] +pub type X = i32; + +pub type Y = char; diff --git a/src/test/incremental/rlib_cross_crate/b.rs b/src/test/incremental/rlib_cross_crate/b.rs new file mode 100644 index 00000000000..55398370425 --- /dev/null +++ b/src/test/incremental/rlib_cross_crate/b.rs @@ -0,0 +1,38 @@ +// Copyright 2014 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Same test as `type_alias_cross_crate`, but with +// `no-prefer-dynamic`, ensuring that we test what happens when we +// build rlibs (before we were only testing dylibs, which meant we +// didn't realize we had to preserve a `bc` file as well). + +// aux-build:a.rs +// revisions:rpass1 rpass2 rpass3 +// no-prefer-dynamic + + +#![feature(rustc_attrs)] + +extern crate a; + +#[rustc_dirty(label="TypeckItemBody", cfg="rpass2")] +#[rustc_clean(label="TypeckItemBody", cfg="rpass3")] +pub fn use_X() -> u32 { + let x: a::X = 22; + x as u32 +} + +#[rustc_clean(label="TypeckItemBody", cfg="rpass2")] +#[rustc_clean(label="TypeckItemBody", cfg="rpass3")] +pub fn use_Y() { + let x: a::Y = 'c'; +} + +pub fn main() { }