Skip to content

Avoid adjusting TLS data twice for queries #90139

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

Closed
wants to merge 9 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 @@ -4019,6 +4019,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 @@ -55,7 +55,7 @@ use std::io::{BufWriter, Write};
pub fn assert_dep_graph(tcx: TyCtxt<'_>) {
tcx.dep_graph.with_ignore(|| {
if tcx.sess.opts.debugging_opts.dump_dep_graph {
tcx.dep_graph.with_query(dump_graph);
tcx.dep_graph.with_debug(dump_graph);
}

if !tcx.sess.opts.debugging_opts.query_dep_graph {
Expand Down Expand Up @@ -207,7 +207,7 @@ fn check_paths<'tcx>(tcx: TyCtxt<'tcx>, if_this_changed: &Sources, then_this_wou
}
return;
}
tcx.dep_graph.with_query(|query| {
tcx.dep_graph.with_debug(|query| {
for &(_, source_def_id, ref source_dep_node) in if_this_changed {
let dependents = query.transitive_predecessors(source_dep_node);
for &(target_span, ref target_pass, _, ref target_dep_node) in then_this_would_need {
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 @@ -42,6 +42,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
40 changes: 24 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::DepNode;
use std::fmt;

/// This is a callback from `rustc_ast` as it cannot access the implicit state
Expand All @@ -35,20 +36,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 @@ -62,11 +49,32 @@ fn def_id_debug(def_id: rustc_hir::def_id::DefId, f: &mut fmt::Formatter<'_>) ->
write!(f, ")")
}

fn dep_node_debug(node: &DepNode, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{:?}(", node.kind)?;

tls::with_opt(|opt_tcx| {
if let Some(tcx) = opt_tcx {
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)
}
} else {
write!(f, "{}", node.hash)
}
})?;

write!(f, ")")
}

/// Sets up the callbacks in prior crates which we want to refer to the
/// TyCtxt in.
pub fn setup_callbacks() {
rustc_span::SPAN_DEBUG.swap(&(span_debug as fn(_, &mut fmt::Formatter<'_>) -> _));
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::NODE_DEBUG.swap(&(dep_node_debug as _));
rustc_errors::TRACK_DIAGNOSTICS.swap(&(rustc_query_system::tls::track_diagnostic as fn(&_)));
}
2 changes: 1 addition & 1 deletion compiler/rustc_interface/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,7 @@ pub fn try_print_query_stack(handler: &Handler, num_frames: Option<usize>) {
// 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)
QueryCtxt::from_tcx(icx.tcx).try_print_query_stack(handler, num_frames)
} else {
0
}
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_interface/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,8 +158,11 @@ unsafe fn handle_deadlock() {

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<'_, '_>);
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;
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_macros/src/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -504,6 +504,7 @@ pub fn rustc_queries(input: TokenStream) -> TokenStream {
}
}
}
#[macro_export]
macro_rules! rustc_dep_node_append {
([$($macro:tt)*][$($other:tt)*]) => {
$($macro)*(
Expand Down
103 changes: 18 additions & 85 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,9 @@ 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::dep_kind_from_label_string;
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 Down Expand Up @@ -130,69 +128,6 @@ pub struct DepKindStruct {
pub try_load_from_on_disk_cache: Option<fn(TyCtxt<'_>, DepNode)>,
}

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

macro_rules! define_dep_nodes {
(<$tcx:tt>
$(
[$($attrs:tt)*]
$variant:ident $(( $tuple_arg_ty:ty $(,)? ))*
,)*
) => (
#[macro_export]
macro_rules! make_dep_kind_array {
($mod:ident) => {[ $($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),*
}

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

/// Contains variant => str representations for constructing
/// DepNode groups for tests.
#[allow(dead_code, non_upper_case_globals)]
pub mod label_strs {
$(
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.
[] CompileCodegenUnit(Symbol),

// WARNING: if `MonoItem` is changed, make sure you update `make_compile_mono_item` below.
// Only used by rustc_codegen_cranelift
[] CompileMonoItem(MonoItem),
]);

// 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 {
Expand All @@ -205,16 +140,15 @@ crate fn make_compile_mono_item(tcx: TyCtxt<'tcx>, mono_item: &MonoItem<'tcx>) -
DepNode::construct(tcx, DepKind::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);
#[inline(always)]
pub fn fingerprint_style(tcx: TyCtxt<'_>, kind: DepKind) -> FingerprintStyle {
// Only fetch the DepKindStruct once.
let data = tcx.query_kind(kind);
if data.is_anon {
return FingerprintStyle::Opaque;
}
data.fingerprint_style
}

pub trait DepNodeExt: Sized {
/// Construct a DepNode from the given DepKind and DefPathHash. This
Expand All @@ -241,16 +175,15 @@ pub trait DepNodeExt: Sized {
def_path_hash: DefPathHash,
) -> Result<Self, ()>;

/// Used in testing
fn has_label_string(label: &str) -> bool;
fn fingerprint_style(self, tcx: TyCtxt<'_>) -> FingerprintStyle;
}

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.
fn from_def_path_hash(tcx: TyCtxt<'_>, def_path_hash: DefPathHash, kind: DepKind) -> DepNode {
debug_assert!(kind.fingerprint_style(tcx) == FingerprintStyle::DefPathHash);
debug_assert_eq!(fingerprint_style(tcx, kind), FingerprintStyle::DefPathHash);
DepNode { kind, hash: def_path_hash.0.into() }
}

Expand All @@ -265,7 +198,7 @@ impl DepNodeExt for DepNode {
/// refers to something from the previous compilation session that
/// has been removed.
fn extract_def_id(&self, tcx: TyCtxt<'tcx>) -> Option<DefId> {
if self.kind.fingerprint_style(tcx) == FingerprintStyle::DefPathHash {
if fingerprint_style(tcx, self.kind) == FingerprintStyle::DefPathHash {
Some(tcx.def_path_hash_to_def_id(DefPathHash(self.hash.into())))
} else {
None
Expand All @@ -280,7 +213,7 @@ impl DepNodeExt for DepNode {
) -> Result<DepNode, ()> {
let kind = dep_kind_from_label_string(label)?;

match kind.fingerprint_style(tcx) {
match fingerprint_style(tcx, kind) {
FingerprintStyle::Opaque => Err(()),
FingerprintStyle::Unit => Ok(DepNode::new_no_params(tcx, kind)),
FingerprintStyle::DefPathHash => {
Expand All @@ -289,9 +222,9 @@ impl DepNodeExt for DepNode {
}
}

/// Used in testing
fn has_label_string(label: &str) -> bool {
dep_kind_from_label_string(label).is_ok()
#[inline(always)]
fn fingerprint_style(self, tcx: TyCtxt<'_>) -> FingerprintStyle {
fingerprint_style(tcx, self.kind)
}
}

Expand Down
68 changes: 7 additions & 61 deletions compiler/rustc_middle/src/dep_graph/mod.rs
Original file line number Diff line number Diff line change
@@ -1,75 +1,21 @@
use crate::ty::{self, TyCtxt};
use crate::ty::TyCtxt;
use rustc_data_structures::profiling::SelfProfilerRef;
use rustc_data_structures::sync::Lock;
use rustc_query_system::ich::StableHashingContext;
use rustc_session::Session;

#[macro_use]
mod dep_node;

pub use rustc_query_system::dep_graph::{
debug::DepNodeFilter, hash_result, DepContext, DepNodeColor, DepNodeIndex,
SerializedDepNodeIndex, WorkProduct, WorkProductId,
debug::DepNodeFilter, debug::EdgeFilter, hash_result, label_strs, DepContext, DepGraph,
DepGraphQuery, DepKind, DepNode, DepNodeColor, DepNodeIndex, SerializedDepGraph,
SerializedDepNodeIndex, TaskDeps, WorkProduct, WorkProductId,
};

pub use dep_node::{label_strs, DepKind, DepKindStruct, DepNode, DepNodeExt};
crate use dep_node::{make_compile_codegen_unit, make_compile_mono_item};

pub type DepGraph = rustc_query_system::dep_graph::DepGraph<DepKind>;
pub type TaskDeps = rustc_query_system::dep_graph::TaskDeps<DepKind>;
pub type DepGraphQuery = rustc_query_system::dep_graph::DepGraphQuery<DepKind>;
pub type SerializedDepGraph = rustc_query_system::dep_graph::SerializedDepGraph<DepKind>;
pub type EdgeFilter = rustc_query_system::dep_graph::debug::EdgeFilter<DepKind>;

impl rustc_query_system::dep_graph::DepKind for DepKind {
const NULL: Self = DepKind::Null;

fn debug_node(node: &DepNode, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(f, "{:?}(", node.kind)?;

ty::tls::with_opt(|opt_tcx| {
if let Some(tcx) = opt_tcx {
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)?;
}
} else {
write!(f, "{}", node.hash)?;
}
Ok(())
})?;

write!(f, ")")
}

fn with_deps<OP, R>(task_deps: Option<&Lock<TaskDeps>>, op: OP) -> R
where
OP: FnOnce() -> R,
{
ty::tls::with_context(|icx| {
let icx = ty::tls::ImplicitCtxt { task_deps, ..icx.clone() };

ty::tls::enter_context(&icx, |_| op())
})
}

fn read_deps<OP>(op: OP)
where
OP: for<'a> FnOnce(Option<&'a Lock<TaskDeps>>),
{
ty::tls::with_context_opt(|icx| {
let icx = if let Some(icx) = icx { icx } else { return };
op(icx.task_deps)
})
}
}
crate use dep_node::{fingerprint_style, make_compile_codegen_unit, make_compile_mono_item};
pub use dep_node::{DepKindStruct, DepNodeExt};

impl<'tcx> DepContext for TyCtxt<'tcx> {
type DepKind = DepKind;

#[inline]
fn create_stable_hashing_context(&self) -> StableHashingContext<'_> {
TyCtxt::create_stable_hashing_context(*self)
Expand All @@ -92,7 +38,7 @@ impl<'tcx> DepContext for TyCtxt<'tcx> {

#[inline(always)]
fn fingerprint_style(&self, kind: DepKind) -> rustc_query_system::dep_graph::FingerprintStyle {
kind.fingerprint_style(*self)
fingerprint_style(*self, kind)
}

#[inline(always)]
Expand Down
Loading