Skip to content

Commit

Permalink
Auto merge of #126505 - compiler-errors:no-vtable, r=lcnr
Browse files Browse the repository at this point in the history
Only compute vtable information during codegen

This PR removes vtable information from the `Object` and `TraitUpcasting` candidate sources in the trait solvers, and defers the computation of relevant information to `Instance::resolve`. This is because vtables really aren't a thing in the trait world -- they're an implementation detail in codegen.

Previously it was just easiest to tangle this information together since we were already doing the work of looking at all the supertraits in the trait solver, and specifically because we use traits to represent when it's possible to call a method via a vtable (`Object` candidate) and do upcasting (`Unsize` candidate). but I am somewhat suspicious we're doing a *lot* of extra work, especially in polymorphic contexts, so let's see what perf says.
  • Loading branch information
bors committed Jun 16, 2024
2 parents cd0c944 + 3b9adbe commit 5639c21
Show file tree
Hide file tree
Showing 14 changed files with 141 additions and 195 deletions.
3 changes: 1 addition & 2 deletions compiler/rustc_codegen_cranelift/src/unsize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ pub(crate) fn unsized_info<'tcx>(
}

// trait upcasting coercion
let vptr_entry_idx =
fx.tcx.vtable_trait_upcasting_coercion_new_vptr_slot((source, target));
let vptr_entry_idx = fx.tcx.supertrait_vtable_slot((source, target));

if let Some(entry_idx) = vptr_entry_idx {
let entry_idx = u32::try_from(entry_idx).unwrap();
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_codegen_ssa/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,7 @@ pub fn unsized_info<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(

// trait upcasting coercion

let vptr_entry_idx =
cx.tcx().vtable_trait_upcasting_coercion_new_vptr_slot((source, target));
let vptr_entry_idx = cx.tcx().supertrait_vtable_slot((source, target));

if let Some(entry_idx) = vptr_entry_idx {
let ptr_size = bx.data_layout().pointer_size;
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_middle/src/query/keys.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,14 @@ impl<'tcx> Key for (ty::ParamEnv<'tcx>, ty::TraitRef<'tcx>) {
}
}

impl<'tcx> Key for ty::TraitRef<'tcx> {
type Cache<V> = DefaultCache<Self, V>;

fn default_span(&self, tcx: TyCtxt<'_>) -> Span {
tcx.def_span(self.def_id)
}
}

impl<'tcx> Key for ty::PolyTraitRef<'tcx> {
type Cache<V> = DefaultCache<Self, V>;

Expand Down
7 changes: 6 additions & 1 deletion compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ use crate::traits::{
};
use crate::ty::fast_reject::SimplifiedType;
use crate::ty::layout::ValidityRequirement;
use crate::ty::print::PrintTraitRefExt;
use crate::ty::util::AlwaysRequiresDrop;
use crate::ty::TyCtxtFeed;
use crate::ty::{
Expand Down Expand Up @@ -1271,7 +1272,11 @@ rustc_queries! {
desc { |tcx| "finding all vtable entries for trait `{}`", tcx.def_path_str(key.def_id()) }
}

query vtable_trait_upcasting_coercion_new_vptr_slot(key: (Ty<'tcx>, Ty<'tcx>)) -> Option<usize> {
query first_method_vtable_slot(key: ty::TraitRef<'tcx>) -> usize {
desc { |tcx| "finding the slot within the vtable of `{}` for the implementation of `{}`", key.self_ty(), key.print_only_trait_name() }
}

query supertrait_vtable_slot(key: (Ty<'tcx>, Ty<'tcx>)) -> Option<usize> {
desc { |tcx| "finding the slot within vtable for trait object `{}` vtable ptr during trait upcasting coercion from `{}` vtable",
key.1, key.0 }
}
Expand Down
9 changes: 5 additions & 4 deletions compiler/rustc_trait_selection/src/solve/assembly/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use rustc_hir::def_id::DefId;
use rustc_hir::LangItem;
use rustc_infer::infer::InferCtxt;
use rustc_infer::traits::query::NoSolution;
use rustc_infer::traits::util::supertraits;
use rustc_middle::bug;
use rustc_middle::traits::solve::inspect::ProbeKind;
use rustc_middle::traits::solve::{Certainty, Goal, MaybeCause, QueryResult};
Expand Down Expand Up @@ -744,14 +745,14 @@ impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> {
// a projection goal.
if let Some(principal) = bounds.principal() {
let principal_trait_ref = principal.with_self_ty(tcx, self_ty);
self.walk_vtable(principal_trait_ref, |ecx, assumption, vtable_base, _| {
for (idx, assumption) in supertraits(self.interner(), principal_trait_ref).enumerate() {
candidates.extend(G::probe_and_consider_object_bound_candidate(
ecx,
CandidateSource::BuiltinImpl(BuiltinImplSource::Object { vtable_base }),
self,
CandidateSource::BuiltinImpl(BuiltinImplSource::Object(idx)),
goal,
assumption.upcast(tcx),
));
});
}
}
}

Expand Down
36 changes: 0 additions & 36 deletions compiler/rustc_trait_selection/src/solve/eval_ctxt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ use rustc_type_ir_macros::{Lift_Generic, TypeFoldable_Generic, TypeVisitable_Gen
use std::ops::ControlFlow;

use crate::traits::coherence;
use crate::traits::vtable::{count_own_vtable_entries, prepare_vtable_segments, VtblSegment};

use super::inspect::ProofTreeBuilder;
use super::{search_graph, GoalEvaluationKind, FIXPOINT_STEP_LIMIT};
Expand Down Expand Up @@ -1022,41 +1021,6 @@ impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> {
}
}
}

/// Walk through the vtable of a principal trait ref, executing a `supertrait_visitor`
/// for every trait ref encountered (including the principal). Passes both the vtable
/// base and the (optional) vptr slot.
pub(super) fn walk_vtable(
&mut self,
principal: ty::PolyTraitRef<'tcx>,
mut supertrait_visitor: impl FnMut(&mut Self, ty::PolyTraitRef<'tcx>, usize, Option<usize>),
) {
let tcx = self.interner();
let mut offset = 0;
prepare_vtable_segments::<()>(tcx, principal, |segment| {
match segment {
VtblSegment::MetadataDSA => {
offset += TyCtxt::COMMON_VTABLE_ENTRIES.len();
}
VtblSegment::TraitOwnEntries { trait_ref, emit_vptr } => {
let own_vtable_entries = count_own_vtable_entries(tcx, trait_ref);

supertrait_visitor(
self,
trait_ref,
offset,
emit_vptr.then(|| offset + own_vtable_entries),
);

offset += own_vtable_entries;
if emit_vptr {
offset += 1;
}
}
}
ControlFlow::Continue(())
});
}
}

/// Eagerly replace aliases with inference variables, emitting `AliasRelate`
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_trait_selection/src/solve/eval_ctxt/select.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@ fn candidate_should_be_dropped_in_favor_of<'tcx>(
// In the old trait solver, we arbitrarily choose lower vtable candidates
// over higher ones.
(
CandidateSource::BuiltinImpl(BuiltinImplSource::Object { vtable_base: a }),
CandidateSource::BuiltinImpl(BuiltinImplSource::Object { vtable_base: b }),
CandidateSource::BuiltinImpl(BuiltinImplSource::Object(a)),
CandidateSource::BuiltinImpl(BuiltinImplSource::Object(b)),
) => a >= b,
// Prefer dyn candidates over non-dyn candidates. This is necessary to
// handle the unsoundness between `impl<T: ?Sized> Any for T` and `dyn Any: Any`.
Expand Down
32 changes: 14 additions & 18 deletions compiler/rustc_trait_selection/src/solve/trait_goals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use rustc_hir::{LangItem, Movability};
use rustc_infer::infer::InferCtxt;
use rustc_infer::traits::query::NoSolution;
use rustc_infer::traits::solve::MaybeCause;
use rustc_infer::traits::util::supertraits;
use rustc_middle::bug;
use rustc_middle::traits::solve::inspect::ProbeKind;
use rustc_middle::traits::solve::{CandidateSource, Certainty, Goal, QueryResult};
Expand Down Expand Up @@ -756,24 +757,19 @@ impl<'tcx> EvalCtxt<'_, InferCtxt<'tcx>> {
a_data.principal(),
));
} else if let Some(a_principal) = a_data.principal() {
self.walk_vtable(
a_principal.with_self_ty(tcx, a_ty),
|ecx, new_a_principal, _, vtable_vptr_slot| {
responses.extend(ecx.consider_builtin_upcast_to_principal(
goal,
CandidateSource::BuiltinImpl(BuiltinImplSource::TraitUpcasting {
vtable_vptr_slot,
}),
a_data,
a_region,
b_data,
b_region,
Some(new_a_principal.map_bound(|trait_ref| {
ty::ExistentialTraitRef::erase_self_ty(tcx, trait_ref)
})),
));
},
);
for new_a_principal in supertraits(tcx, a_principal.with_self_ty(tcx, a_ty)).skip(1) {
responses.extend(self.consider_builtin_upcast_to_principal(
goal,
CandidateSource::BuiltinImpl(BuiltinImplSource::TraitUpcasting),
a_data,
a_region,
b_data,
b_region,
Some(new_a_principal.map_bound(|trait_ref| {
ty::ExistentialTraitRef::erase_self_ty(tcx, trait_ref)
})),
));
}
}

responses
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_trait_selection/src/traits/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ pub use self::structural_match::search_for_structural_match_violation;
pub use self::structural_normalize::StructurallyNormalizeExt;
pub use self::util::elaborate;
pub use self::util::{expand_trait_aliases, TraitAliasExpander, TraitAliasExpansionInfo};
pub use self::util::{get_vtable_index_of_object_method, impl_item_is_final, upcast_choices};
pub use self::util::{impl_item_is_final, upcast_choices};
pub use self::util::{supertraits, transitive_bounds, transitive_bounds_that_define_assoc_item};
pub use self::util::{with_replaced_escaping_bound_vars, BoundVarReplacer, PlaceholderReplacer};

Expand Down
43 changes: 2 additions & 41 deletions compiler/rustc_trait_selection/src/traits/select/confirmation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ use rustc_span::def_id::DefId;

use crate::traits::normalize::{normalize_with_depth, normalize_with_depth_to};
use crate::traits::util::{self, closure_trait_ref_and_return_type};
use crate::traits::vtable::{
count_own_vtable_entries, prepare_vtable_segments, vtable_trait_first_method_offset,
VtblSegment,
};
use crate::traits::{
ImplDerivedCause, ImplSource, ImplSourceUserDefinedData, Normalized, Obligation,
ObligationCause, PolyTraitObligation, PredicateObligation, Selection, SelectionError,
Expand Down Expand Up @@ -689,13 +685,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {

debug!(?nested, "object nested obligations");

let vtable_base = vtable_trait_first_method_offset(
tcx,
unnormalized_upcast_trait_ref,
ty::Binder::dummy(object_trait_ref),
);

Ok(ImplSource::Builtin(BuiltinImplSource::Object { vtable_base: vtable_base }, nested))
Ok(ImplSource::Builtin(BuiltinImplSource::Object(index), nested))
}

fn confirm_fn_pointer_candidate(
Expand Down Expand Up @@ -1125,36 +1115,7 @@ impl<'cx, 'tcx> SelectionContext<'cx, 'tcx> {
)?
.expect("did not expect ambiguity during confirmation");

let vtable_segment_callback = {
let mut vptr_offset = 0;
move |segment| {
match segment {
VtblSegment::MetadataDSA => {
vptr_offset += TyCtxt::COMMON_VTABLE_ENTRIES.len();
}
VtblSegment::TraitOwnEntries { trait_ref, emit_vptr } => {
vptr_offset += count_own_vtable_entries(tcx, trait_ref);
if trait_ref == unnormalized_upcast_principal {
if emit_vptr {
return ControlFlow::Break(Some(vptr_offset));
} else {
return ControlFlow::Break(None);
}
}

if emit_vptr {
vptr_offset += 1;
}
}
}
ControlFlow::Continue(())
}
};

let vtable_vptr_slot =
prepare_vtable_segments(tcx, source_principal, vtable_segment_callback).unwrap();

Ok(ImplSource::Builtin(BuiltinImplSource::TraitUpcasting { vtable_vptr_slot }, nested))
Ok(ImplSource::Builtin(BuiltinImplSource::TraitUpcasting, nested))
}

fn confirm_builtin_unsize_candidate(
Expand Down
17 changes: 0 additions & 17 deletions compiler/rustc_trait_selection/src/traits/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -208,23 +208,6 @@ pub fn upcast_choices<'tcx>(
supertraits(tcx, source_trait_ref).filter(|r| r.def_id() == target_trait_def_id).collect()
}

/// Given an upcast trait object described by `object`, returns the
/// index of the method `method_def_id` (which should be part of
/// `object.upcast_trait_ref`) within the vtable for `object`.
pub fn get_vtable_index_of_object_method<'tcx>(
tcx: TyCtxt<'tcx>,
vtable_base: usize,
method_def_id: DefId,
) -> Option<usize> {
// Count number of methods preceding the one we are selecting and
// add them to the total offset.
tcx.own_existential_vtable_entries(tcx.parent(method_def_id))
.iter()
.copied()
.position(|def_id| def_id == method_def_id)
.map(|index| vtable_base + index)
}

pub fn closure_trait_ref_and_return_type<'tcx>(
tcx: TyCtxt<'tcx>,
fn_trait_def_id: DefId,
Expand Down
Loading

0 comments on commit 5639c21

Please sign in to comment.