Skip to content

Commit 1c62cff

Browse files
authored
Rollup merge of rust-lang#131491 - lcnr:nalgebra-perrrrf, r=compiler-errors
impossible obligations fast path fixes the remaining performance regression in nalgebra for rust-lang#130654 r? `@compiler-errors` Fixes rust-lang#124894
2 parents 7eb5621 + d6fd45c commit 1c62cff

File tree

8 files changed

+90
-45
lines changed

8 files changed

+90
-45
lines changed

Diff for: compiler/rustc_next_trait_solver/src/solve/eval_ctxt/mod.rs

+54-18
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ use crate::solve::inspect::{self, ProofTreeBuilder};
1818
use crate::solve::search_graph::SearchGraph;
1919
use crate::solve::{
2020
CanonicalInput, CanonicalResponse, Certainty, FIXPOINT_STEP_LIMIT, Goal, GoalEvaluationKind,
21-
GoalSource, NestedNormalizationGoals, NoSolution, PredefinedOpaquesData, QueryResult,
22-
SolverMode,
21+
GoalSource, HasChanged, NestedNormalizationGoals, NoSolution, PredefinedOpaquesData,
22+
QueryResult, SolverMode,
2323
};
2424

2525
pub(super) mod canonical;
@@ -126,11 +126,31 @@ pub enum GenerateProofTree {
126126
}
127127

128128
pub trait SolverDelegateEvalExt: SolverDelegate {
129+
/// Evaluates a goal from **outside** of the trait solver.
130+
///
131+
/// Using this while inside of the solver is wrong as it uses a new
132+
/// search graph which would break cycle detection.
129133
fn evaluate_root_goal(
130134
&self,
131135
goal: Goal<Self::Interner, <Self::Interner as Interner>::Predicate>,
132136
generate_proof_tree: GenerateProofTree,
133-
) -> (Result<(bool, Certainty), NoSolution>, Option<inspect::GoalEvaluation<Self::Interner>>);
137+
) -> (
138+
Result<(HasChanged, Certainty), NoSolution>,
139+
Option<inspect::GoalEvaluation<Self::Interner>>,
140+
);
141+
142+
/// Check whether evaluating `goal` with a depth of `root_depth` may
143+
/// succeed. This only returns `false` if the goal is guaranteed to
144+
/// not hold. In case evaluation overflows and fails with ambiguity this
145+
/// returns `true`.
146+
///
147+
/// This is only intended to be used as a performance optimization
148+
/// in coherence checking.
149+
fn root_goal_may_hold_with_depth(
150+
&self,
151+
root_depth: usize,
152+
goal: Goal<Self::Interner, <Self::Interner as Interner>::Predicate>,
153+
) -> bool;
134154

135155
// FIXME: This is only exposed because we need to use it in `analyse.rs`
136156
// which is not yet uplifted. Once that's done, we should remove this.
@@ -139,7 +159,7 @@ pub trait SolverDelegateEvalExt: SolverDelegate {
139159
goal: Goal<Self::Interner, <Self::Interner as Interner>::Predicate>,
140160
generate_proof_tree: GenerateProofTree,
141161
) -> (
142-
Result<(NestedNormalizationGoals<Self::Interner>, bool, Certainty), NoSolution>,
162+
Result<(NestedNormalizationGoals<Self::Interner>, HasChanged, Certainty), NoSolution>,
143163
Option<inspect::GoalEvaluation<Self::Interner>>,
144164
);
145165
}
@@ -149,31 +169,41 @@ where
149169
D: SolverDelegate<Interner = I>,
150170
I: Interner,
151171
{
152-
/// Evaluates a goal from **outside** of the trait solver.
153-
///
154-
/// Using this while inside of the solver is wrong as it uses a new
155-
/// search graph which would break cycle detection.
156172
#[instrument(level = "debug", skip(self))]
157173
fn evaluate_root_goal(
158174
&self,
159175
goal: Goal<I, I::Predicate>,
160176
generate_proof_tree: GenerateProofTree,
161-
) -> (Result<(bool, Certainty), NoSolution>, Option<inspect::GoalEvaluation<I>>) {
162-
EvalCtxt::enter_root(self, generate_proof_tree, |ecx| {
177+
) -> (Result<(HasChanged, Certainty), NoSolution>, Option<inspect::GoalEvaluation<I>>) {
178+
EvalCtxt::enter_root(self, self.cx().recursion_limit(), generate_proof_tree, |ecx| {
163179
ecx.evaluate_goal(GoalEvaluationKind::Root, GoalSource::Misc, goal)
164180
})
165181
}
166182

183+
fn root_goal_may_hold_with_depth(
184+
&self,
185+
root_depth: usize,
186+
goal: Goal<Self::Interner, <Self::Interner as Interner>::Predicate>,
187+
) -> bool {
188+
self.probe(|| {
189+
EvalCtxt::enter_root(self, root_depth, GenerateProofTree::No, |ecx| {
190+
ecx.evaluate_goal(GoalEvaluationKind::Root, GoalSource::Misc, goal)
191+
})
192+
.0
193+
})
194+
.is_ok()
195+
}
196+
167197
#[instrument(level = "debug", skip(self))]
168198
fn evaluate_root_goal_raw(
169199
&self,
170200
goal: Goal<I, I::Predicate>,
171201
generate_proof_tree: GenerateProofTree,
172202
) -> (
173-
Result<(NestedNormalizationGoals<I>, bool, Certainty), NoSolution>,
203+
Result<(NestedNormalizationGoals<I>, HasChanged, Certainty), NoSolution>,
174204
Option<inspect::GoalEvaluation<I>>,
175205
) {
176-
EvalCtxt::enter_root(self, generate_proof_tree, |ecx| {
206+
EvalCtxt::enter_root(self, self.cx().recursion_limit(), generate_proof_tree, |ecx| {
177207
ecx.evaluate_goal_raw(GoalEvaluationKind::Root, GoalSource::Misc, goal)
178208
})
179209
}
@@ -197,10 +227,11 @@ where
197227
/// over using this manually (such as [`SolverDelegateEvalExt::evaluate_root_goal`]).
198228
pub(super) fn enter_root<R>(
199229
delegate: &D,
230+
root_depth: usize,
200231
generate_proof_tree: GenerateProofTree,
201232
f: impl FnOnce(&mut EvalCtxt<'_, D>) -> R,
202233
) -> (R, Option<inspect::GoalEvaluation<I>>) {
203-
let mut search_graph = SearchGraph::new(delegate.solver_mode());
234+
let mut search_graph = SearchGraph::new(delegate.solver_mode(), root_depth);
204235

205236
let mut ecx = EvalCtxt {
206237
delegate,
@@ -339,7 +370,7 @@ where
339370
goal_evaluation_kind: GoalEvaluationKind,
340371
source: GoalSource,
341372
goal: Goal<I, I::Predicate>,
342-
) -> Result<(bool, Certainty), NoSolution> {
373+
) -> Result<(HasChanged, Certainty), NoSolution> {
343374
let (normalization_nested_goals, has_changed, certainty) =
344375
self.evaluate_goal_raw(goal_evaluation_kind, source, goal)?;
345376
assert!(normalization_nested_goals.is_empty());
@@ -360,7 +391,7 @@ where
360391
goal_evaluation_kind: GoalEvaluationKind,
361392
_source: GoalSource,
362393
goal: Goal<I, I::Predicate>,
363-
) -> Result<(NestedNormalizationGoals<I>, bool, Certainty), NoSolution> {
394+
) -> Result<(NestedNormalizationGoals<I>, HasChanged, Certainty), NoSolution> {
364395
let (orig_values, canonical_goal) = self.canonicalize_goal(goal);
365396
let mut goal_evaluation =
366397
self.inspect.new_goal_evaluation(goal, &orig_values, goal_evaluation_kind);
@@ -378,8 +409,13 @@ where
378409
Ok(response) => response,
379410
};
380411

381-
let has_changed = !response.value.var_values.is_identity_modulo_regions()
382-
|| !response.value.external_constraints.opaque_types.is_empty();
412+
let has_changed = if !response.value.var_values.is_identity_modulo_regions()
413+
|| !response.value.external_constraints.opaque_types.is_empty()
414+
{
415+
HasChanged::Yes
416+
} else {
417+
HasChanged::No
418+
};
383419

384420
let (normalization_nested_goals, certainty) =
385421
self.instantiate_and_apply_query_response(goal.param_env, orig_values, response);
@@ -552,7 +588,7 @@ where
552588
for (source, goal) in goals.goals {
553589
let (has_changed, certainty) =
554590
self.evaluate_goal(GoalEvaluationKind::Nested, source, goal)?;
555-
if has_changed {
591+
if has_changed == HasChanged::Yes {
556592
unchanged_certainty = None;
557593
}
558594

Diff for: compiler/rustc_next_trait_solver/src/solve/mod.rs

+8
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,14 @@ enum GoalEvaluationKind {
4848
Nested,
4949
}
5050

51+
/// Whether evaluating this goal ended up changing the
52+
/// inference state.
53+
#[derive(PartialEq, Eq, Debug, Hash, Clone, Copy)]
54+
pub enum HasChanged {
55+
Yes,
56+
No,
57+
}
58+
5159
// FIXME(trait-system-refactor-initiative#117): we don't detect whether a response
5260
// ended up pulling down any universes.
5361
fn has_no_inference_or_external_constraints<I: Interner>(

Diff for: compiler/rustc_next_trait_solver/src/solve/search_graph.rs

-3
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,6 @@ where
4040
}
4141

4242
const DIVIDE_AVAILABLE_DEPTH_ON_OVERFLOW: usize = 4;
43-
fn recursion_limit(cx: I) -> usize {
44-
cx.recursion_limit()
45-
}
4643

4744
fn initial_provisional_result(
4845
cx: I,

Diff for: compiler/rustc_trait_selection/src/solve.rs

+1
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ pub mod inspect;
66
mod normalize;
77
mod select;
88

9+
pub(crate) use delegate::SolverDelegate;
910
pub use fulfill::{FulfillmentCtxt, NextSolverError};
1011
pub(crate) use normalize::deeply_normalize_for_diagnostics;
1112
pub use normalize::{deeply_normalize, deeply_normalize_with_skipped_universes};

Diff for: compiler/rustc_trait_selection/src/solve/fulfill.rs

+8-7
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use rustc_infer::traits::{
1212
use rustc_middle::bug;
1313
use rustc_middle::ty::error::{ExpectedFound, TypeError};
1414
use rustc_middle::ty::{self, TyCtxt};
15-
use rustc_next_trait_solver::solve::{GenerateProofTree, SolverDelegateEvalExt as _};
15+
use rustc_next_trait_solver::solve::{GenerateProofTree, HasChanged, SolverDelegateEvalExt as _};
1616
use tracing::instrument;
1717

1818
use super::Certainty;
@@ -86,10 +86,7 @@ impl<'tcx> ObligationStorage<'tcx> {
8686
let result = <&SolverDelegate<'tcx>>::from(infcx)
8787
.evaluate_root_goal(goal, GenerateProofTree::No)
8888
.0;
89-
match result {
90-
Ok((has_changed, _)) => has_changed,
91-
_ => false,
92-
}
89+
matches!(result, Ok((HasChanged::Yes, _)))
9390
}));
9491
})
9592
}
@@ -113,7 +110,7 @@ impl<'tcx, E: 'tcx> FulfillmentCtxt<'tcx, E> {
113110
&self,
114111
infcx: &InferCtxt<'tcx>,
115112
obligation: &PredicateObligation<'tcx>,
116-
result: &Result<(bool, Certainty), NoSolution>,
113+
result: &Result<(HasChanged, Certainty), NoSolution>,
117114
) {
118115
if let Some(inspector) = infcx.obligation_inspector.get() {
119116
let result = match result {
@@ -181,7 +178,11 @@ where
181178
continue;
182179
}
183180
};
184-
has_changed |= changed;
181+
182+
if changed == HasChanged::Yes {
183+
has_changed = true;
184+
}
185+
185186
match certainty {
186187
Certainty::Yes => {}
187188
Certainty::Maybe(_) => self.obligations.register(obligation),

Diff for: compiler/rustc_trait_selection/src/traits/coherence.rs

+12-1
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use rustc_middle::ty::fast_reject::DeepRejectCtxt;
1919
use rustc_middle::ty::visit::{TypeSuperVisitable, TypeVisitable, TypeVisitableExt, TypeVisitor};
2020
use rustc_middle::ty::{self, Ty, TyCtxt};
2121
pub use rustc_next_trait_solver::coherence::*;
22+
use rustc_next_trait_solver::solve::SolverDelegateEvalExt;
2223
use rustc_span::symbol::sym;
2324
use rustc_span::{DUMMY_SP, Span};
2425
use tracing::{debug, instrument, warn};
@@ -28,7 +29,7 @@ use crate::error_reporting::traits::suggest_new_overflow_limit;
2829
use crate::infer::InferOk;
2930
use crate::infer::outlives::env::OutlivesEnvironment;
3031
use crate::solve::inspect::{InspectGoal, ProofTreeInferCtxtExt, ProofTreeVisitor};
31-
use crate::solve::{deeply_normalize_for_diagnostics, inspect};
32+
use crate::solve::{SolverDelegate, deeply_normalize_for_diagnostics, inspect};
3233
use crate::traits::query::evaluate_obligation::InferCtxtExt;
3334
use crate::traits::select::IntercrateAmbiguityCause;
3435
use crate::traits::{
@@ -333,6 +334,16 @@ fn impl_intersection_has_impossible_obligation<'a, 'cx, 'tcx>(
333334
let infcx = selcx.infcx;
334335

335336
if infcx.next_trait_solver() {
337+
// A fast path optimization, try evaluating all goals with
338+
// a very low recursion depth and bail if any of them don't
339+
// hold.
340+
if !obligations.iter().all(|o| {
341+
<&SolverDelegate<'tcx>>::from(infcx)
342+
.root_goal_may_hold_with_depth(8, Goal::new(infcx.tcx, o.param_env, o.predicate))
343+
}) {
344+
return IntersectionHasImpossibleObligations::Yes;
345+
}
346+
336347
let ocx = ObligationCtxt::new_with_diagnostics(infcx);
337348
ocx.register_obligations(obligations.iter().cloned());
338349
let errors_and_ambiguities = ocx.select_all_or_error();

Diff for: compiler/rustc_type_ir/src/search_graph/mod.rs

+7-5
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,6 @@ pub trait Delegate {
8181
fn inspect_is_noop(inspect: &mut Self::ProofTreeBuilder) -> bool;
8282

8383
const DIVIDE_AVAILABLE_DEPTH_ON_OVERFLOW: usize;
84-
fn recursion_limit(cx: Self::Cx) -> usize;
8584

8685
fn initial_provisional_result(
8786
cx: Self::Cx,
@@ -156,7 +155,7 @@ impl AvailableDepth {
156155
/// the remaining depth of all nested goals to prevent hangs
157156
/// in case there is exponential blowup.
158157
fn allowed_depth_for_nested<D: Delegate>(
159-
cx: D::Cx,
158+
root_depth: AvailableDepth,
160159
stack: &IndexVec<StackDepth, StackEntry<D::Cx>>,
161160
) -> Option<AvailableDepth> {
162161
if let Some(last) = stack.raw.last() {
@@ -170,7 +169,7 @@ impl AvailableDepth {
170169
AvailableDepth(last.available_depth.0 - 1)
171170
})
172171
} else {
173-
Some(AvailableDepth(D::recursion_limit(cx)))
172+
Some(root_depth)
174173
}
175174
}
176175

@@ -360,6 +359,7 @@ struct ProvisionalCacheEntry<X: Cx> {
360359

361360
pub struct SearchGraph<D: Delegate<Cx = X>, X: Cx = <D as Delegate>::Cx> {
362361
mode: SolverMode,
362+
root_depth: AvailableDepth,
363363
/// The stack of goals currently being computed.
364364
///
365365
/// An element is *deeper* in the stack if its index is *lower*.
@@ -374,9 +374,10 @@ pub struct SearchGraph<D: Delegate<Cx = X>, X: Cx = <D as Delegate>::Cx> {
374374
}
375375

376376
impl<D: Delegate<Cx = X>, X: Cx> SearchGraph<D> {
377-
pub fn new(mode: SolverMode) -> SearchGraph<D> {
377+
pub fn new(mode: SolverMode, root_depth: usize) -> SearchGraph<D> {
378378
Self {
379379
mode,
380+
root_depth: AvailableDepth(root_depth),
380381
stack: Default::default(),
381382
provisional_cache: Default::default(),
382383
_marker: PhantomData,
@@ -460,7 +461,8 @@ impl<D: Delegate<Cx = X>, X: Cx> SearchGraph<D> {
460461
inspect: &mut D::ProofTreeBuilder,
461462
mut evaluate_goal: impl FnMut(&mut Self, &mut D::ProofTreeBuilder) -> X::Result,
462463
) -> X::Result {
463-
let Some(available_depth) = AvailableDepth::allowed_depth_for_nested::<D>(cx, &self.stack)
464+
let Some(available_depth) =
465+
AvailableDepth::allowed_depth_for_nested::<D>(self.root_depth, &self.stack)
464466
else {
465467
return self.handle_overflow(cx, input, inspect);
466468
};

Diff for: tests/crashes/124894.rs

-11
This file was deleted.

0 commit comments

Comments
 (0)