From 3560122bfc2c2d1901a7bfe71882e40b6228f68c Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 17 Nov 2023 08:05:56 +1100 Subject: [PATCH 1/6] Streamline `Queries::linker`. --- compiler/rustc_interface/src/queries.rs | 32 ++++++++++--------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/compiler/rustc_interface/src/queries.rs b/compiler/rustc_interface/src/queries.rs index ace5ec732fbc5..c30ff6c483199 100644 --- a/compiler/rustc_interface/src/queries.rs +++ b/compiler/rustc_interface/src/queries.rs @@ -236,25 +236,19 @@ impl<'tcx> Queries<'tcx> { } pub fn linker(&'tcx self, ongoing_codegen: Box) -> Result { - let sess = self.session().clone(); - let codegen_backend = self.codegen_backend().clone(); - - let (crate_hash, prepare_outputs, dep_graph) = self.global_ctxt()?.enter(|tcx| { - ( - if tcx.needs_crate_hash() { Some(tcx.crate_hash(LOCAL_CRATE)) } else { None }, - tcx.output_filenames(()).clone(), - tcx.dep_graph.clone(), - ) - }); - - Ok(Linker { - sess, - codegen_backend, - - dep_graph, - prepare_outputs, - crate_hash, - ongoing_codegen, + self.global_ctxt()?.enter(|tcx| { + Ok(Linker { + sess: self.session().clone(), + codegen_backend: self.codegen_backend().clone(), + dep_graph: tcx.dep_graph.clone(), + prepare_outputs: tcx.output_filenames(()).clone(), + crate_hash: if tcx.needs_crate_hash() { + Some(tcx.crate_hash(LOCAL_CRATE)) + } else { + None + }, + ongoing_codegen, + }) }) } } From de91b6d24962a4662786bbf0fab71854b3976af1 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 17 Nov 2023 08:15:36 +1100 Subject: [PATCH 2/6] Move `Session` out of `Linker`. It can easily be passed in. And that removes the single clone of `Compiler::session`, which means it no longer needs to be `Lrc`. --- compiler/rustc_driver_impl/src/lib.rs | 2 +- compiler/rustc_interface/src/interface.rs | 6 ++--- compiler/rustc_interface/src/queries.rs | 26 +++++++++------------- tests/run-make-fulldeps/issue-19371/foo.rs | 2 +- 4 files changed, 15 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 4ad7b9f6cd186..7cded5507cb1f 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -482,7 +482,7 @@ fn run_compiler( if let Some(linker) = linker { let _timer = sess.timer("link"); - linker.link()? + linker.link(sess)? } if sess.opts.unstable_opts.print_fuel.is_some() { diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index d113e03896668..e054d0a8f0b9e 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -38,13 +38,13 @@ pub type Result = result::Result; /// Can be used to run `rustc_interface` queries. /// Created by passing [`Config`] to [`run_compiler`]. pub struct Compiler { - pub(crate) sess: Lrc, + pub(crate) sess: Session, codegen_backend: Lrc, pub(crate) override_queries: Option, } impl Compiler { - pub fn session(&self) -> &Lrc { + pub fn session(&self) -> &Session { &self.sess } pub fn codegen_backend(&self) -> &Lrc { @@ -492,7 +492,7 @@ pub fn run_compiler(config: Config, f: impl FnOnce(&Compiler) -> R + Se sess.lint_store = Some(Lrc::new(lint_store)); let compiler = Compiler { - sess: Lrc::new(sess), + sess, codegen_backend: Lrc::from(codegen_backend), override_queries: config.override_queries, }; diff --git a/compiler/rustc_interface/src/queries.rs b/compiler/rustc_interface/src/queries.rs index c30ff6c483199..8345753c2e997 100644 --- a/compiler/rustc_interface/src/queries.rs +++ b/compiler/rustc_interface/src/queries.rs @@ -101,9 +101,10 @@ impl<'tcx> Queries<'tcx> { } } - fn session(&self) -> &Lrc { + fn session(&self) -> &Session { &self.compiler.sess } + fn codegen_backend(&self) -> &Lrc { self.compiler.codegen_backend() } @@ -238,7 +239,6 @@ impl<'tcx> Queries<'tcx> { pub fn linker(&'tcx self, ongoing_codegen: Box) -> Result { self.global_ctxt()?.enter(|tcx| { Ok(Linker { - sess: self.session().clone(), codegen_backend: self.codegen_backend().clone(), dep_graph: tcx.dep_graph.clone(), prepare_outputs: tcx.output_filenames(()).clone(), @@ -255,7 +255,6 @@ impl<'tcx> Queries<'tcx> { pub struct Linker { // compilation inputs - sess: Lrc, codegen_backend: Lrc, // compilation outputs @@ -267,30 +266,25 @@ pub struct Linker { } impl Linker { - pub fn link(self) -> Result<()> { - let (codegen_results, work_products) = self.codegen_backend.join_codegen( - self.ongoing_codegen, - &self.sess, - &self.prepare_outputs, - )?; + pub fn link(self, sess: &Session) -> Result<()> { + let (codegen_results, work_products) = + self.codegen_backend.join_codegen(self.ongoing_codegen, sess, &self.prepare_outputs)?; - self.sess.compile_status()?; + sess.compile_status()?; - let sess = &self.sess; let dep_graph = self.dep_graph; sess.time("serialize_work_products", || { rustc_incremental::save_work_product_index(sess, &dep_graph, work_products) }); - let prof = self.sess.prof.clone(); + let prof = sess.prof.clone(); prof.generic_activity("drop_dep_graph").run(move || drop(dep_graph)); // Now that we won't touch anything in the incremental compilation directory // any more, we can finalize it (which involves renaming it) - rustc_incremental::finalize_session_directory(&self.sess, self.crate_hash); + rustc_incremental::finalize_session_directory(sess, self.crate_hash); - if !self - .sess + if !sess .opts .output_types .keys() @@ -307,7 +301,7 @@ impl Linker { } let _timer = sess.prof.verbose_generic_activity("link_crate"); - self.codegen_backend.link(&self.sess, codegen_results, &self.prepare_outputs) + self.codegen_backend.link(sess, codegen_results, &self.prepare_outputs) } } diff --git a/tests/run-make-fulldeps/issue-19371/foo.rs b/tests/run-make-fulldeps/issue-19371/foo.rs index 1a94649163b2b..8fdf4c88f1f3a 100644 --- a/tests/run-make-fulldeps/issue-19371/foo.rs +++ b/tests/run-make-fulldeps/issue-19371/foo.rs @@ -72,6 +72,6 @@ fn compile(code: String, output: PathBuf, sysroot: PathBuf) { let ongoing_codegen = queries.ongoing_codegen()?; queries.linker(ongoing_codegen) }); - linker.unwrap().link().unwrap(); + linker.unwrap().link(compiler.session()).unwrap(); }); } From aed8e1f3b635c379d03007b86cff4dd36cb49eb4 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 17 Nov 2023 08:34:55 +1100 Subject: [PATCH 3/6] Move `CodegenBackend` out of `Linker`. It can easily be passed in. And that removes the single clone of `Compiler::codegen_backend`, which means it no longer needs to be `Lrc`. --- compiler/rustc_driver_impl/src/lib.rs | 11 +++++------ compiler/rustc_interface/src/interface.rs | 13 +++++-------- compiler/rustc_interface/src/queries.rs | 17 ++++++----------- tests/run-make-fulldeps/issue-19371/foo.rs | 2 +- 4 files changed, 17 insertions(+), 26 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 7cded5507cb1f..f38a8945c7ff1 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -361,7 +361,7 @@ fn run_compiler( } let should_stop = print_crate_info( &handler, - &**compiler.codegen_backend(), + compiler.codegen_backend(), compiler.session(), false, ); @@ -385,12 +385,11 @@ fn run_compiler( interface::run_compiler(config, |compiler| { let sess = compiler.session(); + let codegen_backend = compiler.codegen_backend(); let handler = EarlyErrorHandler::new(sess.opts.error_format); - let should_stop = print_crate_info(&handler, &**compiler.codegen_backend(), sess, true) - .and_then(|| { - list_metadata(&handler, sess, &*compiler.codegen_backend().metadata_loader()) - }) + let should_stop = print_crate_info(&handler, codegen_backend, sess, true) + .and_then(|| list_metadata(&handler, sess, &*codegen_backend.metadata_loader())) .and_then(|| try_process_rlink(sess, compiler)); if should_stop == Compilation::Stop { @@ -482,7 +481,7 @@ fn run_compiler( if let Some(linker) = linker { let _timer = sess.timer("link"); - linker.link(sess)? + linker.link(sess, codegen_backend)? } if sess.opts.unstable_opts.print_fuel.is_some() { diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index e054d0a8f0b9e..6db26b976d963 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -39,7 +39,7 @@ pub type Result = result::Result; /// Created by passing [`Config`] to [`run_compiler`]. pub struct Compiler { pub(crate) sess: Session, - codegen_backend: Lrc, + codegen_backend: Box, pub(crate) override_queries: Option, } @@ -47,8 +47,8 @@ impl Compiler { pub fn session(&self) -> &Session { &self.sess } - pub fn codegen_backend(&self) -> &Lrc { - &self.codegen_backend + pub fn codegen_backend(&self) -> &dyn CodegenBackend { + &*self.codegen_backend } pub fn build_output_filenames( &self, @@ -491,11 +491,8 @@ pub fn run_compiler(config: Config, f: impl FnOnce(&Compiler) -> R + Se } sess.lint_store = Some(Lrc::new(lint_store)); - let compiler = Compiler { - sess, - codegen_backend: Lrc::from(codegen_backend), - override_queries: config.override_queries, - }; + let compiler = + Compiler { sess, codegen_backend, override_queries: config.override_queries }; rustc_span::set_source_map(compiler.sess.parse_sess.clone_source_map(), move || { let r = { diff --git a/compiler/rustc_interface/src/queries.rs b/compiler/rustc_interface/src/queries.rs index 8345753c2e997..a3161be0c3daa 100644 --- a/compiler/rustc_interface/src/queries.rs +++ b/compiler/rustc_interface/src/queries.rs @@ -7,7 +7,7 @@ use rustc_codegen_ssa::traits::CodegenBackend; use rustc_codegen_ssa::CodegenResults; use rustc_data_structures::steal::Steal; use rustc_data_structures::svh::Svh; -use rustc_data_structures::sync::{AppendOnlyIndexVec, FreezeLock, Lrc, OnceLock, WorkerLocal}; +use rustc_data_structures::sync::{AppendOnlyIndexVec, FreezeLock, OnceLock, WorkerLocal}; use rustc_hir::def_id::{StableCrateId, CRATE_DEF_ID, LOCAL_CRATE}; use rustc_hir::definitions::Definitions; use rustc_incremental::setup_dep_graph; @@ -105,7 +105,7 @@ impl<'tcx> Queries<'tcx> { &self.compiler.sess } - fn codegen_backend(&self) -> &Lrc { + fn codegen_backend(&self) -> &dyn CodegenBackend { self.compiler.codegen_backend() } @@ -198,7 +198,7 @@ impl<'tcx> Queries<'tcx> { // Hook for UI tests. Self::check_for_rustc_errors_attr(tcx); - Ok(passes::start_codegen(&**self.codegen_backend(), tcx)) + Ok(passes::start_codegen(self.codegen_backend(), tcx)) }) } @@ -239,7 +239,6 @@ impl<'tcx> Queries<'tcx> { pub fn linker(&'tcx self, ongoing_codegen: Box) -> Result { self.global_ctxt()?.enter(|tcx| { Ok(Linker { - codegen_backend: self.codegen_backend().clone(), dep_graph: tcx.dep_graph.clone(), prepare_outputs: tcx.output_filenames(()).clone(), crate_hash: if tcx.needs_crate_hash() { @@ -254,10 +253,6 @@ impl<'tcx> Queries<'tcx> { } pub struct Linker { - // compilation inputs - codegen_backend: Lrc, - - // compilation outputs dep_graph: DepGraph, prepare_outputs: Arc, // Only present when incr. comp. is enabled. @@ -266,9 +261,9 @@ pub struct Linker { } impl Linker { - pub fn link(self, sess: &Session) -> Result<()> { + pub fn link(self, sess: &Session, codegen_backend: &dyn CodegenBackend) -> Result<()> { let (codegen_results, work_products) = - self.codegen_backend.join_codegen(self.ongoing_codegen, sess, &self.prepare_outputs)?; + codegen_backend.join_codegen(self.ongoing_codegen, sess, &self.prepare_outputs)?; sess.compile_status()?; @@ -301,7 +296,7 @@ impl Linker { } let _timer = sess.prof.verbose_generic_activity("link_crate"); - self.codegen_backend.link(sess, codegen_results, &self.prepare_outputs) + codegen_backend.link(sess, codegen_results, &self.prepare_outputs) } } diff --git a/tests/run-make-fulldeps/issue-19371/foo.rs b/tests/run-make-fulldeps/issue-19371/foo.rs index 8fdf4c88f1f3a..9be0fdccebecf 100644 --- a/tests/run-make-fulldeps/issue-19371/foo.rs +++ b/tests/run-make-fulldeps/issue-19371/foo.rs @@ -72,6 +72,6 @@ fn compile(code: String, output: PathBuf, sysroot: PathBuf) { let ongoing_codegen = queries.ongoing_codegen()?; queries.linker(ongoing_codegen) }); - linker.unwrap().link(compiler.session()).unwrap(); + linker.unwrap().link(compiler.session(), compiler.codegen_backend()).unwrap(); }); } From 94c9075b278660c72031b7b34f19f0f9e6b7f2e5 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 17 Nov 2023 09:04:45 +1100 Subject: [PATCH 4/6] Rename `Linker::prepare_outputs` as `output_filenames`. It matches the type, and a noun makes more sense than a verb. The `output_filenames` function still uses a profiling label named `prepare_outputs`, but I think that makes sense as a verb and can be left unchanged. --- compiler/rustc_interface/src/queries.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_interface/src/queries.rs b/compiler/rustc_interface/src/queries.rs index a3161be0c3daa..7d31fd8cb5c92 100644 --- a/compiler/rustc_interface/src/queries.rs +++ b/compiler/rustc_interface/src/queries.rs @@ -240,7 +240,7 @@ impl<'tcx> Queries<'tcx> { self.global_ctxt()?.enter(|tcx| { Ok(Linker { dep_graph: tcx.dep_graph.clone(), - prepare_outputs: tcx.output_filenames(()).clone(), + output_filenames: tcx.output_filenames(()).clone(), crate_hash: if tcx.needs_crate_hash() { Some(tcx.crate_hash(LOCAL_CRATE)) } else { @@ -254,7 +254,7 @@ impl<'tcx> Queries<'tcx> { pub struct Linker { dep_graph: DepGraph, - prepare_outputs: Arc, + output_filenames: Arc, // Only present when incr. comp. is enabled. crate_hash: Option, ongoing_codegen: Box, @@ -263,7 +263,7 @@ pub struct Linker { impl Linker { pub fn link(self, sess: &Session, codegen_backend: &dyn CodegenBackend) -> Result<()> { let (codegen_results, work_products) = - codegen_backend.join_codegen(self.ongoing_codegen, sess, &self.prepare_outputs)?; + codegen_backend.join_codegen(self.ongoing_codegen, sess, &self.output_filenames)?; sess.compile_status()?; @@ -289,14 +289,14 @@ impl Linker { } if sess.opts.unstable_opts.no_link { - let rlink_file = self.prepare_outputs.with_extension(config::RLINK_EXT); + let rlink_file = self.output_filenames.with_extension(config::RLINK_EXT); CodegenResults::serialize_rlink(sess, &rlink_file, &codegen_results) .map_err(|error| sess.emit_fatal(FailedWritingFile { path: &rlink_file, error }))?; return Ok(()); } let _timer = sess.prof.verbose_generic_activity("link_crate"); - codegen_backend.link(sess, codegen_results, &self.prepare_outputs) + codegen_backend.link(sess, codegen_results, &self.output_filenames) } } From 4a57b80f3fbedf86992d15016ea8f14b662b56f3 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 17 Nov 2023 09:19:11 +1100 Subject: [PATCH 5/6] Remove a low-value local variable. --- compiler/rustc_interface/src/queries.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_interface/src/queries.rs b/compiler/rustc_interface/src/queries.rs index 7d31fd8cb5c92..9762e924cc459 100644 --- a/compiler/rustc_interface/src/queries.rs +++ b/compiler/rustc_interface/src/queries.rs @@ -267,13 +267,12 @@ impl Linker { sess.compile_status()?; - let dep_graph = self.dep_graph; sess.time("serialize_work_products", || { - rustc_incremental::save_work_product_index(sess, &dep_graph, work_products) + rustc_incremental::save_work_product_index(sess, &self.dep_graph, work_products) }); let prof = sess.prof.clone(); - prof.generic_activity("drop_dep_graph").run(move || drop(dep_graph)); + prof.generic_activity("drop_dep_graph").run(move || drop(self.dep_graph)); // Now that we won't touch anything in the incremental compilation directory // any more, we can finalize it (which involves renaming it) From 9582172964608c7eed72a79d05a7e0ed58b23bca Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 17 Nov 2023 09:44:28 +1100 Subject: [PATCH 6/6] Make `Compiler::sess` private. Like `Compiler::codegen_backend`. --- compiler/rustc_interface/src/interface.rs | 2 +- compiler/rustc_interface/src/queries.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index 6db26b976d963..6225424fe4ac8 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -38,7 +38,7 @@ pub type Result = result::Result; /// Can be used to run `rustc_interface` queries. /// Created by passing [`Config`] to [`run_compiler`]. pub struct Compiler { - pub(crate) sess: Session, + sess: Session, codegen_backend: Box, pub(crate) override_queries: Option, } diff --git a/compiler/rustc_interface/src/queries.rs b/compiler/rustc_interface/src/queries.rs index 9762e924cc459..fbf2dbcdae50a 100644 --- a/compiler/rustc_interface/src/queries.rs +++ b/compiler/rustc_interface/src/queries.rs @@ -102,7 +102,7 @@ impl<'tcx> Queries<'tcx> { } fn session(&self) -> &Session { - &self.compiler.sess + &self.compiler.session() } fn codegen_backend(&self) -> &dyn CodegenBackend {