Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move DepKind and query TLS to rustc_query_system #86038

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3883,6 +3883,7 @@ dependencies = [
"rustc_plugin_impl",
"rustc_privacy",
"rustc_query_impl",
"rustc_query_system",
"rustc_resolve",
"rustc_serialize",
"rustc_session",
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_incremental/src/assert_dep_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ use rustc_hir as hir;
use rustc_hir::def_id::DefId;
use rustc_hir::intravisit::{self, Visitor};
use rustc_middle::dep_graph::{
DepGraphQuery, DepKind, DepNode, DepNodeExt, DepNodeFilter, EdgeFilter,
dep_kind, DepGraphQuery, DepNode, DepNodeExt, DepNodeFilter, EdgeFilter,
};
use rustc_middle::hir::nested_filter;
use rustc_middle::ty::TyCtxt;
Expand Down Expand Up @@ -128,7 +128,7 @@ impl<'tcx> IfThisChanged<'tcx> {
let dep_node_interned = self.argument(attr);
let dep_node = match dep_node_interned {
None => {
DepNode::from_def_path_hash(self.tcx, def_path_hash, DepKind::hir_owner)
DepNode::from_def_path_hash(self.tcx, def_path_hash, dep_kind::hir_owner)
}
Some(n) => {
match DepNode::from_label_string(self.tcx, n.as_str(), def_path_hash) {
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_interface/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ rustc_lint = { path = "../rustc_lint" }
rustc_errors = { path = "../rustc_errors" }
rustc_plugin_impl = { path = "../rustc_plugin_impl" }
rustc_privacy = { path = "../rustc_privacy" }
rustc_query_system = { path = "../rustc_query_system" }
rustc_query_impl = { path = "../rustc_query_impl" }
rustc_resolve = { path = "../rustc_resolve" }
rustc_trait_selection = { path = "../rustc_trait_selection" }
Expand Down
51 changes: 35 additions & 16 deletions compiler/rustc_interface/src/callbacks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,9 @@
//! The functions in this file should fall back to the default set in their
//! origin crate when the `TyCtxt` is not present in TLS.

use rustc_errors::{Diagnostic, TRACK_DIAGNOSTICS};
use rustc_middle::dep_graph::DepNodeExt;
use rustc_middle::ty::tls;
use rustc_query_system::dep_graph::{DepKind, DepNode};
use std::fmt;

fn track_span_parent(def_id: rustc_span::def_id::LocalDefId) {
Expand All @@ -23,20 +24,6 @@ fn track_span_parent(def_id: rustc_span::def_id::LocalDefId) {
})
}

/// This is a callback from `rustc_ast` as it cannot access the implicit state
/// in `rustc_middle` otherwise. It is used to when diagnostic messages are
/// emitted and stores them in the current query, if there is one.
fn track_diagnostic(diagnostic: &Diagnostic) {
tls::with_context_opt(|icx| {
if let Some(icx) = icx {
if let Some(diagnostics) = icx.diagnostics {
let mut diagnostics = diagnostics.lock();
diagnostics.extend(Some(diagnostic.clone()));
}
}
})
}

/// This is a callback from `rustc_hir` as it cannot access the implicit state
/// in `rustc_middle` otherwise.
fn def_id_debug(def_id: rustc_hir::def_id::DefId, f: &mut fmt::Formatter<'_>) -> fmt::Result {
Expand All @@ -50,10 +37,42 @@ fn def_id_debug(def_id: rustc_hir::def_id::DefId, f: &mut fmt::Formatter<'_>) ->
write!(f, ")")
}

fn dep_kind_debug(kind: &DepKind, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
tls::with_opt(|opt_tcx| {
if let Some(tcx) = opt_tcx {
write!(f, "{}", tcx.query_name(*kind))
} else {
write!(f, "DepKind({:?})", kind.index())
}
})
}

fn dep_node_debug(node: &DepNode, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
tls::with_opt(|opt_tcx| {
if let Some(tcx) = opt_tcx {
write!(f, "{}(", tcx.query_name(node.kind))?;

if let Some(def_id) = node.extract_def_id(tcx) {
write!(f, "{}", tcx.def_path_debug_str(def_id))?;
} else if let Some(ref s) = tcx.dep_graph.dep_node_debug_str(*node) {
write!(f, "{}", s)?;
} else {
write!(f, "{}", node.hash)?;
}

write!(f, ")")
} else {
write!(f, "DepKind({:?})({})", node.kind.index(), node.hash)
}
})
}

/// Sets up the callbacks in prior crates which we want to refer to the
/// TyCtxt in.
pub fn setup_callbacks() {
rustc_span::SPAN_TRACK.swap(&(track_span_parent as fn(_)));
rustc_hir::def_id::DEF_ID_DEBUG.swap(&(def_id_debug as fn(_, &mut fmt::Formatter<'_>) -> _));
TRACK_DIAGNOSTICS.swap(&(track_diagnostic as fn(&_)));
rustc_query_system::dep_graph::KIND_DEBUG.swap(&(dep_kind_debug as _));
rustc_query_system::dep_graph::NODE_DEBUG.swap(&(dep_node_debug as _));
rustc_errors::TRACK_DIAGNOSTICS.swap(&(rustc_query_system::tls::track_diagnostic as fn(&_)));
}
6 changes: 3 additions & 3 deletions compiler/rustc_interface/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,9 +344,9 @@ pub fn try_print_query_stack(handler: &Handler, num_frames: Option<usize>) {
// Be careful relying on global state here: this code is called from
// a panic hook, which means that the global `Handler` may be in a weird
// state if it was responsible for triggering the panic.
let i = ty::tls::with_context_opt(|icx| {
if let Some(icx) = icx {
QueryCtxt::from_tcx(icx.tcx).try_print_query_stack(icx.query, handler, num_frames)
let i = ty::tls::with_opt(|tcx| {
if let Some(tcx) = tcx {
QueryCtxt::from_tcx(tcx).try_print_query_stack(handler, num_frames)
} else {
0
}
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_interface/src/passes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -806,13 +806,12 @@ pub struct QueryContext<'tcx> {
gcx: &'tcx GlobalCtxt<'tcx>,
}

impl<'tcx> QueryContext<'tcx> {
impl<'gcx> QueryContext<'gcx> {
pub fn enter<F, R>(&mut self, f: F) -> R
where
F: FnOnce(TyCtxt<'tcx>) -> R,
F: for<'tcx> FnOnce(TyCtxt<'tcx>) -> R,
{
let icx = ty::tls::ImplicitCtxt::new(self.gcx);
ty::tls::enter_context(&icx, |_| f(icx.tcx))
ty::tls::enter_context(self.gcx, f)
}
}

Expand Down
22 changes: 12 additions & 10 deletions compiler/rustc_interface/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use rustc_data_structures::jobserver;
use rustc_data_structures::sync::Lrc;
use rustc_errors::registry::Registry;
#[cfg(parallel_compiler)]
use rustc_middle::ty::tls;
use rustc_middle::ty::{tls, GlobalCtxt};
use rustc_parse::validate_attr;
#[cfg(parallel_compiler)]
use rustc_query_impl::QueryCtxt;
Expand Down Expand Up @@ -166,19 +166,21 @@ pub fn run_in_thread_pool_with_globals<F: FnOnce() -> R + Send, R: Send>(
unsafe fn handle_deadlock() {
let registry = rustc_rayon_core::Registry::current();

let context = tls::get_tlv();
assert!(context != 0);
rustc_data_structures::sync::assert_sync::<tls::ImplicitCtxt<'_, '_>>();
let icx: &tls::ImplicitCtxt<'_, '_> = &*(context as *const tls::ImplicitCtxt<'_, '_>);
// We do not need to copy the query ImplicitCtxt since this deadlock handler is not part of a
// normal query invocation.

let session_globals = rustc_span::with_session_globals(|sg| sg as *const _);
let session_globals = &*session_globals;

// Extend the lifetime of the GlobalCtxt so the new thread can know of it.
// The current thread will not free it, it is deadlocked.
let tcx: &'static GlobalCtxt<'static> =
tls::with(|tcx| &*(*tcx as *const GlobalCtxt<'_>).cast());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is incorrect since it's only set on one of Rayon's threads. The deadlock handler can be called on any thread. The current state of things is also broken to be fair.

We can not use the Rayon "Task Local Value" either, since the deadlock handler can also be called from a thread with no rustc task is running. My solution to this was to point all threads to a lock which holds the GlobalCtxt when available. It was reverted in #74969.


thread::spawn(move || {
tls::enter_context(icx, |_| {
rustc_span::set_session_globals_then(session_globals, || {
tls::with(|tcx| QueryCtxt::from_tcx(tcx).deadlock(&registry))
})
});
rustc_span::set_session_globals_then(session_globals, || {
tls::enter_context(tcx, |tcx| QueryCtxt::from_tcx(tcx).deadlock(&registry))
})
});
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,8 @@ macro_rules! provide_one {
// External query providers call `crate_hash` in order to register a dependency
// on the crate metadata. The exception is `crate_hash` itself, which obviously
// doesn't need to do this (and can't, as it would cause a query cycle).
use rustc_middle::dep_graph::DepKind;
if DepKind::$name != DepKind::crate_hash && $tcx.dep_graph.is_fully_enabled() {
use rustc_middle::dep_graph::dep_kind;
if dep_kind::$name != dep_kind::crate_hash && $tcx.dep_graph.is_fully_enabled() {
$tcx.ensure().crate_hash($def_id.krate);
}

Expand Down
73 changes: 36 additions & 37 deletions compiler/rustc_middle/src/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,8 @@ use rustc_data_structures::fingerprint::Fingerprint;
use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, CRATE_DEF_INDEX};
use rustc_hir::definitions::DefPathHash;
use rustc_hir::HirId;
use rustc_query_system::dep_graph::FingerprintStyle;
use rustc_query_system::dep_graph::{DepKind, DepNode, DepNodeParams, FingerprintStyle};
use rustc_span::symbol::Symbol;
use std::hash::Hash;

pub use rustc_query_system::dep_graph::{DepContext, DepNodeParams};

/// This struct stores metadata about each DepKind.
///
Expand All @@ -89,6 +86,9 @@ pub struct DepKindStruct {
/// See [DepNodeParams] trait for the behaviour of each key type.
pub fingerprint_style: FingerprintStyle,

/// Name of the query for pretty-printing.
pub label: &'static str,

/// The red/green evaluation system will try to mark a specific DepNode in the
/// dependency graph as green by recursively trying to mark the dependencies of
/// that `DepNode` as green. While doing so, it will sometimes encounter a `DepNode`
Expand Down Expand Up @@ -130,16 +130,21 @@ pub struct DepKindStruct {
pub try_load_from_on_disk_cache: Option<fn(TyCtxt<'_>, DepNode)>,
}

impl DepKind {
impl TyCtxt<'_> {
#[inline(always)]
pub fn fingerprint_style(self, tcx: TyCtxt<'_>) -> FingerprintStyle {
crate fn query_fingerprint_style(self, dep_kind: DepKind) -> FingerprintStyle {
// Only fetch the DepKindStruct once.
let data = tcx.query_kind(self);
let data = self.query_kind(dep_kind);
if data.is_anon {
return FingerprintStyle::Opaque;
}
data.fingerprint_style
}

#[inline(always)]
pub fn query_name(self, dep_kind: DepKind) -> &'static str {
self.query_kind(dep_kind).label
}
}

macro_rules! define_dep_nodes {
Expand All @@ -151,38 +156,40 @@ macro_rules! define_dep_nodes {
) => (
#[macro_export]
macro_rules! make_dep_kind_array {
($mod:ident) => {[ $($mod::$variant()),* ]};
($mod:ident) => {[ $mod::NULL(), $($mod::$variant()),* ]};
}

/// This enum serves as an index into arrays built by `make_dep_kind_array`.
#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Encodable, Decodable)]
#[allow(non_camel_case_types)]
pub enum DepKind {
$($variant),*
/// Definition of the `DepKind`s for the known queries.
/// The constants are indices in the arrays produced by `make_dep_kind_array`,
/// and should be kept in sync.
#[allow(non_upper_case_globals)]
pub mod dep_kind {
use super::DepKind;

/// We use this for most things when incr. comp. is turned off.
pub const NULL: DepKind = DepKind::NULL;
$(pub const $variant: DepKind = DepKind::new(1 + ${index()});)*
}

fn dep_kind_from_label_string(label: &str) -> Result<DepKind, ()> {
match label {
$(stringify!($variant) => Ok(DepKind::$variant),)*
"NULL" => Ok(dep_kind::NULL),
$(stringify!($variant) => Ok(dep_kind::$variant),)*
_ => Err(()),
}
}

/// Contains variant => str representations for constructing
/// DepNode groups for tests.
#[allow(dead_code, non_upper_case_globals)]
#[allow(non_upper_case_globals)]
pub mod label_strs {
$(
pub const $variant: &str = stringify!($variant);
)*
pub const NULL: &str = "NULL";
$(pub const $variant: &str = stringify!($variant);)*
}
);
}

rustc_dep_node_append!([define_dep_nodes!][ <'tcx>
// We use this for most things when incr. comp. is turned off.
[] Null,

[anon] TraitSelect,

// WARNING: if `Symbol` is changed, make sure you update `make_compile_codegen_unit` below.
Expand All @@ -196,26 +203,15 @@ rustc_dep_node_append!([define_dep_nodes!][ <'tcx>
// WARNING: `construct` is generic and does not know that `CompileCodegenUnit` takes `Symbol`s as keys.
// Be very careful changing this type signature!
crate fn make_compile_codegen_unit(tcx: TyCtxt<'_>, name: Symbol) -> DepNode {
DepNode::construct(tcx, DepKind::CompileCodegenUnit, &name)
DepNode::construct(tcx, dep_kind::CompileCodegenUnit, &name)
}

// WARNING: `construct` is generic and does not know that `CompileMonoItem` takes `MonoItem`s as keys.
// Be very careful changing this type signature!
crate fn make_compile_mono_item<'tcx>(tcx: TyCtxt<'tcx>, mono_item: &MonoItem<'tcx>) -> DepNode {
DepNode::construct(tcx, DepKind::CompileMonoItem, mono_item)
DepNode::construct(tcx, dep_kind::CompileMonoItem, mono_item)
}

pub type DepNode = rustc_query_system::dep_graph::DepNode<DepKind>;

// We keep a lot of `DepNode`s in memory during compilation. It's not
// required that their size stay the same, but we don't want to change
// it inadvertently. This assert just ensures we're aware of any change.
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
static_assert_size!(DepNode, 18);

#[cfg(not(any(target_arch = "x86", target_arch = "x86_64")))]
static_assert_size!(DepNode, 24);

pub trait DepNodeExt: Sized {
/// Construct a DepNode from the given DepKind and DefPathHash. This
/// method will assert that the given DepKind actually requires a
Expand Down Expand Up @@ -249,8 +245,9 @@ impl DepNodeExt for DepNode {
/// Construct a DepNode from the given DepKind and DefPathHash. This
/// method will assert that the given DepKind actually requires a
/// single DefId/DefPathHash parameter.
#[inline]
fn from_def_path_hash(tcx: TyCtxt<'_>, def_path_hash: DefPathHash, kind: DepKind) -> DepNode {
debug_assert!(kind.fingerprint_style(tcx) == FingerprintStyle::DefPathHash);
debug_assert!(tcx.query_fingerprint_style(kind) == FingerprintStyle::DefPathHash);
DepNode { kind, hash: def_path_hash.0.into() }
}

Expand All @@ -264,8 +261,9 @@ impl DepNodeExt for DepNode {
/// DepNode. Condition (2) might not be fulfilled if a DepNode
/// refers to something from the previous compilation session that
/// has been removed.
#[inline]
fn extract_def_id<'tcx>(&self, tcx: TyCtxt<'tcx>) -> Option<DefId> {
if self.kind.fingerprint_style(tcx) == FingerprintStyle::DefPathHash {
if tcx.query_fingerprint_style(self.kind) == FingerprintStyle::DefPathHash {
Some(tcx.def_path_hash_to_def_id(DefPathHash(self.hash.into()), &mut || {
panic!("Failed to extract DefId: {:?} {}", self.kind, self.hash)
}))
Expand All @@ -282,7 +280,7 @@ impl DepNodeExt for DepNode {
) -> Result<DepNode, ()> {
let kind = dep_kind_from_label_string(label)?;

match kind.fingerprint_style(tcx) {
match tcx.query_fingerprint_style(kind) {
FingerprintStyle::Opaque => Err(()),
FingerprintStyle::Unit => Ok(DepNode::new_no_params(tcx, kind)),
FingerprintStyle::DefPathHash => {
Expand All @@ -292,6 +290,7 @@ impl DepNodeExt for DepNode {
}

/// Used in testing
#[inline]
fn has_label_string(label: &str) -> bool {
dep_kind_from_label_string(label).is_ok()
}
Expand Down
Loading