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

Refactor fingerprint reconstruction #89343

Merged
merged 2 commits into from
Oct 9, 2021
Merged
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
72 changes: 38 additions & 34 deletions compiler/rustc_middle/src/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ 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_span::symbol::Symbol;
use std::hash::Hash;

Expand All @@ -89,9 +90,9 @@ pub struct DepKindStruct {

/// 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
// FIXME: Make this a simple boolean once DepNodeParams::fingerprint_style
// can be made a specialized associated const.
can_reconstruct_query_key: fn() -> bool,
fingerprint_style: fn() -> FingerprintStyle,
}

impl std::ops::Deref for DepKind {
Expand All @@ -103,14 +104,14 @@ impl std::ops::Deref for DepKind {

impl DepKind {
#[inline(always)]
pub fn can_reconstruct_query_key(&self) -> bool {
pub fn fingerprint_style(&self) -> FingerprintStyle {
// Only fetch the DepKindStruct once.
let data: &DepKindStruct = &**self;
if data.is_anon {
return false;
return FingerprintStyle::Opaque;
}

(data.can_reconstruct_query_key)()
(data.fingerprint_style)()
}
}

Expand Down Expand Up @@ -151,38 +152,39 @@ macro_rules! contains_eval_always_attr {
pub mod dep_kind {
use super::*;
use crate::ty::query::query_keys;
use rustc_query_system::dep_graph::FingerprintStyle;

// 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,

can_reconstruct_query_key: || true,
fingerprint_style: || FingerprintStyle::Unit,
};

pub const TraitSelect: DepKindStruct = DepKindStruct {
has_params: false,
is_anon: true,
is_eval_always: false,

can_reconstruct_query_key: || true,
fingerprint_style: || FingerprintStyle::Unit,
};

pub const CompileCodegenUnit: DepKindStruct = DepKindStruct {
has_params: true,
is_anon: false,
is_eval_always: false,

can_reconstruct_query_key: || false,
fingerprint_style: || FingerprintStyle::Opaque,
};

pub const CompileMonoItem: DepKindStruct = DepKindStruct {
has_params: true,
is_anon: false,
is_eval_always: false,

can_reconstruct_query_key: || false,
fingerprint_style: || FingerprintStyle::Opaque,
};

macro_rules! define_query_dep_kinds {
Expand All @@ -196,16 +198,16 @@ pub mod dep_kind {
const is_eval_always: bool = contains_eval_always_attr!($($attrs)*);

#[inline(always)]
fn can_reconstruct_query_key() -> bool {
fn fingerprint_style() -> rustc_query_system::dep_graph::FingerprintStyle {
<query_keys::$variant<'_> as DepNodeParams<TyCtxt<'_>>>
::can_reconstruct_query_key()
::fingerprint_style()
}

DepKindStruct {
has_params,
is_anon,
is_eval_always,
can_reconstruct_query_key,
fingerprint_style,
}
};)*
);
Expand Down Expand Up @@ -320,7 +322,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.fingerprint_style() == FingerprintStyle::DefPathHash);
DepNode { kind, hash: def_path_hash.0.into() }
}

Expand All @@ -335,7 +337,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.can_reconstruct_query_key() {
if self.kind.fingerprint_style() == FingerprintStyle::DefPathHash {
Some(
tcx.on_disk_cache
.as_ref()?
Expand All @@ -350,14 +352,16 @@ impl DepNodeExt for DepNode {
fn from_label_string(label: &str, def_path_hash: DefPathHash) -> Result<DepNode, ()> {
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))
match kind.fingerprint_style() {
FingerprintStyle::Opaque => Err(()),
FingerprintStyle::Unit => {
if !kind.has_params {
Copy link
Contributor

Choose a reason for hiding this comment

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

FingerprintStyle::Unit should now be equivalent to has_params == false, isn't it? We should be able to get rid of has_params.

Copy link
Member Author

@Mark-Simulacrum Mark-Simulacrum Oct 2, 2021

Choose a reason for hiding this comment

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

Not currently. A query with () key currently "has_params"; the only two things to not have params are Null and TraitSelect which are not generated through the normal macro infrastructure. I dropped the "fix" for that from this branch for now, though I can re-add it, it's not big (b2b53bb9a4f2f5e1f96f8f01ad7b378f5eb1b4d1).

Ok(DepNode::new_no_params(kind))
} else {
Err(())
}
}
FingerprintStyle::DefPathHash => Ok(DepNode::from_def_path_hash(def_path_hash, kind)),
}
}

Expand All @@ -369,8 +373,8 @@ impl DepNodeExt for DepNode {

impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for () {
#[inline(always)]
fn can_reconstruct_query_key() -> bool {
true
fn fingerprint_style() -> FingerprintStyle {
FingerprintStyle::Unit
}

fn to_fingerprint(&self, _: TyCtxt<'tcx>) -> Fingerprint {
Expand All @@ -384,8 +388,8 @@ impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for () {

impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for DefId {
#[inline(always)]
fn can_reconstruct_query_key() -> bool {
true
fn fingerprint_style() -> FingerprintStyle {
FingerprintStyle::DefPathHash
}

fn to_fingerprint(&self, tcx: TyCtxt<'tcx>) -> Fingerprint {
Expand All @@ -403,8 +407,8 @@ impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for DefId {

impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for LocalDefId {
#[inline(always)]
fn can_reconstruct_query_key() -> bool {
true
fn fingerprint_style() -> FingerprintStyle {
FingerprintStyle::DefPathHash
}

fn to_fingerprint(&self, tcx: TyCtxt<'tcx>) -> Fingerprint {
Expand All @@ -422,8 +426,8 @@ impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for LocalDefId {

impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for CrateNum {
#[inline(always)]
fn can_reconstruct_query_key() -> bool {
true
fn fingerprint_style() -> FingerprintStyle {
FingerprintStyle::DefPathHash
}

fn to_fingerprint(&self, tcx: TyCtxt<'tcx>) -> Fingerprint {
Expand All @@ -442,8 +446,8 @@ impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for CrateNum {

impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for (DefId, DefId) {
#[inline(always)]
fn can_reconstruct_query_key() -> bool {
false
fn fingerprint_style() -> FingerprintStyle {
FingerprintStyle::Opaque
}

// We actually would not need to specialize the implementation of this
Expand All @@ -467,8 +471,8 @@ impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for (DefId, DefId) {

impl<'tcx> DepNodeParams<TyCtxt<'tcx>> for HirId {
#[inline(always)]
fn can_reconstruct_query_key() -> bool {
false
fn fingerprint_style() -> FingerprintStyle {
FingerprintStyle::Opaque
}

// We actually would not need to specialize the implementation of this
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/dep_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ 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)
fn fingerprint_style(&self) -> rustc_query_system::dep_graph::FingerprintStyle {
DepKind::fingerprint_style(self)
}

#[inline(always)]
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_query_impl/src/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,7 @@ macro_rules! define_queries {
use rustc_middle::ty::query::query_keys;
use rustc_query_system::dep_graph::DepNodeParams;
use rustc_query_system::query::{force_query, QueryDescription};
use rustc_query_system::dep_graph::FingerprintStyle;

// We use this for most things when incr. comp. is turned off.
pub const Null: QueryStruct = QueryStruct {
Expand All @@ -454,9 +455,9 @@ macro_rules! define_queries {
const is_anon: bool = is_anon!([$($modifiers)*]);

#[inline(always)]
fn can_reconstruct_query_key() -> bool {
fn fingerprint_style() -> FingerprintStyle {
<query_keys::$name<'_> as DepNodeParams<TyCtxt<'_>>>
::can_reconstruct_query_key()
::fingerprint_style()
}

fn recover<'tcx>(tcx: TyCtxt<'tcx>, dep_node: &DepNode) -> Option<query_keys::$name<'tcx>> {
Expand All @@ -472,7 +473,7 @@ macro_rules! define_queries {
return
}

if !can_reconstruct_query_key() {
if !fingerprint_style().reconstructible() {
return
}

Expand Down
12 changes: 6 additions & 6 deletions compiler/rustc_query_system/src/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
//! `DefId` it was computed from. In other cases, too much information gets
//! lost during fingerprint computation.

use super::{DepContext, DepKind};
use super::{DepContext, DepKind, FingerprintStyle};
use crate::ich::StableHashingContext;

use rustc_data_structures::fingerprint::{Fingerprint, PackedFingerprint};
Expand Down Expand Up @@ -75,7 +75,7 @@ impl<K: DepKind> DepNode<K> {

#[cfg(debug_assertions)]
{
if !kind.can_reconstruct_query_key()
if !kind.fingerprint_style().reconstructible()
&& (tcx.sess().opts.debugging_opts.incremental_info
|| tcx.sess().opts.debugging_opts.query_dep_graph)
{
Expand All @@ -94,7 +94,7 @@ impl<K: DepKind> fmt::Debug for DepNode<K> {
}

pub trait DepNodeParams<Ctxt: DepContext>: fmt::Debug + Sized {
fn can_reconstruct_query_key() -> bool;
fn fingerprint_style() -> FingerprintStyle;

/// This method turns the parameters of a DepNodeConstructor into an opaque
/// Fingerprint to be used in DepNode.
Expand All @@ -111,7 +111,7 @@ pub trait DepNodeParams<Ctxt: DepContext>: fmt::Debug + Sized {
/// This method tries to recover the query key from the given `DepNode`,
/// something which is needed when forcing `DepNode`s during red-green
/// evaluation. The query system will only call this method if
/// `can_reconstruct_query_key()` is `true`.
/// `fingerprint_style()` is not `FingerprintStyle::Opaque`.
/// It is always valid to return `None` here, in which case incremental
/// compilation will treat the query as having changed instead of forcing it.
fn recover(tcx: Ctxt, dep_node: &DepNode<Ctxt::DepKind>) -> Option<Self>;
Expand All @@ -122,8 +122,8 @@ where
T: for<'a> HashStable<StableHashingContext<'a>> + fmt::Debug,
{
#[inline]
default fn can_reconstruct_query_key() -> bool {
false
default fn fingerprint_style() -> FingerprintStyle {
FingerprintStyle::Opaque
}

default fn to_fingerprint(&self, tcx: Ctxt) -> Fingerprint {
Expand Down
23 changes: 22 additions & 1 deletion compiler/rustc_query_system/src/dep_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,27 @@ impl<T: DepContext> HasDepContext for T {
}
}

/// Describes the contents of the fingerprint generated by a given query.
#[derive(PartialEq, Eq, Copy, Clone)]
pub enum FingerprintStyle {
/// The fingerprint is actually a DefPathHash.
DefPathHash,
/// Query key was `()` or equivalent, so fingerprint is just zero.
Unit,
/// Some opaque hash.
Opaque,
}

impl FingerprintStyle {
#[inline]
pub fn reconstructible(self) -> bool {
match self {
FingerprintStyle::DefPathHash | FingerprintStyle::Unit => true,
FingerprintStyle::Opaque => false,
}
}
}

/// Describe the different families of dependency nodes.
pub trait DepKind: Copy + fmt::Debug + Eq + Hash + Send + Encodable<FileEncoder> + 'static {
const NULL: Self;
Expand All @@ -73,5 +94,5 @@ pub trait DepKind: Copy + fmt::Debug + Eq + Hash + Send + Encodable<FileEncoder>
where
OP: for<'a> FnOnce(Option<&'a Lock<TaskDeps<Self>>>);

fn can_reconstruct_query_key(&self) -> bool;
fn fingerprint_style(&self) -> FingerprintStyle;
}
4 changes: 2 additions & 2 deletions compiler/rustc_query_system/src/query/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ where
// We always expect to find a cached result for things that
// can be forced from `DepNode`.
debug_assert!(
!dep_node.kind.can_reconstruct_query_key() || result.is_some(),
!dep_node.kind.fingerprint_style().reconstructible() || result.is_some(),
"missing on-disk cache entry for {:?}",
dep_node
);
Expand Down Expand Up @@ -778,7 +778,7 @@ where
return false;
}

if !<Q::Key as DepNodeParams<CTX::DepContext>>::can_reconstruct_query_key() {
if !<Q::Key as DepNodeParams<CTX::DepContext>>::fingerprint_style().reconstructible() {
return false;
}

Expand Down
12 changes: 12 additions & 0 deletions src/test/run-make/dep-graph/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-include ../../run-make-fulldeps/tools.mk

# ignore-cross-compile

# Just verify that we successfully run and produce dep graphs when requested.

all:
RUST_DEP_GRAPH=$(TMPDIR)/dep-graph $(RUSTC) \
-Cincremental=$(TMPDIR)/incr \
-Zquery-dep-graph -Zdump-dep-graph foo.rs
test -f $(TMPDIR)/dep-graph.txt
test -f $(TMPDIR)/dep-graph.dot
1 change: 1 addition & 0 deletions src/test/run-make/dep-graph/foo.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fn main() {}