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

add with_hash_task to generate DepNode deterministically #100987

Closed
wants to merge 2 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
8 changes: 7 additions & 1 deletion compiler/rustc_query_system/src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use crate::dep_graph::{DepContext, DepNodeIndex};

use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::sync::Lock;
use rustc_data_structures::sync::{HashMapExt, Lock};

use std::hash::Hash;

Expand Down Expand Up @@ -35,6 +35,12 @@ impl<Key: Eq + Hash, Value: Clone> Cache<Key, Value> {
}
}

impl<Key: Eq + Hash, Value: Clone + Eq> Cache<Key, Value> {
pub fn insert_same(&self, key: Key, dep_node: DepNodeIndex, value: Value) {
self.hashmap.borrow_mut().insert_same(key, WithDepNode::new(dep_node, value));
}
}

#[derive(Clone, Eq, PartialEq)]
pub struct WithDepNode<T> {
dep_node: DepNodeIndex,
Expand Down
33 changes: 27 additions & 6 deletions compiler/rustc_query_system/src/dep_graph/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,7 @@ impl<K: DepKind> DepGraph<K> {

/// Executes something within an "anonymous" task, that is, a task the
/// `DepNode` of which is determined by the list of inputs it read from.
#[inline]
pub fn with_anon_task<Tcx: DepContext<DepKind = K>, OP, R>(
&self,
cx: Tcx,
Expand All @@ -385,6 +386,22 @@ impl<K: DepKind> DepGraph<K> {
) -> (R, DepNodeIndex)
where
OP: FnOnce() -> R,
{
self.with_hash_task(cx, dep_kind, op, None::<()>)
}

/// Executes something within an "anonymous" task. The `hash` is used for
/// generating the `DepNode`.
pub fn with_hash_task<Ctxt: DepContext<DepKind = K>, OP, R, H>(
&self,
cx: Ctxt,
dep_kind: K,
op: OP,
hash: Option<H>,
) -> (R, DepNodeIndex)
where
OP: FnOnce() -> R,
H: Hash,
{
debug_assert!(!cx.is_eval_always(dep_kind));

Expand All @@ -408,13 +425,17 @@ impl<K: DepKind> DepGraph<K> {
task_deps[0]
}
_ => {
// The dep node indices are hashed here instead of hashing the dep nodes of the
// dependencies. These indices may refer to different nodes per session, but this isn't
// a problem here because we that ensure the final dep node hash is per session only by
// combining it with the per session random number `anon_id_seed`. This hash only need
// to map the dependencies to a single value on a per session basis.
let mut hasher = StableHasher::new();
task_deps.hash(&mut hasher);
if let Some(hash) = hash {
hash.hash(&mut hasher);
} else {
// The dep node indices are hashed here instead of hashing the dep nodes of the
// dependencies. These indices may refer to different nodes per session, but this isn't
// a problem here because we that ensure the final dep node hash is per session only by
// combining it with the per session random number `anon_id_seed`. This hash only need
// to map the dependencies to a single value on a per session basis.
task_deps.hash(&mut hasher);
}

let target_dep_node = DepNode {
kind: dep_kind,
Expand Down
29 changes: 21 additions & 8 deletions compiler/rustc_trait_selection/src/traits/select/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@ use rustc_span::symbol::sym;
use std::cell::{Cell, RefCell};
use std::cmp;
use std::fmt::{self, Display};
use std::hash::Hash;
use std::iter;

pub use rustc_middle::traits::select::*;
use rustc_middle::ty::print::with_no_trimmed_paths;

mod candidate_assembly;
mod confirmation;

Expand Down Expand Up @@ -1017,7 +1017,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
return Ok(cycle_result);
}

let (result, dep_node) = self.in_task(|this| this.evaluate_stack(&stack));
let (result, dep_node) = self
.in_task_with_hash(|this| this.evaluate_stack(&stack), &(param_env, fresh_trait_pred));

let result = result?;

if !result.must_apply_modulo_regions() {
Expand Down Expand Up @@ -1263,17 +1265,13 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
if self.can_use_global_caches(param_env) {
if !trait_pred.needs_infer() {
debug!(?trait_pred, ?result, "insert_evaluation_cache global");
// This may overwrite the cache with the same value
// FIXME: Due to #50507 this overwrites the different values
// This should be changed to use HashMapExt::insert_same
// when that is fixed
self.tcx().evaluation_cache.insert((param_env, trait_pred), dep_node, result);
self.tcx().evaluation_cache.insert_same((param_env, trait_pred), dep_node, result);
return;
}
}

debug!(?trait_pred, ?result, "insert_evaluation_cache");
self.infcx.evaluation_cache.insert((param_env, trait_pred), dep_node, result);
self.infcx.evaluation_cache.insert_same((param_env, trait_pred), dep_node, result);
}

/// For various reasons, it's possible for a subobligation
Expand Down Expand Up @@ -1344,6 +1342,21 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
(result, dep_node)
}

fn in_task_with_hash<OP, R, H>(&mut self, op: OP, hash: H) -> (R, DepNodeIndex)
where
OP: FnOnce(&mut Self) -> R,
H: Hash,
{
let (result, dep_node) = self.tcx().dep_graph.with_hash_task(
self.tcx(),
DepKind::TraitSelect,
|| op(self),
Some(hash),
);
self.tcx().dep_graph.read_index(dep_node);
(result, dep_node)
}

/// filter_impls filters constant trait obligations and candidates that have a positive impl
/// for a negative goal and a negative impl for a positive goal
#[instrument(level = "debug", skip(self, candidates))]
Expand Down