Skip to content

add flag for enabling global cache usage for proof trees and printing proof trees on error #113296

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

Merged
merged 5 commits into from
Jul 5, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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: 8 additions & 0 deletions compiler/rustc_session/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -745,6 +745,14 @@ pub enum TraitSolver {
NextCoherence,
}

#[derive(Default, Debug, Copy, Clone, Hash, PartialEq, Eq)]
pub enum SolverProofTreeCondition {
#[default]
Never,
Always,
OnError,
}

pub enum Input {
/// Load source code from a file.
File(PathBuf),
Expand Down
20 changes: 18 additions & 2 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ mod desc {
"a `,` separated combination of `bti`, `b-key`, `pac-ret`, or `leaf`";
pub const parse_proc_macro_execution_strategy: &str =
"one of supported execution strategies (`same-thread`, or `cross-thread`)";
pub const parse_solver_proof_tree_condition: &str = "one of: `always`, `never`, `on_error`";
}

mod parse {
Expand Down Expand Up @@ -1238,6 +1239,19 @@ mod parse {
};
true
}

pub(crate) fn parse_solver_proof_tree_condition(
slot: &mut SolverProofTreeCondition,
v: Option<&str>,
) -> bool {
match v {
None | Some("always") => *slot = SolverProofTreeCondition::Always,
Some("never") => *slot = SolverProofTreeCondition::Never,
Some("on-error") => *slot = SolverProofTreeCondition::OnError,
_ => return false,
};
true
}
}

options! {
Expand Down Expand Up @@ -1463,8 +1477,10 @@ options! {
"output statistics about monomorphization collection"),
dump_mono_stats_format: DumpMonoStatsFormat = (DumpMonoStatsFormat::Markdown, parse_dump_mono_stats, [UNTRACKED],
"the format to use for -Z dump-mono-stats (`markdown` (default) or `json`)"),
dump_solver_proof_tree: bool = (false, parse_bool, [UNTRACKED],
"dump a proof tree for every goal evaluated by the new trait solver"),
dump_solver_proof_tree: SolverProofTreeCondition = (SolverProofTreeCondition::Never, parse_solver_proof_tree_condition, [UNTRACKED],
"dump a proof tree for every goal evaluated by the new trait solver. The default is `always`"),
dump_solver_proof_tree_uses_cache: Option<bool> = (None, parse_opt_bool, [UNTRACKED],
"determines whether proof tree generation uses the global cache"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"determines whether proof tree generation uses the global cache"),
"determines whether dumping proof trees should disable the global cache"),

this flag should not affect general proof tree generation, only the proof trees which should get dumped

Copy link
Member Author

Choose a reason for hiding this comment

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

that's.... not how the code works. the flag does cause us to use or not use the global cache during proof tree creation. It's not a step done when we dump the proof tree to edit all the "GLOBAL CACHE HIT" to actual proof trees. Implementing it that way sounds trickier than what was done in this PR

Copy link
Member Author

Choose a reason for hiding this comment

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

from the description:

This flag currently would affect misc uses of GenerateProofTree::Yes which will be added in the future for things like diagnostics logic and rustdoc's auto_trait file. We can fix this when we start using proof tree generation for those use cases if it's desirable.

When -Zdump-solver-proof-tree-use-cache=no/yes is set it acts on every call to evaluate_goal with GenerateProofTree::Yes and also to all proof trees generated by -Zdump-solver-proof-tree=always

Copy link
Contributor

Choose a reason for hiding this comment

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

my thought was that setting this to true does/will not prevent diagnostics and auto trait computation from disabling the cache, so this only affects trait solving when we construct a tree for "proof tree dumping"

Copy link
Member Author

Choose a reason for hiding this comment

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

ideally that's how it would work but right now it wouldn't if you implemented auto_trait/diagnostics by callign evalaute_goal with GenerateProofTree::Yes(UseGlobalCache::No).

Copy link
Contributor

Choose a reason for hiding this comment

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

yes, but that would completely ignore this compile flag

dwarf_version: Option<u32> = (None, parse_opt_number, [TRACKED],
"version of DWARF debug information to emit (default: 2 or 4, depending on platform)"),
dylib_lto: bool = (false, parse_bool, [UNTRACKED],
Expand Down
60 changes: 53 additions & 7 deletions compiler/rustc_trait_selection/src/solve/eval_ctxt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@ use rustc_middle::ty::{
self, OpaqueTypeKey, Ty, TyCtxt, TypeFoldable, TypeSuperVisitable, TypeVisitable,
TypeVisitableExt, TypeVisitor,
};
use rustc_session::config::SolverProofTreeCondition;
use rustc_span::DUMMY_SP;
use std::io::Write;
use std::ops::ControlFlow;

use crate::traits::specialization_graph;
Expand Down Expand Up @@ -113,9 +115,23 @@ impl NestedGoals<'_> {

#[derive(PartialEq, Eq, Debug, Hash, HashStable, Clone, Copy)]
pub enum GenerateProofTree {
Yes(DisableGlobalCache),
No,
}

#[derive(PartialEq, Eq, Debug, Hash, HashStable, Clone, Copy)]
pub enum DisableGlobalCache {
Yes,
No,
}
impl DisableGlobalCache {
pub fn from_bool(disable_cache: bool) -> Self {
match disable_cache {
true => DisableGlobalCache::Yes,
false => DisableGlobalCache::No,
}
}
}

pub trait InferCtxtEvalExt<'tcx> {
/// Evaluates a goal from **outside** of the trait solver.
Expand Down Expand Up @@ -164,6 +180,36 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
let mode = if infcx.intercrate { SolverMode::Coherence } else { SolverMode::Normal };
let mut search_graph = search_graph::SearchGraph::new(infcx.tcx, mode);

let inspect = {
let generate_proof_tree = match (
infcx.tcx.sess.opts.unstable_opts.dump_solver_proof_tree,
infcx.tcx.sess.opts.unstable_opts.dump_solver_proof_tree_uses_cache,
generate_proof_tree,
) {
(_, Some(use_cache), GenerateProofTree::Yes(_)) => {
GenerateProofTree::Yes(DisableGlobalCache::from_bool(!use_cache))
}

(SolverProofTreeCondition::Always, use_cache, GenerateProofTree::No) => {
let use_cache = use_cache.unwrap_or(true);
GenerateProofTree::Yes(DisableGlobalCache::from_bool(!use_cache))
}

(_, None, GenerateProofTree::Yes(_)) => generate_proof_tree,
// `Never` is kind of weird- it doesn't actually force us to not generate proof trees
// its just the default setting for rustflags forced proof tree generation.
(SolverProofTreeCondition::Never, _, _) => generate_proof_tree,
(SolverProofTreeCondition::OnError, _, _) => generate_proof_tree,
};

match generate_proof_tree {
GenerateProofTree::No => ProofTreeBuilder::new_noop(),
GenerateProofTree::Yes(global_cache_disabled) => {
ProofTreeBuilder::new_root(global_cache_disabled)
}
}
};

let mut ecx = EvalCtxt {
search_graph: &mut search_graph,
infcx: infcx,
Expand All @@ -177,17 +223,17 @@ impl<'a, 'tcx> EvalCtxt<'a, 'tcx> {
var_values: CanonicalVarValues::dummy(),
nested_goals: NestedGoals::new(),
tainted: Ok(()),
inspect: (infcx.tcx.sess.opts.unstable_opts.dump_solver_proof_tree
|| matches!(generate_proof_tree, GenerateProofTree::Yes))
.then(ProofTreeBuilder::new_root)
.unwrap_or_else(ProofTreeBuilder::new_noop),
inspect,
};
let result = f(&mut ecx);

let tree = ecx.inspect.finalize();
if let Some(tree) = &tree {
// module to allow more granular RUSTC_LOG filtering to just proof tree output
super::inspect::dump::print_tree(tree);
if let (Some(tree), SolverProofTreeCondition::Always) =
(&tree, infcx.tcx.sess.opts.unstable_opts.dump_solver_proof_tree)
{
let mut lock = std::io::stdout().lock();
let _ = lock.write_fmt(format_args!("{tree:?}"));
let _ = lock.flush();
}

assert!(
Expand Down
92 changes: 61 additions & 31 deletions compiler/rustc_trait_selection/src/solve/inspect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rustc_middle::traits::solve::{
};
use rustc_middle::ty;

pub mod dump;
use super::eval_ctxt::DisableGlobalCache;

#[derive(Eq, PartialEq, Debug, Hash, HashStable)]
pub struct WipGoalEvaluation<'tcx> {
Expand Down Expand Up @@ -145,11 +145,15 @@ impl<'tcx> From<WipGoalCandidate<'tcx>> for DebugSolver<'tcx> {

pub struct ProofTreeBuilder<'tcx> {
state: Option<Box<DebugSolver<'tcx>>>,
disable_global_cache: DisableGlobalCache,
}

impl<'tcx> ProofTreeBuilder<'tcx> {
fn new(state: impl Into<DebugSolver<'tcx>>) -> ProofTreeBuilder<'tcx> {
ProofTreeBuilder { state: Some(Box::new(state.into())) }
fn new(
state: impl Into<DebugSolver<'tcx>>,
disable_global_cache: DisableGlobalCache,
) -> ProofTreeBuilder<'tcx> {
ProofTreeBuilder { state: Some(Box::new(state.into())), disable_global_cache }
}

fn as_mut(&mut self) -> Option<&mut DebugSolver<'tcx>> {
Expand All @@ -165,12 +169,16 @@ impl<'tcx> ProofTreeBuilder<'tcx> {
}
}

pub fn new_root() -> ProofTreeBuilder<'tcx> {
ProofTreeBuilder::new(DebugSolver::Root)
pub fn disable_global_cache(&self) -> DisableGlobalCache {
self.disable_global_cache
}

pub fn new_root(disable_global_cache: DisableGlobalCache) -> ProofTreeBuilder<'tcx> {
ProofTreeBuilder::new(DebugSolver::Root, disable_global_cache)
}

pub fn new_noop() -> ProofTreeBuilder<'tcx> {
ProofTreeBuilder { state: None }
ProofTreeBuilder { state: None, disable_global_cache: DisableGlobalCache::No }
}

pub fn is_noop(&self) -> bool {
Expand All @@ -183,18 +191,24 @@ impl<'tcx> ProofTreeBuilder<'tcx> {
is_normalizes_to_hack: IsNormalizesToHack,
) -> ProofTreeBuilder<'tcx> {
if self.state.is_none() {
return ProofTreeBuilder { state: None };
return ProofTreeBuilder {
state: None,
disable_global_cache: self.disable_global_cache,
};
}

ProofTreeBuilder::new(WipGoalEvaluation {
uncanonicalized_goal: goal,
canonicalized_goal: None,
evaluation_steps: vec![],
is_normalizes_to_hack,
cache_hit: None,
returned_goals: vec![],
result: None,
})
ProofTreeBuilder::new(
WipGoalEvaluation {
uncanonicalized_goal: goal,
canonicalized_goal: None,
evaluation_steps: vec![],
is_normalizes_to_hack,
cache_hit: None,
returned_goals: vec![],
result: None,
},
self.disable_global_cache,
)
}

pub fn canonicalized_goal(&mut self, canonical_goal: CanonicalInput<'tcx>) {
Expand Down Expand Up @@ -250,15 +264,21 @@ impl<'tcx> ProofTreeBuilder<'tcx> {
instantiated_goal: QueryInput<'tcx, ty::Predicate<'tcx>>,
) -> ProofTreeBuilder<'tcx> {
if self.state.is_none() {
return ProofTreeBuilder { state: None };
return ProofTreeBuilder {
state: None,
disable_global_cache: self.disable_global_cache,
};
}

ProofTreeBuilder::new(WipGoalEvaluationStep {
instantiated_goal,
nested_goal_evaluations: vec![],
candidates: vec![],
result: None,
})
ProofTreeBuilder::new(
WipGoalEvaluationStep {
instantiated_goal,
nested_goal_evaluations: vec![],
candidates: vec![],
result: None,
},
self.disable_global_cache,
)
}
pub fn goal_evaluation_step(&mut self, goal_eval_step: ProofTreeBuilder<'tcx>) {
if let Some(this) = self.as_mut() {
Expand All @@ -273,14 +293,17 @@ impl<'tcx> ProofTreeBuilder<'tcx> {

pub fn new_goal_candidate(&mut self) -> ProofTreeBuilder<'tcx> {
if self.state.is_none() {
return ProofTreeBuilder { state: None };
return ProofTreeBuilder {
state: None,

disable_global_cache: self.disable_global_cache,
};
}

ProofTreeBuilder::new(WipGoalCandidate {
nested_goal_evaluations: vec![],
candidates: vec![],
kind: None,
})
ProofTreeBuilder::new(
WipGoalCandidate { nested_goal_evaluations: vec![], candidates: vec![], kind: None },
self.disable_global_cache,
)
}

pub fn candidate_kind(&mut self, candidate_kind: CandidateKind<'tcx>) {
Expand Down Expand Up @@ -309,10 +332,17 @@ impl<'tcx> ProofTreeBuilder<'tcx> {

pub fn new_evaluate_added_goals(&mut self) -> ProofTreeBuilder<'tcx> {
if self.state.is_none() {
return ProofTreeBuilder { state: None };
return ProofTreeBuilder {
state: None,

disable_global_cache: self.disable_global_cache,
};
}

ProofTreeBuilder::new(WipAddedGoalsEvaluation { evaluations: vec![], result: None })
ProofTreeBuilder::new(
WipAddedGoalsEvaluation { evaluations: vec![], result: None },
self.disable_global_cache,
)
}

pub fn evaluate_added_goals_loop_start(&mut self) {
Expand Down
5 changes: 0 additions & 5 deletions compiler/rustc_trait_selection/src/solve/inspect/dump.rs

This file was deleted.

4 changes: 3 additions & 1 deletion compiler/rustc_trait_selection/src/solve/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ mod search_graph;
mod trait_goals;
mod weak_types;

pub use eval_ctxt::{EvalCtxt, InferCtxtEvalExt, InferCtxtSelectExt};
pub use eval_ctxt::{
DisableGlobalCache, EvalCtxt, GenerateProofTree, InferCtxtEvalExt, InferCtxtSelectExt,
};
pub use fulfill::FulfillmentCtxt;
pub(crate) use normalize::deeply_normalize;

Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_trait_selection/src/solve/search_graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use rustc_middle::traits::solve::{CanonicalInput, Certainty, MaybeCause, QueryRe
use rustc_middle::ty::TyCtxt;
use std::{collections::hash_map::Entry, mem};

use super::eval_ctxt::DisableGlobalCache;
use super::inspect::ProofTreeBuilder;
use super::SolverMode;

Expand Down Expand Up @@ -213,7 +214,9 @@ impl<'tcx> SearchGraph<'tcx> {
inspect: &mut ProofTreeBuilder<'tcx>,
mut loop_body: impl FnMut(&mut Self, &mut ProofTreeBuilder<'tcx>) -> QueryResult<'tcx>,
) -> QueryResult<'tcx> {
if self.should_use_global_cache() {
if self.should_use_global_cache()
&& inspect.disable_global_cache() == DisableGlobalCache::No
{
if let Some(result) = tcx.new_solver_evaluation_cache.get(&canonical_input, tcx) {
debug!(?canonical_input, ?result, "cache hit");
inspect.cache_hit(CacheHit::Global);
Expand Down
Loading