From af6800517cfb087b4a54481cedebab8ed22b6c29 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 18 May 2022 17:45:11 +1000 Subject: [PATCH 1/2] Improve fast rejection. In particular, for `bitmaps-3.1.0` which does a huge number of comparisons between `BitsImpl` and `BitsImpl` where `M` and `N` are const integers. --- .../src/traits/select/mod.rs | 133 ++++++++++++++++-- 1 file changed, 118 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index b0b17d0f9e667..b3246bbc3769e 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -33,7 +33,6 @@ use rustc_infer::infer::LateBoundRegionConversionTime; use rustc_middle::dep_graph::{DepKind, DepNodeIndex}; use rustc_middle::mir::interpret::ErrorHandled; use rustc_middle::thir::abstract_const::NotConstEvaluatable; -use rustc_middle::ty::fast_reject::{self, TreatParams}; use rustc_middle::ty::fold::BottomUpFolder; use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::relate::TypeRelation; @@ -214,6 +213,122 @@ enum BuiltinImplConditions<'tcx> { Ambiguous, } +fn tys_may_be_equivalent<'tcx>(oblig_ty: Ty<'tcx>, impl_ty: Ty<'tcx>) -> bool { + let k = impl_ty.kind(); + + // First, look for obvious equivalences, e.g. the types are equal. The most + // common case is to find such an equivalence trivially, e.g. `ty::Bool` + // and `ty::Bool`. + let mut maybe_equiv = match oblig_ty.kind() { + ty::Bool => matches!(k, ty::Bool), + ty::Char => matches!(k, ty::Char), + ty::Int(ity) => matches!(k, ty::Int(ity2) if ity == ity2), + ty::Uint(uty) => matches!(k, ty::Uint(uty2) if uty == uty2), + ty::Float(fty) => matches!(k, ty::Float(fty2) if fty == fty2), + ty::Adt(def_id, substs1) => match k { + ty::Adt(def_id2, substs2) => { + def_id == def_id2 && { + // Check for cases like `Foo<3>` vs `Foo<4>`, which are hot + // in crates such as `bitmaps` and `nalgebra`. + let differ_by_single_const_arg = if + substs1.len() == 1 + && substs2.len() == 1 + && let ty::subst::GenericArgKind::Const(c1) = substs1[0].unpack() + && let ty::subst::GenericArgKind::Const(c2) = substs2[0].unpack() + && let Some(s1) = c1.val().try_to_scalar_int() + && let Some(s2) = c2.val().try_to_scalar_int() + && s1 != s2 + { + true + } else { + false + }; + !differ_by_single_const_arg + } + } + _ => false, + }, + ty::Str => matches!(k, ty::Str), + ty::Array(..) => matches!(k, ty::Array(..)), + ty::Slice(..) => matches!(k, ty::Slice(..)), + ty::RawPtr(ptr) => matches!(k, ty::RawPtr(ptr2) if ptr.mutbl == ptr2.mutbl), + ty::Dynamic(trait_info, ..) => { + matches!(k, ty::Dynamic(trait_info2, ..) if + trait_info.principal_def_id() == trait_info2.principal_def_id() + ) + } + ty::Ref(_, _, mutbl) => matches!(k, ty::Ref(_, _, mutbl2) if mutbl == mutbl2), + ty::FnDef(def_id, _) => matches!(k, ty::FnDef(def_id2, _) if def_id == def_id2), + ty::Closure(def_id, _) => matches!(k, ty::Closure(def_id2, _) if def_id == def_id2), + ty::Generator(def_id, ..) => { + matches!(k, ty::Generator(def_id2, ..) if def_id == def_id2) + } + ty::Never => matches!(k, ty::Never), + ty::Tuple(tys) => matches!(k, ty::Tuple(tys2) if tys.len() == tys2.len()), + ty::FnPtr(tys) => { + matches!( + k, + ty::FnPtr(tys2) + if tys.skip_binder().inputs().len() == tys2.skip_binder().inputs().len() + ) + } + ty::Opaque(def_id, _) => matches!(k, ty::Opaque(def_id2, _) if def_id == def_id2), + ty::Foreign(def_id) => matches!(k, ty::Foreign(def_id2) if def_id == def_id2), + ty::Param(_) => { + // Note, we simplify parameters for the obligation but not the impl + // so that we do not reject a blanket impl but do reject more + // concrete impls if we're searching for `T: Trait`. + matches!(k, ty::Placeholder(..)) + } + ty::Projection(_) => { + // When treating `ty::Param` as a placeholder, projections also + // don't unify with anything else as long as they are fully normalized. + // + // We will have to be careful with lazy normalization here. + oblig_ty.has_infer_types_or_consts() || matches!(k, ty::Placeholder(..)) + } + ty::Infer(_) => true, + + ty::GeneratorWitness(..) | ty::Placeholder(..) | ty::Bound(..) | ty::Error(_) => { + unreachable!() + } + }; + + // Next, if necessary, look for less obvious sources of equivalence. + maybe_equiv = maybe_equiv + || match impl_ty.kind() { + ty::Bool + | ty::Char + | ty::Int(_) + | ty::Uint(_) + | ty::Float(_) + | ty::Adt(..) + | ty::Str + | ty::Array(..) + | ty::Slice(..) + | ty::RawPtr(..) + | ty::Dynamic(..) + | ty::Ref(..) + | ty::Never + | ty::Tuple(..) + | ty::FnPtr(..) + | ty::Foreign(..) => false, + + ty::Param(_) | ty::Projection(_) | ty::Error(_) => true, + + ty::FnDef(..) + | ty::Closure(..) + | ty::Generator(..) + | ty::GeneratorWitness(..) + | ty::Placeholder(..) + | ty::Opaque(..) + | ty::Bound(..) + | ty::Infer(_) => unreachable!(), + }; + + maybe_equiv +} + impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { pub fn new(infcx: &'cx InferCtxt<'cx, 'tcx>) -> SelectionContext<'cx, 'tcx> { SelectionContext { @@ -2142,20 +2257,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { |(obligation_arg, impl_arg)| { match (obligation_arg.unpack(), impl_arg.unpack()) { (GenericArgKind::Type(obligation_ty), GenericArgKind::Type(impl_ty)) => { - // Note, we simplify parameters for the obligation but not the - // impl so that we do not reject a blanket impl but do reject - // more concrete impls if we're searching for `T: Trait`. - let simplified_obligation_ty = fast_reject::simplify_type( - self.tcx(), - obligation_ty, - TreatParams::AsPlaceholder, - ); - let simplified_impl_ty = - fast_reject::simplify_type(self.tcx(), impl_ty, TreatParams::AsInfer); - - simplified_obligation_ty.is_some() - && simplified_impl_ty.is_some() - && simplified_obligation_ty != simplified_impl_ty + let tys_definitely_differ = !tys_may_be_equivalent(obligation_ty, impl_ty); + tys_definitely_differ } (GenericArgKind::Lifetime(_), GenericArgKind::Lifetime(_)) => { // Lifetimes can never cause a rejection. From bf47503052248bf2d27de21cdaa0a8cc1c24d77b Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Mon, 23 May 2022 17:53:57 +1000 Subject: [PATCH 2/2] Move fast reject test out of `SelectionContext::match_impl`. `match_impl` has two call sites. For one of them (within `rematch_impl`) the fast reject test isn't necessary, because any rejection would represent a compiler bug. This commit moves the fast reject test to the other `match_impl` call site, in `assemble_candidates_from_impls`. This lets us move the fast reject test outside the `probe` call in that function. This avoids the taking of useless snapshots when the fast reject test succeeds, which gives a performance win when compiling the `bitmaps` and `nalgebra` crates. --- .../src/traits/select/candidate_assembly.rs | 10 +++++++++- .../rustc_trait_selection/src/traits/select/mod.rs | 13 +++---------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs index 07720ba71ca95..c8b4303e1e0fe 100644 --- a/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs +++ b/compiler/rustc_trait_selection/src/traits/select/candidate_assembly.rs @@ -539,8 +539,16 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { obligation.predicate.def_id(), obligation.predicate.skip_binder().trait_ref.self_ty(), |impl_def_id| { + // Before we create the substitutions and everything, first + // consider a "quick reject". This avoids creating more types + // and so forth that we need to. + let impl_trait_ref = self.tcx().bound_impl_trait_ref(impl_def_id).unwrap(); + if self.fast_reject_trait_refs(obligation, &impl_trait_ref.0) { + return; + } + self.infcx.probe(|_| { - if let Ok(_substs) = self.match_impl(impl_def_id, obligation) { + if let Ok(_substs) = self.match_impl(impl_def_id, impl_trait_ref, obligation) { candidates.vec.push(ImplCandidate(impl_def_id)); } }); diff --git a/compiler/rustc_trait_selection/src/traits/select/mod.rs b/compiler/rustc_trait_selection/src/traits/select/mod.rs index b3246bbc3769e..1ab98e54778af 100644 --- a/compiler/rustc_trait_selection/src/traits/select/mod.rs +++ b/compiler/rustc_trait_selection/src/traits/select/mod.rs @@ -2158,7 +2158,8 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { impl_def_id: DefId, obligation: &TraitObligation<'tcx>, ) -> Normalized<'tcx, SubstsRef<'tcx>> { - match self.match_impl(impl_def_id, obligation) { + let impl_trait_ref = self.tcx().bound_impl_trait_ref(impl_def_id).unwrap(); + match self.match_impl(impl_def_id, impl_trait_ref, obligation) { Ok(substs) => substs, Err(()) => { self.infcx.tcx.sess.delay_span_bug( @@ -2185,17 +2186,9 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> { fn match_impl( &mut self, impl_def_id: DefId, + impl_trait_ref: EarlyBinder>, obligation: &TraitObligation<'tcx>, ) -> Result>, ()> { - let impl_trait_ref = self.tcx().bound_impl_trait_ref(impl_def_id).unwrap(); - - // Before we create the substitutions and everything, first - // consider a "quick reject". This avoids creating more types - // and so forth that we need to. - if self.fast_reject_trait_refs(obligation, &impl_trait_ref.0) { - return Err(()); - } - let placeholder_obligation = self.infcx().replace_bound_vars_with_placeholders(obligation.predicate); let placeholder_obligation_trait_ref = placeholder_obligation.trait_ref;