From 79a57625f5900a67cb76732c4bb7634778f066ae Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 24 Oct 2020 17:37:26 +0200 Subject: [PATCH 01/11] Move DepNodeExt outside of the macro. --- .../rustc_middle/src/dep_graph/dep_node.rs | 186 +++++++++--------- 1 file changed, 91 insertions(+), 95 deletions(-) diff --git a/compiler/rustc_middle/src/dep_graph/dep_node.rs b/compiler/rustc_middle/src/dep_graph/dep_node.rs index d954c8ab5fb9c..2a9d6b00db113 100644 --- a/compiler/rustc_middle/src/dep_graph/dep_node.rs +++ b/compiler/rustc_middle/src/dep_graph/dep_node.rs @@ -191,101 +191,10 @@ macro_rules! define_dep_nodes { )* } - pub type DepNode = rustc_query_system::dep_graph::DepNode; - - // 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, 17); - - #[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 - /// single DefId/DefPathHash parameter. - fn from_def_path_hash(def_path_hash: DefPathHash, kind: DepKind) -> Self; - - /// Extracts the DefId corresponding to this DepNode. This will work - /// if two conditions are met: - /// - /// 1. The Fingerprint of the DepNode actually is a DefPathHash, and - /// 2. the item that the DefPath refers to exists in the current tcx. - /// - /// Condition (1) is determined by the DepKind variant of the - /// DepNode. Condition (2) might not be fulfilled if a DepNode - /// refers to something from the previous compilation session that - /// has been removed. - fn extract_def_id(&self, tcx: TyCtxt<'_>) -> Option; - - /// Used in testing - fn from_label_string(label: &str, def_path_hash: DefPathHash) - -> Result; - - /// Used in testing - fn has_label_string(label: &str) -> bool; - } - - 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(def_path_hash: DefPathHash, kind: DepKind) -> DepNode { - debug_assert!(kind.can_reconstruct_query_key() && kind.has_params()); - DepNode { - kind, - hash: def_path_hash.0.into(), - } - } - - /// Extracts the DefId corresponding to this DepNode. This will work - /// if two conditions are met: - /// - /// 1. The Fingerprint of the DepNode actually is a DefPathHash, and - /// 2. the item that the DefPath refers to exists in the current tcx. - /// - /// Condition (1) is determined by the DepKind variant of the - /// DepNode. Condition (2) might not be fulfilled if a DepNode - /// refers to something from the previous compilation session that - /// has been removed. - fn extract_def_id(&self, tcx: TyCtxt<'tcx>) -> Option { - if self.kind.can_reconstruct_query_key() { - tcx.queries.on_disk_cache.as_ref()?.def_path_hash_to_def_id(tcx, DefPathHash(self.hash.into())) - } else { - None - } - } - - /// Used in testing - fn from_label_string(label: &str, def_path_hash: DefPathHash) -> Result { - let kind = match label { - $( - stringify!($variant) => DepKind::$variant, - )* - _ => return Err(()), - }; - - if !kind.can_reconstruct_query_key() { - return Err(()); - } - - if kind.has_params() { - Ok(DepNode::from_def_path_hash(def_path_hash, kind)) - } else { - Ok(DepNode::new_no_params(kind)) - } - } - - /// Used in testing - fn has_label_string(label: &str) -> bool { - match label { - $( - stringify!($variant) => true, - )* - _ => false, - } + fn dep_kind_from_label_string(label: &str) -> Result { + match label { + $(stringify!($variant) => Ok(DepKind::$variant),)* + _ => Err(()), } } @@ -312,6 +221,93 @@ rustc_dep_node_append!([define_dep_nodes!][ <'tcx> [] CompileCodegenUnit(Symbol), ]); +pub type DepNode = rustc_query_system::dep_graph::DepNode; + +// 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, 17); + +#[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 + /// single DefId/DefPathHash parameter. + fn from_def_path_hash(def_path_hash: DefPathHash, kind: DepKind) -> Self; + + /// Extracts the DefId corresponding to this DepNode. This will work + /// if two conditions are met: + /// + /// 1. The Fingerprint of the DepNode actually is a DefPathHash, and + /// 2. the item that the DefPath refers to exists in the current tcx. + /// + /// Condition (1) is determined by the DepKind variant of the + /// DepNode. Condition (2) might not be fulfilled if a DepNode + /// refers to something from the previous compilation session that + /// has been removed. + fn extract_def_id(&self, tcx: TyCtxt<'_>) -> Option; + + /// Used in testing + fn from_label_string(label: &str, def_path_hash: DefPathHash) -> Result; + + /// Used in testing + fn has_label_string(label: &str) -> bool; +} + +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(def_path_hash: DefPathHash, kind: DepKind) -> DepNode { + debug_assert!(kind.can_reconstruct_query_key() && kind.has_params()); + DepNode { kind, hash: def_path_hash.0.into() } + } + + /// Extracts the DefId corresponding to this DepNode. This will work + /// if two conditions are met: + /// + /// 1. The Fingerprint of the DepNode actually is a DefPathHash, and + /// 2. the item that the DefPath refers to exists in the current tcx. + /// + /// Condition (1) is determined by the DepKind variant of the + /// DepNode. Condition (2) might not be fulfilled if a DepNode + /// refers to something from the previous compilation session that + /// has been removed. + fn extract_def_id(&self, tcx: TyCtxt<'tcx>) -> Option { + if self.kind.can_reconstruct_query_key() { + tcx.queries + .on_disk_cache + .as_ref()? + .def_path_hash_to_def_id(tcx, DefPathHash(self.hash.into())) + } else { + None + } + } + + /// Used in testing + fn from_label_string(label: &str, def_path_hash: DefPathHash) -> Result { + let kind = dep_kind_from_label_string(label)?; + + if !kind.can_reconstruct_query_key() { + return Err(()); + } + + if kind.has_params() { + Ok(DepNode::from_def_path_hash(def_path_hash, kind)) + } else { + Ok(DepNode::new_no_params(kind)) + } + } + + /// Used in testing + fn has_label_string(label: &str) -> bool { + dep_kind_from_label_string(label).is_ok() + } +} + impl<'tcx> DepNodeParams> for DefId { #[inline] fn can_reconstruct_query_key() -> bool { From d1220fdedf030049a22a3ae430e15fda2a3f4477 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Tue, 27 Oct 2020 19:54:28 +0100 Subject: [PATCH 02/11] Simplify DepNodeParams. --- .../rustc_middle/src/dep_graph/dep_node.rs | 25 +++++++++++++++---- .../src/dep_graph/dep_node.rs | 6 ----- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_middle/src/dep_graph/dep_node.rs b/compiler/rustc_middle/src/dep_graph/dep_node.rs index 2a9d6b00db113..fa96b800928ec 100644 --- a/compiler/rustc_middle/src/dep_graph/dep_node.rs +++ b/compiler/rustc_middle/src/dep_graph/dep_node.rs @@ -308,8 +308,23 @@ impl DepNodeExt for DepNode { } } +impl<'tcx> DepNodeParams> for () { + #[inline(always)] + fn can_reconstruct_query_key() -> bool { + true + } + + fn to_fingerprint(&self, _: TyCtxt<'tcx>) -> Fingerprint { + Fingerprint::ZERO + } + + fn recover(_: TyCtxt<'tcx>, _: &DepNode) -> Option { + Some(()) + } +} + impl<'tcx> DepNodeParams> for DefId { - #[inline] + #[inline(always)] fn can_reconstruct_query_key() -> bool { true } @@ -338,7 +353,7 @@ impl<'tcx> DepNodeParams> for DefId { } impl<'tcx> DepNodeParams> for LocalDefId { - #[inline] + #[inline(always)] fn can_reconstruct_query_key() -> bool { true } @@ -357,7 +372,7 @@ impl<'tcx> DepNodeParams> for LocalDefId { } impl<'tcx> DepNodeParams> for CrateNum { - #[inline] + #[inline(always)] fn can_reconstruct_query_key() -> bool { true } @@ -377,7 +392,7 @@ impl<'tcx> DepNodeParams> for CrateNum { } impl<'tcx> DepNodeParams> for (DefId, DefId) { - #[inline] + #[inline(always)] fn can_reconstruct_query_key() -> bool { false } @@ -402,7 +417,7 @@ impl<'tcx> DepNodeParams> for (DefId, DefId) { } impl<'tcx> DepNodeParams> for HirId { - #[inline] + #[inline(always)] fn can_reconstruct_query_key() -> bool { false } diff --git a/compiler/rustc_query_system/src/dep_graph/dep_node.rs b/compiler/rustc_query_system/src/dep_graph/dep_node.rs index ff52fdab19c50..64aba870502c7 100644 --- a/compiler/rustc_query_system/src/dep_graph/dep_node.rs +++ b/compiler/rustc_query_system/src/dep_graph/dep_node.rs @@ -153,12 +153,6 @@ where } } -impl DepNodeParams for () { - fn to_fingerprint(&self, _: Ctxt) -> Fingerprint { - Fingerprint::ZERO - } -} - /// A "work product" corresponds to a `.o` (or other) file that we /// save in between runs. These IDs do not have a `DefId` but rather /// some independent path or string that persists between runs without From 016ea6b319e12df6ed568a147f9e5f06ceccbfff Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Tue, 27 Oct 2020 18:36:11 +0100 Subject: [PATCH 03/11] Use a side-table of consts instead of matching on the DepKind enum. --- .../rustc_middle/src/dep_graph/dep_node.rs | 48 ++++++++++++++++++- .../rustc_query_system/src/dep_graph/mod.rs | 2 +- 2 files changed, 48 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_middle/src/dep_graph/dep_node.rs b/compiler/rustc_middle/src/dep_graph/dep_node.rs index fa96b800928ec..516184f4629e5 100644 --- a/compiler/rustc_middle/src/dep_graph/dep_node.rs +++ b/compiler/rustc_middle/src/dep_graph/dep_node.rs @@ -70,6 +70,20 @@ use std::hash::Hash; pub use rustc_query_system::dep_graph::{DepContext, DepNodeParams}; +/// This struct stores metadata about each DepKind. +/// +/// Information is retrieved by indexing the `DEP_KINDS` array using the integer value +/// of the `DepKind`. Overall, this allows to implement `DepContext` using this manual +/// jump table instead of large matches. +pub struct DepKindStruct {} + +impl std::ops::Deref for DepKind { + type Target = DepKindStruct; + fn deref(&self) -> &DepKindStruct { + &DEP_KINDS[*self as usize] + } +} + // erase!() just makes tokens go away. It's used to specify which macro argument // is repeated (i.e., which sub-expression of the macro we are in) but don't need // to actually use any of the arguments. @@ -103,6 +117,35 @@ macro_rules! contains_eval_always_attr { ($($attr:ident $(($($attr_args:tt)*))* ),*) => ({$(is_eval_always_attr!($attr) | )* false}); } +#[allow(non_upper_case_globals)] +pub mod dep_kind { + use super::*; + + // We use this for most things when incr. comp. is turned off. + pub const Null: DepKindStruct = DepKindStruct {}; + + // Represents metadata from an extern crate. + pub const CrateMetadata: DepKindStruct = DepKindStruct {}; + + pub const TraitSelect: DepKindStruct = DepKindStruct {}; + + pub const CompileCodegenUnit: DepKindStruct = DepKindStruct {}; + + macro_rules! define_query_dep_kinds { + ($( + [$($attrs:tt)*] + $variant:ident $(( $tuple_arg_ty:ty $(,)? ))* + ,)*) => ( + $(pub const $variant: DepKindStruct = { + DepKindStruct { + } + };)* + ); + } + + rustc_dep_node_append!([define_query_dep_kinds!][]); +} + macro_rules! define_dep_nodes { (<$tcx:tt> $( @@ -110,7 +153,10 @@ macro_rules! define_dep_nodes { $variant:ident $(( $tuple_arg_ty:ty $(,)? ))* ,)* ) => ( - #[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Encodable, Decodable)] + static DEP_KINDS: &[DepKindStruct] = &[ $(dep_kind::$variant),* ]; + + /// This enum serves as an index into the `DEP_KINDS` array. + #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, Encodable, Decodable)] #[allow(non_camel_case_types)] pub enum DepKind { $($variant),* diff --git a/compiler/rustc_query_system/src/dep_graph/mod.rs b/compiler/rustc_query_system/src/dep_graph/mod.rs index da0b5aad6c811..b1c901633a71b 100644 --- a/compiler/rustc_query_system/src/dep_graph/mod.rs +++ b/compiler/rustc_query_system/src/dep_graph/mod.rs @@ -61,7 +61,7 @@ pub trait DepContext: Copy { } /// Describe the different families of dependency nodes. -pub trait DepKind: Copy + fmt::Debug + Eq + Ord + Hash { +pub trait DepKind: Copy + fmt::Debug + Eq + Hash { const NULL: Self; /// Return whether this kind always require evaluation. From 24f0b957e75a314eff99e11103399d5b54629294 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Fri, 20 Nov 2020 19:35:17 +0100 Subject: [PATCH 04/11] Use a field for is_anon. --- .../rustc_middle/src/dep_graph/dep_node.rs | 34 +++++++++---------- compiler/rustc_middle/src/dep_graph/mod.rs | 2 +- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/compiler/rustc_middle/src/dep_graph/dep_node.rs b/compiler/rustc_middle/src/dep_graph/dep_node.rs index 516184f4629e5..a5f632f251643 100644 --- a/compiler/rustc_middle/src/dep_graph/dep_node.rs +++ b/compiler/rustc_middle/src/dep_graph/dep_node.rs @@ -75,7 +75,12 @@ pub use rustc_query_system::dep_graph::{DepContext, DepNodeParams}; /// Information is retrieved by indexing the `DEP_KINDS` array using the integer value /// of the `DepKind`. Overall, this allows to implement `DepContext` using this manual /// jump table instead of large matches. -pub struct DepKindStruct {} +pub struct DepKindStruct { + /// Anonymous queries cannot be replayed from one compiler invocation to the next. + /// When their result is needed, it is recomputed. They are useful for fine-grained + /// dependency tracking, and caching within one compiler invocation. + pub(super) is_anon: bool, +} impl std::ops::Deref for DepKind { type Target = DepKindStruct; @@ -122,14 +127,14 @@ pub mod dep_kind { use super::*; // We use this for most things when incr. comp. is turned off. - pub const Null: DepKindStruct = DepKindStruct {}; + pub const Null: DepKindStruct = DepKindStruct { is_anon: false }; // Represents metadata from an extern crate. - pub const CrateMetadata: DepKindStruct = DepKindStruct {}; + pub const CrateMetadata: DepKindStruct = DepKindStruct { is_anon: false }; - pub const TraitSelect: DepKindStruct = DepKindStruct {}; + pub const TraitSelect: DepKindStruct = DepKindStruct { is_anon: true }; - pub const CompileCodegenUnit: DepKindStruct = DepKindStruct {}; + pub const CompileCodegenUnit: DepKindStruct = DepKindStruct { is_anon: false }; macro_rules! define_query_dep_kinds { ($( @@ -137,7 +142,10 @@ pub mod dep_kind { $variant:ident $(( $tuple_arg_ty:ty $(,)? ))* ,)*) => ( $(pub const $variant: DepKindStruct = { + const is_anon: bool = contains_anon_attr!($($attrs)*); + DepKindStruct { + is_anon, } };)* ); @@ -165,13 +173,13 @@ macro_rules! define_dep_nodes { impl DepKind { #[allow(unreachable_code)] pub fn can_reconstruct_query_key<$tcx>(&self) -> bool { + if self.is_anon { + return false; + } + match *self { $( DepKind :: $variant => { - if contains_anon_attr!($($attrs)*) { - return false; - } - // tuple args $({ return <$tuple_arg_ty as DepNodeParams>> @@ -184,14 +192,6 @@ macro_rules! define_dep_nodes { } } - pub fn is_anon(&self) -> bool { - match *self { - $( - DepKind :: $variant => { contains_anon_attr!($($attrs)*) } - )* - } - } - pub fn is_eval_always(&self) -> bool { match *self { $( diff --git a/compiler/rustc_middle/src/dep_graph/mod.rs b/compiler/rustc_middle/src/dep_graph/mod.rs index 728bfef9f4673..b1ee279d66672 100644 --- a/compiler/rustc_middle/src/dep_graph/mod.rs +++ b/compiler/rustc_middle/src/dep_graph/mod.rs @@ -37,7 +37,7 @@ impl rustc_query_system::dep_graph::DepKind for DepKind { fn debug_node(node: &DepNode, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{:?}", node.kind)?; - if !node.kind.has_params() && !node.kind.is_anon() { + if !node.kind.has_params() && !node.kind.is_anon { return Ok(()); } From d8c87ac080a67327eaaefc336733bc3e5f937448 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Tue, 27 Oct 2020 18:43:49 +0100 Subject: [PATCH 05/11] Use a field for is_eval_always. --- .../rustc_middle/src/dep_graph/dep_node.rs | 24 +++++++++---------- compiler/rustc_middle/src/dep_graph/mod.rs | 3 ++- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/compiler/rustc_middle/src/dep_graph/dep_node.rs b/compiler/rustc_middle/src/dep_graph/dep_node.rs index a5f632f251643..848c8b8ea9cbc 100644 --- a/compiler/rustc_middle/src/dep_graph/dep_node.rs +++ b/compiler/rustc_middle/src/dep_graph/dep_node.rs @@ -80,6 +80,11 @@ pub struct DepKindStruct { /// When their result is needed, it is recomputed. They are useful for fine-grained /// dependency tracking, and caching within one compiler invocation. pub(super) is_anon: bool, + + /// Eval-always queries do not track their dependencies, and are always recomputed, even if + /// their inputs have not changed since the last compiler invocation. The result is still + /// cached within one compiler invocation. + pub(super) is_eval_always: bool, } impl std::ops::Deref for DepKind { @@ -127,14 +132,15 @@ pub mod dep_kind { use super::*; // We use this for most things when incr. comp. is turned off. - pub const Null: DepKindStruct = DepKindStruct { is_anon: false }; + pub const Null: DepKindStruct = DepKindStruct { is_anon: false, is_eval_always: false }; // Represents metadata from an extern crate. - pub const CrateMetadata: DepKindStruct = DepKindStruct { is_anon: false }; + pub const CrateMetadata: DepKindStruct = DepKindStruct { is_anon: false, is_eval_always: true }; - pub const TraitSelect: DepKindStruct = DepKindStruct { is_anon: true }; + pub const TraitSelect: DepKindStruct = DepKindStruct { is_anon: true, is_eval_always: false }; - pub const CompileCodegenUnit: DepKindStruct = DepKindStruct { is_anon: false }; + pub const CompileCodegenUnit: DepKindStruct = + DepKindStruct { is_anon: false, is_eval_always: false }; macro_rules! define_query_dep_kinds { ($( @@ -143,9 +149,11 @@ pub mod dep_kind { ,)*) => ( $(pub const $variant: DepKindStruct = { const is_anon: bool = contains_anon_attr!($($attrs)*); + const is_eval_always: bool = contains_eval_always_attr!($($attrs)*); DepKindStruct { is_anon, + is_eval_always, } };)* ); @@ -192,14 +200,6 @@ macro_rules! define_dep_nodes { } } - pub fn is_eval_always(&self) -> bool { - match *self { - $( - DepKind :: $variant => { contains_eval_always_attr!($($attrs)*) } - )* - } - } - #[allow(unreachable_code)] pub fn has_params(&self) -> bool { match *self { diff --git a/compiler/rustc_middle/src/dep_graph/mod.rs b/compiler/rustc_middle/src/dep_graph/mod.rs index b1ee279d66672..b81b5b3ede107 100644 --- a/compiler/rustc_middle/src/dep_graph/mod.rs +++ b/compiler/rustc_middle/src/dep_graph/mod.rs @@ -26,8 +26,9 @@ pub type SerializedDepGraph = rustc_query_system::dep_graph::SerializedDepGraph< impl rustc_query_system::dep_graph::DepKind for DepKind { const NULL: Self = DepKind::Null; + #[inline(always)] fn is_eval_always(&self) -> bool { - DepKind::is_eval_always(self) + self.is_eval_always } fn has_params(&self) -> bool { From 5027f1c6ea777b0e7485f997b2aed94852c7174f Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Tue, 27 Oct 2020 18:46:43 +0100 Subject: [PATCH 06/11] Use a field for has_params. --- .../rustc_middle/src/dep_graph/dep_node.rs | 37 +++++++------------ compiler/rustc_middle/src/dep_graph/mod.rs | 5 ++- 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/compiler/rustc_middle/src/dep_graph/dep_node.rs b/compiler/rustc_middle/src/dep_graph/dep_node.rs index 848c8b8ea9cbc..bd89b8cfe7c6e 100644 --- a/compiler/rustc_middle/src/dep_graph/dep_node.rs +++ b/compiler/rustc_middle/src/dep_graph/dep_node.rs @@ -76,6 +76,9 @@ pub use rustc_query_system::dep_graph::{DepContext, DepNodeParams}; /// of the `DepKind`. Overall, this allows to implement `DepContext` using this manual /// jump table instead of large matches. pub struct DepKindStruct { + /// Whether the DepNode has parameters (query keys). + pub(super) has_params: bool, + /// Anonymous queries cannot be replayed from one compiler invocation to the next. /// When their result is needed, it is recomputed. They are useful for fine-grained /// dependency tracking, and caching within one compiler invocation. @@ -132,15 +135,18 @@ pub mod dep_kind { use super::*; // We use this for most things when incr. comp. is turned off. - pub const Null: DepKindStruct = DepKindStruct { is_anon: false, is_eval_always: false }; + pub const Null: DepKindStruct = + DepKindStruct { has_params: false, is_anon: false, is_eval_always: false }; // Represents metadata from an extern crate. - pub const CrateMetadata: DepKindStruct = DepKindStruct { is_anon: false, is_eval_always: true }; + pub const CrateMetadata: DepKindStruct = + DepKindStruct { has_params: true, is_anon: false, is_eval_always: true }; - pub const TraitSelect: DepKindStruct = DepKindStruct { is_anon: true, is_eval_always: false }; + pub const TraitSelect: DepKindStruct = + DepKindStruct { has_params: false, is_anon: true, is_eval_always: false }; pub const CompileCodegenUnit: DepKindStruct = - DepKindStruct { is_anon: false, is_eval_always: false }; + DepKindStruct { has_params: true, is_anon: false, is_eval_always: false }; macro_rules! define_query_dep_kinds { ($( @@ -148,10 +154,12 @@ pub mod dep_kind { $variant:ident $(( $tuple_arg_ty:ty $(,)? ))* ,)*) => ( $(pub const $variant: DepKindStruct = { + const has_params: bool = $({ erase!($tuple_arg_ty); true } |)* false; const is_anon: bool = contains_anon_attr!($($attrs)*); const is_eval_always: bool = contains_eval_always_attr!($($attrs)*); DepKindStruct { + has_params, is_anon, is_eval_always, } @@ -199,23 +207,6 @@ macro_rules! define_dep_nodes { )* } } - - #[allow(unreachable_code)] - pub fn has_params(&self) -> bool { - match *self { - $( - DepKind :: $variant => { - // tuple args - $({ - erase!($tuple_arg_ty); - return true; - })* - - false - } - )* - } - } } pub struct DepConstructor; @@ -308,7 +299,7 @@ impl DepNodeExt for DepNode { /// method will assert that the given DepKind actually requires a /// single DefId/DefPathHash parameter. fn from_def_path_hash(def_path_hash: DefPathHash, kind: DepKind) -> DepNode { - debug_assert!(kind.can_reconstruct_query_key() && kind.has_params()); + debug_assert!(kind.can_reconstruct_query_key() && kind.has_params); DepNode { kind, hash: def_path_hash.0.into() } } @@ -341,7 +332,7 @@ impl DepNodeExt for DepNode { return Err(()); } - if kind.has_params() { + if kind.has_params { Ok(DepNode::from_def_path_hash(def_path_hash, kind)) } else { Ok(DepNode::new_no_params(kind)) diff --git a/compiler/rustc_middle/src/dep_graph/mod.rs b/compiler/rustc_middle/src/dep_graph/mod.rs index b81b5b3ede107..546c3877e4d8e 100644 --- a/compiler/rustc_middle/src/dep_graph/mod.rs +++ b/compiler/rustc_middle/src/dep_graph/mod.rs @@ -31,14 +31,15 @@ impl rustc_query_system::dep_graph::DepKind for DepKind { self.is_eval_always } + #[inline(always)] fn has_params(&self) -> bool { - DepKind::has_params(self) + self.has_params } fn debug_node(node: &DepNode, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, "{:?}", node.kind)?; - if !node.kind.has_params() && !node.kind.is_anon { + if !node.kind.has_params && !node.kind.is_anon { return Ok(()); } From 438c430c765ca531f17dfed300c1ff4b35ca3cce Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Tue, 27 Oct 2020 19:49:10 +0100 Subject: [PATCH 07/11] Make can_reconstruct_query_key a function pointer. --- .../rustc_middle/src/dep_graph/dep_node.rs | 87 ++++++++++++------- compiler/rustc_middle/src/dep_graph/mod.rs | 9 +- 2 files changed, 61 insertions(+), 35 deletions(-) diff --git a/compiler/rustc_middle/src/dep_graph/dep_node.rs b/compiler/rustc_middle/src/dep_graph/dep_node.rs index bd89b8cfe7c6e..0d58ac7ae4930 100644 --- a/compiler/rustc_middle/src/dep_graph/dep_node.rs +++ b/compiler/rustc_middle/src/dep_graph/dep_node.rs @@ -88,6 +88,12 @@ pub struct DepKindStruct { /// their inputs have not changed since the last compiler invocation. The result is still /// cached within one compiler invocation. pub(super) is_eval_always: bool, + + /// Whether the query key can be recovered from the hashed fingerprint. + /// See [DepNodeParams] trait for the behaviour of each key type. + // FIXME: Make this a simple boolean once DepNodeParams::can_reconstruct_query_key + // can be made a specialized associated const. + can_reconstruct_query_key: fn() -> bool, } impl std::ops::Deref for DepKind { @@ -97,6 +103,19 @@ impl std::ops::Deref for DepKind { } } +impl DepKind { + #[inline(always)] + pub fn can_reconstruct_query_key(&self) -> bool { + // Only fetch the DepKindStruct once. + let data: &DepKindStruct = &**self; + if data.is_anon { + return false; + } + + (data.can_reconstruct_query_key)() + } +} + // erase!() just makes tokens go away. It's used to specify which macro argument // is repeated (i.e., which sub-expression of the macro we are in) but don't need // to actually use any of the arguments. @@ -133,20 +152,41 @@ macro_rules! contains_eval_always_attr { #[allow(non_upper_case_globals)] pub mod dep_kind { use super::*; + use crate::ty::query::query_keys; // We use this for most things when incr. comp. is turned off. - pub const Null: DepKindStruct = - DepKindStruct { has_params: false, is_anon: false, is_eval_always: false }; + pub const Null: DepKindStruct = DepKindStruct { + has_params: false, + is_anon: false, + is_eval_always: false, + + can_reconstruct_query_key: || true, + }; // Represents metadata from an extern crate. - pub const CrateMetadata: DepKindStruct = - DepKindStruct { has_params: true, is_anon: false, is_eval_always: true }; + pub const CrateMetadata: DepKindStruct = DepKindStruct { + has_params: true, + is_anon: false, + is_eval_always: true, + + can_reconstruct_query_key: || true, + }; + + pub const TraitSelect: DepKindStruct = DepKindStruct { + has_params: false, + is_anon: true, + is_eval_always: false, - pub const TraitSelect: DepKindStruct = - DepKindStruct { has_params: false, is_anon: true, is_eval_always: false }; + can_reconstruct_query_key: || false, + }; + + pub const CompileCodegenUnit: DepKindStruct = DepKindStruct { + has_params: true, + is_anon: false, + is_eval_always: false, - pub const CompileCodegenUnit: DepKindStruct = - DepKindStruct { has_params: true, is_anon: false, is_eval_always: false }; + can_reconstruct_query_key: || false, + }; macro_rules! define_query_dep_kinds { ($( @@ -158,10 +198,18 @@ pub mod dep_kind { const is_anon: bool = contains_anon_attr!($($attrs)*); const is_eval_always: bool = contains_eval_always_attr!($($attrs)*); + #[inline(always)] + fn can_reconstruct_query_key() -> bool { + !is_anon && + as DepNodeParams>> + ::can_reconstruct_query_key() + } + DepKindStruct { has_params, is_anon, is_eval_always, + can_reconstruct_query_key, } };)* ); @@ -186,29 +234,6 @@ macro_rules! define_dep_nodes { $($variant),* } - impl DepKind { - #[allow(unreachable_code)] - pub fn can_reconstruct_query_key<$tcx>(&self) -> bool { - if self.is_anon { - return false; - } - - match *self { - $( - DepKind :: $variant => { - // tuple args - $({ - return <$tuple_arg_ty as DepNodeParams>> - ::can_reconstruct_query_key(); - })* - - true - } - )* - } - } - } - pub struct DepConstructor; #[allow(non_camel_case_types)] diff --git a/compiler/rustc_middle/src/dep_graph/mod.rs b/compiler/rustc_middle/src/dep_graph/mod.rs index 546c3877e4d8e..d4cbbfa86ae2b 100644 --- a/compiler/rustc_middle/src/dep_graph/mod.rs +++ b/compiler/rustc_middle/src/dep_graph/mod.rs @@ -26,6 +26,11 @@ pub type SerializedDepGraph = rustc_query_system::dep_graph::SerializedDepGraph< impl rustc_query_system::dep_graph::DepKind for DepKind { const NULL: Self = DepKind::Null; + #[inline(always)] + fn can_reconstruct_query_key(&self) -> bool { + DepKind::can_reconstruct_query_key(self) + } + #[inline(always)] fn is_eval_always(&self) -> bool { self.is_eval_always @@ -83,10 +88,6 @@ impl rustc_query_system::dep_graph::DepKind for DepKind { op(icx.task_deps) }) } - - fn can_reconstruct_query_key(&self) -> bool { - DepKind::can_reconstruct_query_key(self) - } } impl<'tcx> DepContext for TyCtxt<'tcx> { From bee1fbb67e6409494986af8bc2ac3048b750f29a Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Tue, 27 Oct 2020 19:22:58 +0100 Subject: [PATCH 08/11] Make try_load_from_on_disk_cache a function pointer. --- .../rustc_middle/src/dep_graph/dep_node.rs | 31 ++++++++++++++++++- compiler/rustc_middle/src/dep_graph/mod.rs | 3 +- compiler/rustc_middle/src/ty/query/mod.rs | 26 ---------------- 3 files changed, 31 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_middle/src/dep_graph/dep_node.rs b/compiler/rustc_middle/src/dep_graph/dep_node.rs index 0d58ac7ae4930..899a805a8d461 100644 --- a/compiler/rustc_middle/src/dep_graph/dep_node.rs +++ b/compiler/rustc_middle/src/dep_graph/dep_node.rs @@ -94,6 +94,9 @@ pub struct DepKindStruct { // FIXME: Make this a simple boolean once DepNodeParams::can_reconstruct_query_key // can be made a specialized associated const. can_reconstruct_query_key: fn() -> bool, + + /// Invoke a query to put the on-disk cached value in memory. + pub(super) try_load_from_on_disk_cache: fn(TyCtxt<'_>, &DepNode), } impl std::ops::Deref for DepKind { @@ -152,7 +155,8 @@ macro_rules! contains_eval_always_attr { #[allow(non_upper_case_globals)] pub mod dep_kind { use super::*; - use crate::ty::query::query_keys; + use crate::ty::query::{queries, query_keys}; + use rustc_query_system::query::QueryDescription; // We use this for most things when incr. comp. is turned off. pub const Null: DepKindStruct = DepKindStruct { @@ -161,6 +165,7 @@ pub mod dep_kind { is_eval_always: false, can_reconstruct_query_key: || true, + try_load_from_on_disk_cache: |_, _| {}, }; // Represents metadata from an extern crate. @@ -170,6 +175,7 @@ pub mod dep_kind { is_eval_always: true, can_reconstruct_query_key: || true, + try_load_from_on_disk_cache: |_, _| {}, }; pub const TraitSelect: DepKindStruct = DepKindStruct { @@ -178,6 +184,7 @@ pub mod dep_kind { is_eval_always: false, can_reconstruct_query_key: || false, + try_load_from_on_disk_cache: |_, _| {}, }; pub const CompileCodegenUnit: DepKindStruct = DepKindStruct { @@ -186,6 +193,7 @@ pub mod dep_kind { is_eval_always: false, can_reconstruct_query_key: || false, + try_load_from_on_disk_cache: |_, _| {}, }; macro_rules! define_query_dep_kinds { @@ -205,11 +213,32 @@ pub mod dep_kind { ::can_reconstruct_query_key() } + fn recover<'tcx>(tcx: TyCtxt<'tcx>, dep_node: &DepNode) -> Option> { + as DepNodeParams>>::recover(tcx, dep_node) + } + + fn try_load_from_on_disk_cache(tcx: TyCtxt<'_>, dep_node: &DepNode) { + if !can_reconstruct_query_key() { + return + } + + debug_assert!(tcx.dep_graph + .node_color(dep_node) + .map(|c| c.is_green()) + .unwrap_or(false)); + + let key = recover(tcx, dep_node).unwrap_or_else(|| panic!("Failed to recover key for {:?} with hash {}", dep_node, dep_node.hash)); + if queries::$variant::cache_on_disk(tcx, &key, None) { + let _ = tcx.$variant(key); + } + } + DepKindStruct { has_params, is_anon, is_eval_always, can_reconstruct_query_key, + try_load_from_on_disk_cache, } };)* ); diff --git a/compiler/rustc_middle/src/dep_graph/mod.rs b/compiler/rustc_middle/src/dep_graph/mod.rs index d4cbbfa86ae2b..88441af674d33 100644 --- a/compiler/rustc_middle/src/dep_graph/mod.rs +++ b/compiler/rustc_middle/src/dep_graph/mod.rs @@ -1,5 +1,4 @@ use crate::ich::StableHashingContext; -use crate::ty::query::try_load_from_on_disk_cache; use crate::ty::{self, TyCtxt}; use rustc_data_structures::profiling::SelfProfilerRef; use rustc_data_structures::sync::Lock; @@ -169,7 +168,7 @@ impl<'tcx> DepContext for TyCtxt<'tcx> { // Interactions with on_disk_cache fn try_load_from_on_disk_cache(&self, dep_node: &DepNode) { - try_load_from_on_disk_cache(*self, dep_node) + (dep_node.kind.try_load_from_on_disk_cache)(*self, dep_node) } fn load_diagnostics(&self, prev_dep_node_index: SerializedDepNodeIndex) -> Vec { diff --git a/compiler/rustc_middle/src/ty/query/mod.rs b/compiler/rustc_middle/src/ty/query/mod.rs index b269dd09b72ea..fd0a93899b114 100644 --- a/compiler/rustc_middle/src/ty/query/mod.rs +++ b/compiler/rustc_middle/src/ty/query/mod.rs @@ -209,32 +209,6 @@ pub fn force_from_dep_node<'tcx>(tcx: TyCtxt<'tcx>, dep_node: &DepNode) -> bool false } -pub(crate) fn try_load_from_on_disk_cache<'tcx>(tcx: TyCtxt<'tcx>, dep_node: &DepNode) { - macro_rules! try_load_from_on_disk_cache { - ($($name:ident,)*) => { - match dep_node.kind { - $(DepKind::$name => { - if as DepNodeParams>>::can_reconstruct_query_key() { - debug_assert!(tcx.dep_graph - .node_color(dep_node) - .map(|c| c.is_green()) - .unwrap_or(false)); - - let key = as DepNodeParams>>::recover(tcx, dep_node).unwrap_or_else(|| panic!("Failed to recover key for {:?} with hash {}", dep_node, dep_node.hash)); - if queries::$name::cache_on_disk(tcx, &key, None) { - let _ = tcx.$name(key); - } - } - })* - - _ => (), - } - } - } - - rustc_cached_queries!(try_load_from_on_disk_cache!); -} - mod sealed { use super::{DefId, LocalDefId}; From 921b2841671baba79b23d17cf37170667c59fdd2 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Tue, 27 Oct 2020 19:33:03 +0100 Subject: [PATCH 09/11] Make force_from_dep_node a function pointer. --- .../rustc_middle/src/dep_graph/dep_node.rs | 70 +++++++++++- compiler/rustc_middle/src/dep_graph/mod.rs | 22 +++- compiler/rustc_middle/src/ty/query/mod.rs | 108 +----------------- 3 files changed, 90 insertions(+), 110 deletions(-) diff --git a/compiler/rustc_middle/src/dep_graph/dep_node.rs b/compiler/rustc_middle/src/dep_graph/dep_node.rs index 899a805a8d461..86707b4283f78 100644 --- a/compiler/rustc_middle/src/dep_graph/dep_node.rs +++ b/compiler/rustc_middle/src/dep_graph/dep_node.rs @@ -66,6 +66,7 @@ use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, CRATE_DEF_INDEX}; use rustc_hir::definitions::DefPathHash; use rustc_hir::HirId; use rustc_span::symbol::Symbol; +use rustc_span::DUMMY_SP; use std::hash::Hash; pub use rustc_query_system::dep_graph::{DepContext, DepNodeParams}; @@ -95,6 +96,50 @@ pub struct DepKindStruct { // can be made a specialized associated const. can_reconstruct_query_key: fn() -> bool, + /// 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` + /// where we don't know if it is red or green and we therefore actually have + /// to recompute its value in order to find out. Since the only piece of + /// information that we have at that point is the `DepNode` we are trying to + /// re-evaluate, we need some way to re-run a query from just that. This is what + /// `force_from_dep_node()` implements. + /// + /// In the general case, a `DepNode` consists of a `DepKind` and an opaque + /// GUID/fingerprint that will uniquely identify the node. This GUID/fingerprint + /// is usually constructed by computing a stable hash of the query-key that the + /// `DepNode` corresponds to. Consequently, it is not in general possible to go + /// back from hash to query-key (since hash functions are not reversible). For + /// this reason `force_from_dep_node()` is expected to fail from time to time + /// because we just cannot find out, from the `DepNode` alone, what the + /// corresponding query-key is and therefore cannot re-run the query. + /// + /// The system deals with this case letting `try_mark_green` fail which forces + /// the root query to be re-evaluated. + /// + /// Now, if `force_from_dep_node()` would always fail, it would be pretty useless. + /// Fortunately, we can use some contextual information that will allow us to + /// reconstruct query-keys for certain kinds of `DepNode`s. In particular, we + /// enforce by construction that the GUID/fingerprint of certain `DepNode`s is a + /// valid `DefPathHash`. Since we also always build a huge table that maps every + /// `DefPathHash` in the current codebase to the corresponding `DefId`, we have + /// everything we need to re-run the query. + /// + /// Take the `mir_promoted` query as an example. Like many other queries, it + /// just has a single parameter: the `DefId` of the item it will compute the + /// validated MIR for. Now, when we call `force_from_dep_node()` on a `DepNode` + /// with kind `MirValidated`, we know that the GUID/fingerprint of the `DepNode` + /// is actually a `DefPathHash`, and can therefore just look up the corresponding + /// `DefId` in `tcx.def_path_hash_to_def_id`. + /// + /// When you implement a new query, it will likely have a corresponding new + /// `DepKind`, and you'll have to support it here in `force_from_dep_node()`. As + /// a rule of thumb, if your query takes a `DefId` or `LocalDefId` as sole parameter, + /// then `force_from_dep_node()` should not fail for it. Otherwise, you can just + /// add it to the "We don't have enough information to reconstruct..." group in + /// the match below. + pub(super) force_from_dep_node: fn(tcx: TyCtxt<'_>, dep_node: &DepNode) -> bool, + /// Invoke a query to put the on-disk cached value in memory. pub(super) try_load_from_on_disk_cache: fn(TyCtxt<'_>, &DepNode), } @@ -156,7 +201,7 @@ macro_rules! contains_eval_always_attr { pub mod dep_kind { use super::*; use crate::ty::query::{queries, query_keys}; - use rustc_query_system::query::QueryDescription; + use rustc_query_system::query::{force_query, QueryDescription}; // We use this for most things when incr. comp. is turned off. pub const Null: DepKindStruct = DepKindStruct { @@ -165,6 +210,7 @@ pub mod dep_kind { is_eval_always: false, can_reconstruct_query_key: || true, + force_from_dep_node: |_, dep_node| bug!("force_from_dep_node: encountered {:?}", dep_node), try_load_from_on_disk_cache: |_, _| {}, }; @@ -175,6 +221,7 @@ pub mod dep_kind { is_eval_always: true, can_reconstruct_query_key: || true, + force_from_dep_node: |_, dep_node| bug!("force_from_dep_node: encountered {:?}", dep_node), try_load_from_on_disk_cache: |_, _| {}, }; @@ -184,6 +231,7 @@ pub mod dep_kind { is_eval_always: false, can_reconstruct_query_key: || false, + force_from_dep_node: |_, _| false, try_load_from_on_disk_cache: |_, _| {}, }; @@ -193,6 +241,7 @@ pub mod dep_kind { is_eval_always: false, can_reconstruct_query_key: || false, + force_from_dep_node: |_, _| false, try_load_from_on_disk_cache: |_, _| {}, }; @@ -217,6 +266,24 @@ pub mod dep_kind { as DepNodeParams>>::recover(tcx, dep_node) } + fn force_from_dep_node(tcx: TyCtxt<'_>, dep_node: &DepNode) -> bool { + if !can_reconstruct_query_key() { + return false; + } + + if let Some(key) = recover(tcx, dep_node) { + force_query::, _>( + tcx, + key, + DUMMY_SP, + *dep_node + ); + return true; + } + + false + } + fn try_load_from_on_disk_cache(tcx: TyCtxt<'_>, dep_node: &DepNode) { if !can_reconstruct_query_key() { return @@ -238,6 +305,7 @@ pub mod dep_kind { is_anon, is_eval_always, can_reconstruct_query_key, + force_from_dep_node, try_load_from_on_disk_cache, } };)* diff --git a/compiler/rustc_middle/src/dep_graph/mod.rs b/compiler/rustc_middle/src/dep_graph/mod.rs index 88441af674d33..a45617f9a5fe8 100644 --- a/compiler/rustc_middle/src/dep_graph/mod.rs +++ b/compiler/rustc_middle/src/dep_graph/mod.rs @@ -8,7 +8,6 @@ use rustc_hir::def_id::LocalDefId; mod dep_node; -pub(crate) use rustc_query_system::dep_graph::DepNodeParams; pub use rustc_query_system::dep_graph::{ debug, hash_result, DepContext, DepNodeColor, DepNodeIndex, SerializedDepNodeIndex, WorkProduct, WorkProductId, @@ -155,7 +154,26 @@ impl<'tcx> DepContext for TyCtxt<'tcx> { } debug!("try_force_from_dep_node({:?}) --- trying to force", dep_node); - ty::query::force_from_dep_node(*self, dep_node) + + // We must avoid ever having to call `force_from_dep_node()` for a + // `DepNode::codegen_unit`: + // Since we cannot reconstruct the query key of a `DepNode::codegen_unit`, we + // would always end up having to evaluate the first caller of the + // `codegen_unit` query that *is* reconstructible. This might very well be + // the `compile_codegen_unit` query, thus re-codegenning the whole CGU just + // to re-trigger calling the `codegen_unit` query with the right key. At + // that point we would already have re-done all the work we are trying to + // avoid doing in the first place. + // The solution is simple: Just explicitly call the `codegen_unit` query for + // each CGU, right after partitioning. This way `try_mark_green` will always + // hit the cache instead of having to go through `force_from_dep_node`. + // This assertion makes sure, we actually keep applying the solution above. + debug_assert!( + dep_node.kind != DepKind::codegen_unit, + "calling force_from_dep_node() on DepKind::codegen_unit" + ); + + (dep_node.kind.force_from_dep_node)(*self, dep_node) } fn has_errors_or_delayed_span_bugs(&self) -> bool { diff --git a/compiler/rustc_middle/src/ty/query/mod.rs b/compiler/rustc_middle/src/ty/query/mod.rs index fd0a93899b114..acfa58e511ed1 100644 --- a/compiler/rustc_middle/src/ty/query/mod.rs +++ b/compiler/rustc_middle/src/ty/query/mod.rs @@ -1,4 +1,4 @@ -use crate::dep_graph::{self, DepKind, DepNode, DepNodeParams}; +use crate::dep_graph; use crate::hir::exports::Export; use crate::hir::map; use crate::infer::canonical::{self, Canonical}; @@ -103,112 +103,6 @@ pub use self::profiling_support::{IntoSelfProfilingString, QueryKeyStringBuilder rustc_query_append! { [define_queries!][<'tcx>] } -/// 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` -/// where we don't know if it is red or green and we therefore actually have -/// to recompute its value in order to find out. Since the only piece of -/// information that we have at that point is the `DepNode` we are trying to -/// re-evaluate, we need some way to re-run a query from just that. This is what -/// `force_from_dep_node()` implements. -/// -/// In the general case, a `DepNode` consists of a `DepKind` and an opaque -/// GUID/fingerprint that will uniquely identify the node. This GUID/fingerprint -/// is usually constructed by computing a stable hash of the query-key that the -/// `DepNode` corresponds to. Consequently, it is not in general possible to go -/// back from hash to query-key (since hash functions are not reversible). For -/// this reason `force_from_dep_node()` is expected to fail from time to time -/// because we just cannot find out, from the `DepNode` alone, what the -/// corresponding query-key is and therefore cannot re-run the query. -/// -/// The system deals with this case letting `try_mark_green` fail which forces -/// the root query to be re-evaluated. -/// -/// Now, if `force_from_dep_node()` would always fail, it would be pretty useless. -/// Fortunately, we can use some contextual information that will allow us to -/// reconstruct query-keys for certain kinds of `DepNode`s. In particular, we -/// enforce by construction that the GUID/fingerprint of certain `DepNode`s is a -/// valid `DefPathHash`. Since we also always build a huge table that maps every -/// `DefPathHash` in the current codebase to the corresponding `DefId`, we have -/// everything we need to re-run the query. -/// -/// Take the `mir_promoted` query as an example. Like many other queries, it -/// just has a single parameter: the `DefId` of the item it will compute the -/// validated MIR for. Now, when we call `force_from_dep_node()` on a `DepNode` -/// with kind `MirValidated`, we know that the GUID/fingerprint of the `DepNode` -/// is actually a `DefPathHash`, and can therefore just look up the corresponding -/// `DefId` in `tcx.def_path_hash_to_def_id`. -/// -/// When you implement a new query, it will likely have a corresponding new -/// `DepKind`, and you'll have to support it here in `force_from_dep_node()`. As -/// a rule of thumb, if your query takes a `DefId` or `LocalDefId` as sole parameter, -/// then `force_from_dep_node()` should not fail for it. Otherwise, you can just -/// add it to the "We don't have enough information to reconstruct..." group in -/// the match below. -pub fn force_from_dep_node<'tcx>(tcx: TyCtxt<'tcx>, dep_node: &DepNode) -> bool { - // We must avoid ever having to call `force_from_dep_node()` for a - // `DepNode::codegen_unit`: - // Since we cannot reconstruct the query key of a `DepNode::codegen_unit`, we - // would always end up having to evaluate the first caller of the - // `codegen_unit` query that *is* reconstructible. This might very well be - // the `compile_codegen_unit` query, thus re-codegenning the whole CGU just - // to re-trigger calling the `codegen_unit` query with the right key. At - // that point we would already have re-done all the work we are trying to - // avoid doing in the first place. - // The solution is simple: Just explicitly call the `codegen_unit` query for - // each CGU, right after partitioning. This way `try_mark_green` will always - // hit the cache instead of having to go through `force_from_dep_node`. - // This assertion makes sure, we actually keep applying the solution above. - debug_assert!( - dep_node.kind != DepKind::codegen_unit, - "calling force_from_dep_node() on DepKind::codegen_unit" - ); - - if !dep_node.kind.can_reconstruct_query_key() { - return false; - } - - macro_rules! force_from_dep_node { - ($($(#[$attr:meta])* [$($modifiers:tt)*] $name:ident($K:ty),)*) => { - match dep_node.kind { - // These are inputs that are expected to be pre-allocated and that - // should therefore always be red or green already. - DepKind::CrateMetadata | - - // These are anonymous nodes. - DepKind::TraitSelect | - - // We don't have enough information to reconstruct the query key of - // these. - DepKind::CompileCodegenUnit | - - // Forcing this makes no sense. - DepKind::Null => { - bug!("force_from_dep_node: encountered {:?}", dep_node) - } - - $(DepKind::$name => { - debug_assert!(<$K as DepNodeParams>>::can_reconstruct_query_key()); - - if let Some(key) = <$K as DepNodeParams>>::recover(tcx, dep_node) { - force_query::, _>( - tcx, - key, - DUMMY_SP, - *dep_node - ); - return true; - } - })* - } - } - } - - rustc_dep_node_append! { [force_from_dep_node!][] } - - false -} - mod sealed { use super::{DefId, LocalDefId}; From 5fcc537d181f5f2176ec144d277e93ec92b39085 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Fri, 25 Dec 2020 19:07:28 +0100 Subject: [PATCH 10/11] Make DepConstructor a module. --- compiler/rustc_middle/src/dep_graph/dep_node.rs | 12 ++++++------ compiler/rustc_middle/src/dep_graph/mod.rs | 2 +- compiler/rustc_middle/src/mir/mono.rs | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/compiler/rustc_middle/src/dep_graph/dep_node.rs b/compiler/rustc_middle/src/dep_graph/dep_node.rs index 86707b4283f78..c4379670f9906 100644 --- a/compiler/rustc_middle/src/dep_graph/dep_node.rs +++ b/compiler/rustc_middle/src/dep_graph/dep_node.rs @@ -29,8 +29,8 @@ //! contained no `DefId` for thing that had been removed. //! //! `DepNode` definition happens in the `define_dep_nodes!()` macro. This macro -//! defines the `DepKind` enum and a corresponding `DepConstructor` enum. The -//! `DepConstructor` enum links a `DepKind` to the parameters that are needed at +//! defines the `DepKind` enum and a corresponding `dep_constructor` module. The +//! `dep_constructor` module links a `DepKind` to the parameters that are needed at //! runtime in order to construct a valid `DepNode` fingerprint. //! //! Because the macro sees what parameters a given `DepKind` requires, it can @@ -44,7 +44,7 @@ //! `DefId` it was computed from. In other cases, too much information gets //! lost during fingerprint computation. //! -//! The `DepConstructor` enum, together with `DepNode::new()`, ensures that only +//! The `dep_constructor` module, together with `DepNode::new()`, ensures that only //! valid `DepNode` instances can be constructed. For example, the API does not //! allow for constructing parameterless `DepNode`s with anything other //! than a zeroed out fingerprint. More generally speaking, it relieves the @@ -331,10 +331,10 @@ macro_rules! define_dep_nodes { $($variant),* } - pub struct DepConstructor; - #[allow(non_camel_case_types)] - impl DepConstructor { + pub mod dep_constructor { + use super::*; + $( #[inline(always)] #[allow(unreachable_code, non_snake_case)] diff --git a/compiler/rustc_middle/src/dep_graph/mod.rs b/compiler/rustc_middle/src/dep_graph/mod.rs index a45617f9a5fe8..22e9cc1cd3ee4 100644 --- a/compiler/rustc_middle/src/dep_graph/mod.rs +++ b/compiler/rustc_middle/src/dep_graph/mod.rs @@ -13,7 +13,7 @@ pub use rustc_query_system::dep_graph::{ WorkProduct, WorkProductId, }; -pub use dep_node::{label_strs, DepConstructor, DepKind, DepNode, DepNodeExt}; +pub use dep_node::{dep_constructor, label_strs, DepKind, DepNode, DepNodeExt}; pub type DepGraph = rustc_query_system::dep_graph::DepGraph; pub type TaskDeps = rustc_query_system::dep_graph::TaskDeps; diff --git a/compiler/rustc_middle/src/mir/mono.rs b/compiler/rustc_middle/src/mir/mono.rs index 1e70f7605045e..f810f6a56a520 100644 --- a/compiler/rustc_middle/src/mir/mono.rs +++ b/compiler/rustc_middle/src/mir/mono.rs @@ -1,4 +1,4 @@ -use crate::dep_graph::{DepConstructor, DepNode, WorkProduct, WorkProductId}; +use crate::dep_graph::{dep_constructor, DepNode, WorkProduct, WorkProductId}; use crate::ich::{NodeIdHashingMode, StableHashingContext}; use crate::ty::{subst::InternalSubsts, Instance, InstanceDef, SymbolName, TyCtxt}; use rustc_attr::InlineAttr; @@ -362,7 +362,7 @@ impl<'tcx> CodegenUnit<'tcx> { } pub fn codegen_dep_node(&self, tcx: TyCtxt<'tcx>) -> DepNode { - DepConstructor::CompileCodegenUnit(tcx, self.name()) + dep_constructor::CompileCodegenUnit(tcx, self.name()) } } From 0f334c3642257f711e0c397fec11707d86e14e70 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Wed, 6 Jan 2021 18:54:27 +0100 Subject: [PATCH 11/11] Check is_anon outside of can_reconstruct_query_key. --- compiler/rustc_middle/src/dep_graph/dep_node.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_middle/src/dep_graph/dep_node.rs b/compiler/rustc_middle/src/dep_graph/dep_node.rs index c4379670f9906..b775846bba452 100644 --- a/compiler/rustc_middle/src/dep_graph/dep_node.rs +++ b/compiler/rustc_middle/src/dep_graph/dep_node.rs @@ -230,7 +230,7 @@ pub mod dep_kind { is_anon: true, is_eval_always: false, - can_reconstruct_query_key: || false, + can_reconstruct_query_key: || true, force_from_dep_node: |_, _| false, try_load_from_on_disk_cache: |_, _| {}, }; @@ -257,7 +257,6 @@ pub mod dep_kind { #[inline(always)] fn can_reconstruct_query_key() -> bool { - !is_anon && as DepNodeParams>> ::can_reconstruct_query_key() } @@ -267,6 +266,10 @@ pub mod dep_kind { } fn force_from_dep_node(tcx: TyCtxt<'_>, dep_node: &DepNode) -> bool { + if is_anon { + return false; + } + if !can_reconstruct_query_key() { return false; } @@ -285,6 +288,10 @@ pub mod dep_kind { } fn try_load_from_on_disk_cache(tcx: TyCtxt<'_>, dep_node: &DepNode) { + if is_anon { + return + } + if !can_reconstruct_query_key() { return }