From d93277b9150d50fae4b086cd6efe2a006b3d88da Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 31 Jul 2020 13:01:29 +1000 Subject: [PATCH 1/2] Remove `GCX_PTR`. We store an `ImplicitCtxt` pointer in a thread-local value (TLV). This allows implicit access to a `GlobalCtxt` and some other things. We also store a `GlobalCtxt` pointer in `GCX_PTR`. This is always the same `GlobalCtxt` as the one within the `ImplicitCtxt` pointer in TLV. `GCX_PTR` is only used in the parallel compiler's `handle_deadlock()` function. This commit does the following. - It removes `GCX_PTR`. - It also adds `ImplicitCtxt::new()`, which constructs an `ImplicitCtxt` from a `GlobalCtxt`. `ImplicitCtxt::new()` + `tls::enter_context()` is now equivalent to the old `tls::enter_global()`. - Makes `tls::get_tlv()` public for the parallel compiler, because it's now used in `handle_deadlock()`. --- Cargo.lock | 1 - src/librustc_interface/passes.rs | 8 ++-- src/librustc_interface/util.rs | 24 +++++------- src/librustc_middle/Cargo.toml | 1 - src/librustc_middle/lib.rs | 2 - src/librustc_middle/ty/context.rs | 59 +++++------------------------ src/librustc_middle/ty/query/job.rs | 13 ++++--- 7 files changed, 33 insertions(+), 75 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 142ac9eceeb..d4f4ec7f6f0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3677,7 +3677,6 @@ dependencies = [ "rustc_session", "rustc_span", "rustc_target", - "scoped-tls", "smallvec 1.4.0", "tracing", ] diff --git a/src/librustc_interface/passes.rs b/src/librustc_interface/passes.rs index 125a020de37..3a562847d3e 100644 --- a/src/librustc_interface/passes.rs +++ b/src/librustc_interface/passes.rs @@ -752,7 +752,8 @@ impl<'tcx> QueryContext<'tcx> { where F: FnOnce(TyCtxt<'tcx>) -> R, { - ty::tls::enter_global(self.0, f) + let icx = ty::tls::ImplicitCtxt::new(self.0); + ty::tls::enter_context(&icx, |_| f(icx.tcx)) } pub fn print_stats(&mut self) { @@ -811,8 +812,9 @@ pub fn create_global_ctxt<'tcx>( }); // Do some initialization of the DepGraph that can only be done with the tcx available. - ty::tls::enter_global(&gcx, |tcx| { - tcx.sess.time("dep_graph_tcx_init", || rustc_incremental::dep_graph_tcx_init(tcx)); + let icx = ty::tls::ImplicitCtxt::new(&gcx); + ty::tls::enter_context(&icx, |_| { + icx.tcx.sess.time("dep_graph_tcx_init", || rustc_incremental::dep_graph_tcx_init(icx.tcx)); }); QueryContext(gcx) diff --git a/src/librustc_interface/util.rs b/src/librustc_interface/util.rs index 5b648608b6b..bbb2f9d8b25 100644 --- a/src/librustc_interface/util.rs +++ b/src/librustc_interface/util.rs @@ -10,10 +10,9 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; #[cfg(parallel_compiler)] use rustc_data_structures::jobserver; use rustc_data_structures::stable_hasher::StableHasher; -use rustc_data_structures::sync::{Lock, Lrc}; +use rustc_data_structures::sync::Lrc; use rustc_errors::registry::Registry; use rustc_metadata::dynamic_lib::DynamicLibrary; -use rustc_middle::ty; use rustc_resolve::{self, Resolver}; use rustc_session as session; use rustc_session::config::{self, CrateType}; @@ -144,12 +143,10 @@ pub fn setup_callbacks_and_run_in_thread_pool_with_globals R + Se let main_handler = move || { rustc_ast::with_session_globals(edition, || { - ty::tls::GCX_PTR.set(&Lock::new(0), || { - if let Some(stderr) = stderr { - io::set_panic(Some(box Sink(stderr.clone()))); - } - f() - }) + if let Some(stderr) = stderr { + io::set_panic(Some(box Sink(stderr.clone()))); + } + f() }) }; @@ -163,6 +160,7 @@ pub fn setup_callbacks_and_run_in_thread_pool_with_globals R + Se stderr: &Option>>>, f: F, ) -> R { + use rustc_middle::ty; crate::callbacks::setup_callbacks(); let mut config = rayon::ThreadPoolBuilder::new() @@ -189,12 +187,10 @@ pub fn setup_callbacks_and_run_in_thread_pool_with_globals R + Se let main_handler = move |thread: rayon::ThreadBuilder| { rustc_ast::SESSION_GLOBALS.set(ast_session_globals, || { rustc_span::SESSION_GLOBALS.set(span_session_globals, || { - ty::tls::GCX_PTR.set(&Lock::new(0), || { - if let Some(stderr) = stderr { - io::set_panic(Some(box Sink(stderr.clone()))); - } - thread.run() - }) + if let Some(stderr) = stderr { + io::set_panic(Some(box Sink(stderr.clone()))); + } + thread.run() }) }) }; diff --git a/src/librustc_middle/Cargo.toml b/src/librustc_middle/Cargo.toml index 678fcfe6539..03431cb5a88 100644 --- a/src/librustc_middle/Cargo.toml +++ b/src/librustc_middle/Cargo.toml @@ -12,7 +12,6 @@ doctest = false [dependencies] rustc_arena = { path = "../librustc_arena" } bitflags = "1.2.1" -scoped-tls = "1.0" log = { package = "tracing", version = "0.1" } rustc-rayon-core = "0.3.0" polonius-engine = "0.12.0" diff --git a/src/librustc_middle/lib.rs b/src/librustc_middle/lib.rs index b7dccb8d8ce..a49287840e1 100644 --- a/src/librustc_middle/lib.rs +++ b/src/librustc_middle/lib.rs @@ -55,8 +55,6 @@ #[macro_use] extern crate bitflags; #[macro_use] -extern crate scoped_tls; -#[macro_use] extern crate rustc_macros; #[macro_use] extern crate rustc_data_structures; diff --git a/src/librustc_middle/ty/context.rs b/src/librustc_middle/ty/context.rs index 775d755444d..6cbf5db8373 100644 --- a/src/librustc_middle/ty/context.rs +++ b/src/librustc_middle/ty/context.rs @@ -1632,7 +1632,6 @@ pub mod tls { use crate::ty::query; use rustc_data_structures::sync::{self, Lock}; use rustc_data_structures::thin_vec::ThinVec; - use rustc_data_structures::OnDrop; use rustc_errors::Diagnostic; use std::mem; @@ -1649,8 +1648,7 @@ pub mod tls { /// in this module. #[derive(Clone)] pub struct ImplicitCtxt<'a, 'tcx> { - /// The current `TyCtxt`. Initially created by `enter_global` and updated - /// by `enter_local` with a new local interner. + /// The current `TyCtxt`. pub tcx: TyCtxt<'tcx>, /// The current query job, if any. This is updated by `JobOwner::start` in @@ -1669,6 +1667,13 @@ pub mod tls { pub task_deps: Option<&'a Lock>, } + impl<'a, 'tcx> ImplicitCtxt<'a, 'tcx> { + pub fn new(gcx: &'tcx GlobalCtxt<'tcx>) -> Self { + let tcx = TyCtxt { gcx }; + ImplicitCtxt { tcx, query: None, diagnostics: None, layout_depth: 0, task_deps: None } + } + } + /// Sets Rayon's thread-local variable, which is preserved for Rayon jobs /// to `value` during the call to `f`. It is restored to its previous value after. /// This is used to set the pointer to the new `ImplicitCtxt`. @@ -1682,7 +1687,7 @@ pub mod tls { /// This is used to get the pointer to the current `ImplicitCtxt`. #[cfg(parallel_compiler)] #[inline] - fn get_tlv() -> usize { + pub fn get_tlv() -> usize { rayon_core::tlv::get() } @@ -1699,7 +1704,7 @@ pub mod tls { #[inline] fn set_tlv R, R>(value: usize, f: F) -> R { let old = get_tlv(); - let _reset = OnDrop(move || TLV.with(|tlv| tlv.set(old))); + let _reset = rustc_data_structures::OnDrop(move || TLV.with(|tlv| tlv.set(old))); TLV.with(|tlv| tlv.set(value)); f() } @@ -1720,50 +1725,6 @@ pub mod tls { set_tlv(context as *const _ as usize, || f(&context)) } - /// Enters `GlobalCtxt` by setting up librustc_ast callbacks and - /// creating a initial `TyCtxt` and `ImplicitCtxt`. - /// This happens once per rustc session and `TyCtxt`s only exists - /// inside the `f` function. - pub fn enter_global<'tcx, F, R>(gcx: &'tcx GlobalCtxt<'tcx>, f: F) -> R - where - F: FnOnce(TyCtxt<'tcx>) -> R, - { - // Update `GCX_PTR` to indicate there's a `GlobalCtxt` available. - GCX_PTR.with(|lock| { - *lock.lock() = gcx as *const _ as usize; - }); - // Set `GCX_PTR` back to 0 when we exit. - let _on_drop = OnDrop(move || { - GCX_PTR.with(|lock| *lock.lock() = 0); - }); - - let tcx = TyCtxt { gcx }; - let icx = - ImplicitCtxt { tcx, query: None, diagnostics: None, layout_depth: 0, task_deps: None }; - enter_context(&icx, |_| f(tcx)) - } - - scoped_thread_local! { - /// Stores a pointer to the `GlobalCtxt` if one is available. - /// This is used to access the `GlobalCtxt` in the deadlock handler given to Rayon. - pub static GCX_PTR: Lock - } - - /// Creates a `TyCtxt` and `ImplicitCtxt` based on the `GCX_PTR` thread local. - /// This is used in the deadlock handler. - pub unsafe fn with_global(f: F) -> R - where - F: for<'tcx> FnOnce(TyCtxt<'tcx>) -> R, - { - let gcx = GCX_PTR.with(|lock| *lock.lock()); - assert!(gcx != 0); - let gcx = &*(gcx as *const GlobalCtxt<'_>); - let tcx = TyCtxt { gcx }; - let icx = - ImplicitCtxt { query: None, diagnostics: None, tcx, layout_depth: 0, task_deps: None }; - enter_context(&icx, |_| f(tcx)) - } - /// Allows access to the current `ImplicitCtxt` in a closure if one is available. #[inline] pub fn with_context_opt(f: F) -> R diff --git a/src/librustc_middle/ty/query/job.rs b/src/librustc_middle/ty/query/job.rs index 60b93b3d44d..1093b4dde0e 100644 --- a/src/librustc_middle/ty/query/job.rs +++ b/src/librustc_middle/ty/query/job.rs @@ -10,18 +10,21 @@ use std::thread; pub unsafe fn handle_deadlock() { let registry = rayon_core::Registry::current(); - let gcx_ptr = tls::GCX_PTR.with(|gcx_ptr| gcx_ptr as *const _); - let gcx_ptr = &*gcx_ptr; + let context = tls::get_tlv(); + assert!(context != 0); + rustc_data_structures::sync::assert_sync::>(); + let icx: &tls::ImplicitCtxt<'_, '_> = &*(context as *const tls::ImplicitCtxt<'_, '_>); let span_session_globals = rustc_span::SESSION_GLOBALS.with(|ssg| ssg as *const _); let span_session_globals = &*span_session_globals; let ast_session_globals = rustc_ast::attr::SESSION_GLOBALS.with(|asg| asg as *const _); let ast_session_globals = &*ast_session_globals; thread::spawn(move || { - tls::GCX_PTR.set(gcx_ptr, || { + tls::enter_context(icx, |_| { rustc_ast::attr::SESSION_GLOBALS.set(ast_session_globals, || { - rustc_span::SESSION_GLOBALS - .set(span_session_globals, || tls::with_global(|tcx| deadlock(tcx, ®istry))) + rustc_span::SESSION_GLOBALS.set(span_session_globals, || { + tls::with_context(|icx| deadlock(icx.tcx, ®istry)) + }) }); }) }); From 8c78fd234be2b3adfeda8748379370e5b8433ce3 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Fri, 31 Jul 2020 16:38:32 +1000 Subject: [PATCH 2/2] Use more appropriate `tls::with_*` methods in some places. --- src/librustc_infer/infer/canonical/canonicalizer.rs | 4 ++-- src/librustc_middle/mir/interpret/error.rs | 6 +++--- src/librustc_middle/ty/query/job.rs | 5 ++--- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/librustc_infer/infer/canonical/canonicalizer.rs b/src/librustc_infer/infer/canonical/canonicalizer.rs index 2dc803b9595..ea32a1ae5a5 100644 --- a/src/librustc_infer/infer/canonical/canonicalizer.rs +++ b/src/librustc_infer/infer/canonical/canonicalizer.rs @@ -199,8 +199,8 @@ impl CanonicalizeRegionMode for CanonicalizeQueryResponse { // rust-lang/rust#57464: `impl Trait` can leak local // scopes (in manner violating typeck). Therefore, use // `delay_span_bug` to allow type error over an ICE. - ty::tls::with_context(|c| { - c.tcx.sess.delay_span_bug( + ty::tls::with(|tcx| { + tcx.sess.delay_span_bug( rustc_span::DUMMY_SP, &format!("unexpected region in query response: `{:?}`", r), ); diff --git a/src/librustc_middle/mir/interpret/error.rs b/src/librustc_middle/mir/interpret/error.rs index 1d76bb525eb..a33fb2e54dd 100644 --- a/src/librustc_middle/mir/interpret/error.rs +++ b/src/librustc_middle/mir/interpret/error.rs @@ -242,9 +242,9 @@ impl From for InterpErrorInfo<'_> { impl<'tcx> From> for InterpErrorInfo<'tcx> { fn from(kind: InterpError<'tcx>) -> Self { - let capture_backtrace = tls::with_context_opt(|ctxt| { - if let Some(ctxt) = ctxt { - *Lock::borrow(&ctxt.tcx.sess.ctfe_backtrace) + let capture_backtrace = tls::with_opt(|tcx| { + if let Some(tcx) = tcx { + *Lock::borrow(&tcx.sess.ctfe_backtrace) } else { CtfeBacktrace::Disabled } diff --git a/src/librustc_middle/ty/query/job.rs b/src/librustc_middle/ty/query/job.rs index 1093b4dde0e..1811d945a1d 100644 --- a/src/librustc_middle/ty/query/job.rs +++ b/src/librustc_middle/ty/query/job.rs @@ -22,9 +22,8 @@ pub unsafe fn handle_deadlock() { thread::spawn(move || { tls::enter_context(icx, |_| { rustc_ast::attr::SESSION_GLOBALS.set(ast_session_globals, || { - rustc_span::SESSION_GLOBALS.set(span_session_globals, || { - tls::with_context(|icx| deadlock(icx.tcx, ®istry)) - }) + rustc_span::SESSION_GLOBALS + .set(span_session_globals, || tls::with(|tcx| deadlock(tcx, ®istry))) }); }) });