From 6117c063063211196e775b131c8b83e2e6d2c126 Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Tue, 4 Apr 2023 14:38:46 +0200 Subject: [PATCH 1/2] incr.comp.: Make sure dependencies are recorded when feeding queries during eval-always queries. --- compiler/rustc_passes/src/hir_id_validator.rs | 54 +++++++++---------- .../rustc_query_system/src/dep_graph/graph.rs | 44 +++++++++++---- .../issue-108481-feed-eval-always.rs | 16 ++++++ 3 files changed, 76 insertions(+), 38 deletions(-) create mode 100644 tests/incremental/issue-108481-feed-eval-always.rs diff --git a/compiler/rustc_passes/src/hir_id_validator.rs b/compiler/rustc_passes/src/hir_id_validator.rs index 9418f3cd322..49aa1da01a2 100644 --- a/compiler/rustc_passes/src/hir_id_validator.rs +++ b/compiler/rustc_passes/src/hir_id_validator.rs @@ -8,34 +8,34 @@ use rustc_middle::hir::nested_filter; use rustc_middle::ty::TyCtxt; pub fn check_crate(tcx: TyCtxt<'_>) { - tcx.dep_graph.assert_ignored(); - - if tcx.sess.opts.unstable_opts.hir_stats { - crate::hir_stats::print_hir_stats(tcx); - } - - #[cfg(debug_assertions)] - { - let errors = Lock::new(Vec::new()); - - tcx.hir().par_for_each_module(|module_id| { - let mut v = HirIdValidator { - tcx, - owner: None, - hir_ids_seen: Default::default(), - errors: &errors, - }; - - tcx.hir().visit_item_likes_in_module(module_id, &mut v); - }); - - let errors = errors.into_inner(); - - if !errors.is_empty() { - let message = errors.iter().fold(String::new(), |s1, s2| s1 + "\n" + s2); - tcx.sess.delay_span_bug(rustc_span::DUMMY_SP, &message); + tcx.dep_graph.with_ignore(|| { + if tcx.sess.opts.unstable_opts.hir_stats { + crate::hir_stats::print_hir_stats(tcx); } - } + + #[cfg(debug_assertions)] + { + let errors = Lock::new(Vec::new()); + + tcx.hir().par_for_each_module(|module_id| { + let mut v = HirIdValidator { + tcx, + owner: None, + hir_ids_seen: Default::default(), + errors: &errors, + }; + + tcx.hir().visit_item_likes_in_module(module_id, &mut v); + }); + + let errors = errors.into_inner(); + + if !errors.is_empty() { + let message = errors.iter().fold(String::new(), |s1, s2| s1 + "\n" + s2); + tcx.sess.delay_span_bug(rustc_span::DUMMY_SP, &message); + } + } + }) } struct HirIdValidator<'a, 'hir> { diff --git a/compiler/rustc_query_system/src/dep_graph/graph.rs b/compiler/rustc_query_system/src/dep_graph/graph.rs index 2ff7de8cb9e..172667e5bdd 100644 --- a/compiler/rustc_query_system/src/dep_graph/graph.rs +++ b/compiler/rustc_query_system/src/dep_graph/graph.rs @@ -143,7 +143,7 @@ impl DepGraph { assert_eq!(_green_node_index, DepNodeIndex::SINGLETON_DEPENDENCYLESS_ANON_NODE); // Instantiate a dependy-less red node only once for anonymous queries. - let (_red_node_index, _prev_and_index) = current.intern_node( + let (red_node_index, red_node_prev_index_and_color) = current.intern_node( profiler, &prev_graph, DepNode { kind: DepKind::RED, hash: Fingerprint::ZERO.into() }, @@ -151,8 +151,21 @@ impl DepGraph { None, false, ); - assert_eq!(_red_node_index, DepNodeIndex::FOREVER_RED_NODE); - assert!(matches!(_prev_and_index, None | Some((_, DepNodeColor::Red)))); + assert_eq!(red_node_index, DepNodeIndex::FOREVER_RED_NODE); + match red_node_prev_index_and_color { + None => { + // This is expected when we have no previous compilation session. + assert!(prev_graph_node_count == 0); + } + Some((prev_red_node_index, DepNodeColor::Red)) => { + assert_eq!(prev_red_node_index.as_usize(), red_node_index.as_usize()); + colors.insert(prev_red_node_index, DepNodeColor::Red); + } + Some((_, DepNodeColor::Green(_))) => { + // There must be a logic error somewhere if we hit this branch. + panic!("DepNodeIndex::FOREVER_RED_NODE evaluated to DepNodeColor::Green") + } + } DepGraph { data: Some(Lrc::new(DepGraphData { @@ -353,10 +366,8 @@ impl DepGraphData { })) }; - let task_deps_ref = match &task_deps { - Some(deps) => TaskDepsRef::Allow(deps), - None => TaskDepsRef::Ignore, - }; + let task_deps_ref = + task_deps.as_ref().map(TaskDepsRef::Allow).unwrap_or(TaskDepsRef::EvalAlways); let result = K::with_deps(task_deps_ref, || task(cx, arg)); let edges = task_deps.map_or_else(|| smallvec![], |lock| lock.into_inner().reads); @@ -461,6 +472,11 @@ impl DepGraph { K::read_deps(|task_deps| { let mut task_deps = match task_deps { TaskDepsRef::Allow(deps) => deps.lock(), + TaskDepsRef::EvalAlways => { + // We don't need to record dependencies of eval_always + // queries. They are re-evaluated unconditionally anyway. + return; + } TaskDepsRef::Ignore => return, TaskDepsRef::Forbid => { panic!("Illegal read of: {dep_node_index:?}") @@ -556,7 +572,10 @@ impl DepGraph { let mut edges = SmallVec::new(); K::read_deps(|task_deps| match task_deps { TaskDepsRef::Allow(deps) => edges.extend(deps.lock().reads.iter().copied()), - TaskDepsRef::Ignore => {} // During HIR lowering, we have no dependencies. + TaskDepsRef::EvalAlways => { + edges.push(DepNodeIndex::FOREVER_RED_NODE); + } + TaskDepsRef::Ignore => {} TaskDepsRef::Forbid => { panic!("Cannot summarize when dependencies are not recorded.") } @@ -1349,10 +1368,13 @@ pub enum TaskDepsRef<'a, K: DepKind> { /// `TaskDeps`. This is used when executing a 'normal' query /// (no `eval_always` modifier) Allow(&'a Lock>), - /// New dependencies are ignored. This is used when - /// executing an `eval_always` query, since there's no + /// This is used when executing an `eval_always` query. We don't /// need to track dependencies for a query that's always - /// re-executed. This is also used for `dep_graph.with_ignore` + /// re-executed -- but we need to know that this is an `eval_always` + /// query in order to emit dependencies to `DepNodeIndex::FOREVER_RED_NODE` + /// when directly feeding other queries. + EvalAlways, + /// New dependencies are ignored. This is also used for `dep_graph.with_ignore`. Ignore, /// Any attempt to add new dependencies will cause a panic. /// This is used when decoding a query result from disk, diff --git a/tests/incremental/issue-108481-feed-eval-always.rs b/tests/incremental/issue-108481-feed-eval-always.rs new file mode 100644 index 00000000000..8f346a7207e --- /dev/null +++ b/tests/incremental/issue-108481-feed-eval-always.rs @@ -0,0 +1,16 @@ +// revisions: cpass1 cpass2 + +#![crate_type = "rlib"] + +use std::fmt::Debug; + +// MCVE kindly provided by Nilstrieb at +// https://github.com/rust-lang/rust/issues/108481#issuecomment-1493080185 + +#[derive(Debug)] +pub struct ConstGeneric { + _p: [(); CHUNK_SIZE], +} + +#[cfg(cpass1)] +impl ConstGeneric {} From 5f52a96b85cf574f083b796d4d61b6d19461694b Mon Sep 17 00:00:00 2001 From: Michael Woerister Date: Wed, 5 Apr 2023 16:21:50 +0200 Subject: [PATCH 2/2] incr.comp.: Don't ignore dep-tracking during HirId validation. --- compiler/rustc_passes/src/hir_id_validator.rs | 52 +++++++++---------- 1 file changed, 25 insertions(+), 27 deletions(-) diff --git a/compiler/rustc_passes/src/hir_id_validator.rs b/compiler/rustc_passes/src/hir_id_validator.rs index 49aa1da01a2..3942a73befd 100644 --- a/compiler/rustc_passes/src/hir_id_validator.rs +++ b/compiler/rustc_passes/src/hir_id_validator.rs @@ -8,34 +8,32 @@ use rustc_middle::hir::nested_filter; use rustc_middle::ty::TyCtxt; pub fn check_crate(tcx: TyCtxt<'_>) { - tcx.dep_graph.with_ignore(|| { - if tcx.sess.opts.unstable_opts.hir_stats { - crate::hir_stats::print_hir_stats(tcx); + if tcx.sess.opts.unstable_opts.hir_stats { + crate::hir_stats::print_hir_stats(tcx); + } + + #[cfg(debug_assertions)] + { + let errors = Lock::new(Vec::new()); + + tcx.hir().par_for_each_module(|module_id| { + let mut v = HirIdValidator { + tcx, + owner: None, + hir_ids_seen: Default::default(), + errors: &errors, + }; + + tcx.hir().visit_item_likes_in_module(module_id, &mut v); + }); + + let errors = errors.into_inner(); + + if !errors.is_empty() { + let message = errors.iter().fold(String::new(), |s1, s2| s1 + "\n" + s2); + tcx.sess.delay_span_bug(rustc_span::DUMMY_SP, &message); } - - #[cfg(debug_assertions)] - { - let errors = Lock::new(Vec::new()); - - tcx.hir().par_for_each_module(|module_id| { - let mut v = HirIdValidator { - tcx, - owner: None, - hir_ids_seen: Default::default(), - errors: &errors, - }; - - tcx.hir().visit_item_likes_in_module(module_id, &mut v); - }); - - let errors = errors.into_inner(); - - if !errors.is_empty() { - let message = errors.iter().fold(String::new(), |s1, s2| s1 + "\n" + s2); - tcx.sess.delay_span_bug(rustc_span::DUMMY_SP, &message); - } - } - }) + } } struct HirIdValidator<'a, 'hir> {