Skip to content

Commit

Permalink
Auto merge of #93505 - lcnr:substsref-vs-ty-list, r=michaelwoerister
Browse files Browse the repository at this point in the history
safely `transmute<&List<Ty<'tcx>>, &List<GenericArg<'tcx>>>`

This PR has 3 relevant steps which are is split in distinct commits.

The first commit now interns `List<Ty<'tcx>>` and `List<GenericArg<'tcx>>` together, potentially reusing memory while allowing free conversions between these two using `List<Ty<'tcx>>::as_substs()` and `SubstsRef<'tcx>::try_as_type_list()`.

Using this, we then use `&'tcx List<Ty<'tcx>>` instead of a `SubstsRef<'tcx>` for tuple fields, simplifying a bunch of code.

Finally, as tuple fields and other generic arguments now use a different `TypeFoldable<'tcx>` impl, we optimize the impl for `List<Ty<'tcx>>` improving perf by slightly less than 1% in tuple heavy benchmarks.
  • Loading branch information
bors committed Feb 21, 2022
2 parents 1103d2e + 80f56cd commit 03a8cc7
Show file tree
Hide file tree
Showing 64 changed files with 289 additions and 210 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2298,7 +2298,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {
// Closure arguments are wrapped in a tuple, so we need to get the first
// from that.
if let ty::Tuple(elems) = argument_ty.kind() {
let argument_ty = elems.first()?.expect_ty();
let &argument_ty = elems.first()?;
if let ty::Ref(_, _, _) = argument_ty.kind() {
return Some(AnnotatedBorrowFnSignature::Closure {
argument_ty,
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_borrowck/src/diagnostics/region_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> {
let search_stack: &mut Vec<(Ty<'tcx>, &hir::Ty<'_>)> = &mut vec![(ty, hir_ty)];

while let Some((ty, hir_ty)) = search_stack.pop() {
match (&ty.kind(), &hir_ty.kind) {
match (ty.kind(), &hir_ty.kind) {
// Check if the `ty` is `&'X ..` where `'X`
// is the region we are looking for -- if so, and we have a `&T`
// on the RHS, then we want to highlight the `&` like so:
Expand Down Expand Up @@ -532,9 +532,8 @@ impl<'tcx> MirBorrowckCtxt<'_, 'tcx> {
// The following cases don't have lifetimes, so we
// just worry about trying to match up the rustc type
// with the HIR types:
(ty::Tuple(elem_tys), hir::TyKind::Tup(elem_hir_tys)) => {
search_stack
.extend(iter::zip(elem_tys.iter().map(|k| k.expect_ty()), *elem_hir_tys));
(&ty::Tuple(elem_tys), hir::TyKind::Tup(elem_hir_tys)) => {
search_stack.extend(iter::zip(elem_tys, *elem_hir_tys));
}

(ty::Slice(elem_ty), hir::TyKind::Slice(elem_hir_ty))
Expand Down
7 changes: 4 additions & 3 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -832,9 +832,10 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> {
return match substs
.as_closure()
.tupled_upvars_ty()
.tuple_element_ty(field.index())
.tuple_fields()
.get(field.index())
{
Some(ty) => Ok(ty),
Some(&ty) => Ok(ty),
None => Err(FieldAccessError::OutOfRange {
field_count: substs.as_closure().upvar_tys().count(),
}),
Expand All @@ -852,7 +853,7 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> {
}
ty::Tuple(tys) => {
return match tys.get(field.index()) {
Some(&ty) => Ok(ty.expect_ty()),
Some(&ty) => Ok(ty),
None => Err(FieldAccessError::OutOfRange { field_count: tys.len() }),
};
}
Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_borrowck/src/universal_regions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -641,15 +641,13 @@ impl<'cx, 'tcx> UniversalRegionsBuilder<'cx, 'tcx> {
let (&output, tuplized_inputs) =
inputs_and_output.skip_binder().split_last().unwrap();
assert_eq!(tuplized_inputs.len(), 1, "multiple closure inputs");
let ty::Tuple(inputs) = tuplized_inputs[0].kind() else {
let &ty::Tuple(inputs) = tuplized_inputs[0].kind() else {
bug!("closure inputs not a tuple: {:?}", tuplized_inputs[0]);
};

ty::Binder::bind_with_vars(
tcx.mk_type_list(
iter::once(closure_ty)
.chain(inputs.iter().map(|k| k.expect_ty()))
.chain(iter::once(output)),
iter::once(closure_ty).chain(inputs).chain(iter::once(output)),
),
bound_vars,
)
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_codegen_cranelift/src/abi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ impl<'tcx> FunctionCx<'_, '_, 'tcx> {
.unzip();
let return_layout = self.layout_of(return_ty);
let return_tys = if let ty::Tuple(tup) = return_ty.kind() {
tup.types().map(|ty| AbiParam::new(self.clif_type(ty).unwrap())).collect()
tup.iter().map(|ty| AbiParam::new(self.clif_type(ty).unwrap())).collect()
} else {
vec![AbiParam::new(self.clif_type(return_ty).unwrap())]
};
Expand Down Expand Up @@ -199,7 +199,7 @@ pub(crate) fn codegen_fn_prelude<'tcx>(fx: &mut FunctionCx<'_, '_, 'tcx>, start_
};

let mut params = Vec::new();
for (i, _arg_ty) in tupled_arg_tys.types().enumerate() {
for (i, _arg_ty) in tupled_arg_tys.iter().enumerate() {
let arg_abi = arg_abis_iter.next().unwrap();
let param =
cvalue_for_param(fx, Some(local), Some(i), arg_abi, &mut block_params_iter);
Expand Down
7 changes: 3 additions & 4 deletions compiler/rustc_codegen_cranelift/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,9 @@ fn clif_pair_type_from_ty<'tcx>(
ty: Ty<'tcx>,
) -> Option<(types::Type, types::Type)> {
Some(match ty.kind() {
ty::Tuple(substs) if substs.len() == 2 => {
let mut types = substs.types();
let a = clif_type_from_ty(tcx, types.next().unwrap())?;
let b = clif_type_from_ty(tcx, types.next().unwrap())?;
ty::Tuple(types) if types.len() == 2 => {
let a = clif_type_from_ty(tcx, types[0])?;
let b = clif_type_from_ty(tcx, types[1])?;
if a.is_vector() || b.is_vector() {
return None;
}
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_codegen_llvm/src/debuginfo/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -752,9 +752,8 @@ pub fn type_metadata<'ll, 'tcx>(cx: &CodegenCx<'ll, 'tcx>, t: Ty<'tcx>) -> &'ll
prepare_enum_metadata(cx, t, def.did, unique_type_id, vec![]).finalize(cx)
}
},
ty::Tuple(elements) => {
let tys: Vec<_> = elements.iter().map(|k| k.expect_ty()).collect();
prepare_tuple_metadata(cx, t, &tys, unique_type_id, NO_SCOPE_METADATA).finalize(cx)
ty::Tuple(tys) => {
prepare_tuple_metadata(cx, t, tys, unique_type_id, NO_SCOPE_METADATA).finalize(cx)
}
// Type parameters from polymorphized functions.
ty::Param(_) => MetadataCreationResult::new(param_type_metadata(cx, t), false),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ fn push_debuginfo_type_name<'tcx>(
}

for component_type in component_types {
push_debuginfo_type_name(tcx, component_type.expect_ty(), true, output, visited);
push_debuginfo_type_name(tcx, component_type, true, output, visited);
push_arg_separator(cpp_like_debuginfo, output);
}
if !component_types.is_empty() {
Expand Down
15 changes: 7 additions & 8 deletions compiler/rustc_infer/src/infer/error_reporting/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ use rustc_middle::ty::{
self,
error::TypeError,
subst::{GenericArgKind, Subst, SubstsRef},
Binder, Region, Ty, TyCtxt, TypeFoldable,
Binder, List, Region, Ty, TyCtxt, TypeFoldable,
};
use rustc_span::{sym, BytePos, DesugaringKind, MultiSpan, Pos, Span};
use rustc_target::spec::abi;
Expand Down Expand Up @@ -1361,7 +1361,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
let mut values =
(DiagnosticStyledString::normal("("), DiagnosticStyledString::normal("("));
let len = substs1.len();
for (i, (left, right)) in substs1.types().zip(substs2.types()).enumerate() {
for (i, (left, right)) in substs1.iter().zip(substs2).enumerate() {
let (x1, x2) = self.cmp(left, right);
(values.0).0.extend(x1.0);
(values.1).0.extend(x2.0);
Expand Down Expand Up @@ -2042,8 +2042,8 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
// If a tuple of length one was expected and the found expression has
// parentheses around it, perhaps the user meant to write `(expr,)` to
// build a tuple (issue #86100)
(ty::Tuple(_), _) => {
self.emit_tuple_wrap_err(&mut err, span, found, expected)
(ty::Tuple(fields), _) => {
self.emit_tuple_wrap_err(&mut err, span, found, fields)
}
// If a character was expected and the found expression is a string literal
// containing a single character, perhaps the user meant to write `'c'` to
Expand Down Expand Up @@ -2111,12 +2111,11 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> {
err: &mut DiagnosticBuilder<'tcx>,
span: Span,
found: Ty<'tcx>,
expected: Ty<'tcx>,
expected_fields: &List<Ty<'tcx>>,
) {
let [expected_tup_elem] = &expected.tuple_fields().collect::<Vec<_>>()[..]
else { return };
let [expected_tup_elem] = expected_fields[..] else { return };

if !same_type_modulo_infer(*expected_tup_elem, found) {
if !same_type_modulo_infer(expected_tup_elem, found) {
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,9 @@ fn closure_args(fn_sig: &ty::PolyFnSig<'_>) -> String {
.skip_binder()
.iter()
.next()
.map(|args| args.tuple_fields().map(|arg| arg.to_string()).collect::<Vec<_>>().join(", "))
.map(|args| {
args.tuple_fields().iter().map(|arg| arg.to_string()).collect::<Vec<_>>().join(", ")
})
.unwrap_or_default()
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_lint/src/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2621,7 +2621,7 @@ impl<'tcx> LateLintPass<'tcx> for InvalidValue {
}
Tuple(..) => {
// Proceed recursively, check all fields.
ty.tuple_fields().find_map(|field| ty_find_init_error(tcx, field, init))
ty.tuple_fields().iter().find_map(|field| ty_find_init_error(tcx, field, init))
}
// Conservative fallback.
_ => None,
Expand Down Expand Up @@ -2934,7 +2934,7 @@ impl ClashingExternDeclarations {
)
}
(Tuple(a_substs), Tuple(b_substs)) => {
a_substs.types().eq_by(b_substs.types(), |a_ty, b_ty| {
a_substs.iter().eq_by(b_substs.iter(), |a_ty, b_ty| {
structurally_same_type_impl(seen_types, cx, a_ty, b_ty, ckind)
})
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_lint/src/unused.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ impl<'tcx> LateLintPass<'tcx> for UnusedResults {
} else {
vec![]
};
for (i, ty) in tys.iter().map(|k| k.expect_ty()).enumerate() {
for (i, ty) in tys.iter().enumerate() {
let descr_post = &format!(" in tuple element {}", i);
let span = *spans.get(i).unwrap_or(&span);
if check_must_use_ty(cx, ty, expr, span, descr_pre, descr_post, plural_len)
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ fn use_verbose<'tcx>(ty: Ty<'tcx>, fn_def: bool) -> bool {
ty::Int(_) | ty::Uint(_) | ty::Bool | ty::Char | ty::Float(_) => false,
// Unit type
ty::Tuple(g_args) if g_args.is_empty() => false,
ty::Tuple(g_args) => g_args.iter().any(|g_arg| use_verbose(g_arg.expect_ty(), fn_def)),
ty::Tuple(g_args) => g_args.iter().any(|g_arg| use_verbose(g_arg, fn_def)),
ty::Array(ty, _) => use_verbose(ty, fn_def),
ty::FnDef(..) => fn_def,
_ => true,
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_middle/src/mir/tcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl<'tcx> PlaceTy<'tcx> {
let field_def = &variant_def.fields[f.index()];
field_def.ty(tcx, substs)
}
ty::Tuple(ref tys) => tys[f.index()].expect_ty(),
ty::Tuple(tys) => tys[f.index()],
_ => bug!("extracting field of non-tuple non-adt: {:?}", self),
};
debug!("field_ty self: {:?} f: {:?} yields: {:?}", self, f, answer);
Expand Down
49 changes: 37 additions & 12 deletions compiler/rustc_middle/src/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ pub struct CtxtInterners<'tcx> {
// Specifically use a speedy hash algorithm for these hash sets, since
// they're accessed quite often.
type_: InternedSet<'tcx, TyS<'tcx>>,
type_list: InternedSet<'tcx, List<Ty<'tcx>>>,
substs: InternedSet<'tcx, InternalSubsts<'tcx>>,
canonical_var_infos: InternedSet<'tcx, List<CanonicalVarInfo<'tcx>>>,
region: InternedSet<'tcx, RegionKind>,
Expand Down Expand Up @@ -129,7 +128,6 @@ impl<'tcx> CtxtInterners<'tcx> {
CtxtInterners {
arena,
type_: Default::default(),
type_list: Default::default(),
substs: Default::default(),
region: Default::default(),
poly_existential_predicates: Default::default(),
Expand Down Expand Up @@ -1657,6 +1655,8 @@ macro_rules! nop_lift {
type Lifted = $lifted;
fn lift_to_tcx(self, tcx: TyCtxt<'tcx>) -> Option<Self::Lifted> {
if tcx.interners.$set.contains_pointer_to(&InternedInSet(self.0.0)) {
// SAFETY: `self` is interned and therefore valid
// for the entire lifetime of the `TyCtxt`.
Some(unsafe { mem::transmute(self) })
} else {
None
Expand All @@ -1666,6 +1666,25 @@ macro_rules! nop_lift {
};
}

// Can't use the macros as we have reuse the `substs` here.
//
// See `intern_type_list` for more info.
impl<'a, 'tcx> Lift<'tcx> for &'a List<Ty<'a>> {
type Lifted = &'tcx List<Ty<'tcx>>;
fn lift_to_tcx(self, tcx: TyCtxt<'tcx>) -> Option<Self::Lifted> {
if self.is_empty() {
return Some(List::empty());
}
if tcx.interners.substs.contains_pointer_to(&InternedInSet(self.as_substs())) {
// SAFETY: `self` is interned and therefore valid
// for the entire lifetime of the `TyCtxt`.
Some(unsafe { mem::transmute::<&'a List<Ty<'a>>, &'tcx List<Ty<'tcx>>>(self) })
} else {
None
}
}
}

macro_rules! nop_list_lift {
($set:ident; $ty:ty => $lifted:ty) => {
impl<'a, 'tcx> Lift<'tcx> for &'a List<$ty> {
Expand All @@ -1690,7 +1709,6 @@ nop_lift! {const_; Const<'a> => Const<'tcx>}
nop_lift_old! {const_allocation; &'a Allocation => &'tcx Allocation}
nop_lift! {predicate; Predicate<'a> => Predicate<'tcx>}

nop_list_lift! {type_list; Ty<'a> => Ty<'tcx>}
nop_list_lift! {poly_existential_predicates; ty::Binder<'a, ExistentialPredicate<'a>> => ty::Binder<'tcx, ExistentialPredicate<'tcx>>}
nop_list_lift! {predicates; Predicate<'a> => Predicate<'tcx>}
nop_list_lift! {canonical_var_infos; CanonicalVarInfo<'a> => CanonicalVarInfo<'tcx>}
Expand Down Expand Up @@ -2189,7 +2207,6 @@ macro_rules! slice_interners {
}

slice_interners!(
type_list: _intern_type_list(Ty<'tcx>),
substs: _intern_substs(GenericArg<'tcx>),
canonical_var_infos: _intern_canonical_var_infos(CanonicalVarInfo<'tcx>),
poly_existential_predicates:
Expand Down Expand Up @@ -2259,7 +2276,7 @@ impl<'tcx> TyCtxt<'tcx> {
) -> PolyFnSig<'tcx> {
sig.map_bound(|s| {
let params_iter = match s.inputs()[0].kind() {
ty::Tuple(params) => params.into_iter().map(|k| k.expect_ty()),
ty::Tuple(params) => params.into_iter(),
_ => bug!(),
};
self.mk_fn_sig(params_iter, s.output(), s.c_variadic, unsafety, abi::Abi::Rust)
Expand Down Expand Up @@ -2421,15 +2438,11 @@ impl<'tcx> TyCtxt<'tcx> {

#[inline]
pub fn intern_tup(self, ts: &[Ty<'tcx>]) -> Ty<'tcx> {
let kinds: Vec<_> = ts.iter().map(|&t| GenericArg::from(t)).collect();
self.mk_ty(Tuple(self.intern_substs(&kinds)))
self.mk_ty(Tuple(self.intern_type_list(&ts)))
}

pub fn mk_tup<I: InternAs<[Ty<'tcx>], Ty<'tcx>>>(self, iter: I) -> I::Output {
iter.intern_with(|ts| {
let kinds: Vec<_> = ts.iter().map(|&t| GenericArg::from(t)).collect();
self.mk_ty(Tuple(self.intern_substs(&kinds)))
})
iter.intern_with(|ts| self.mk_ty(Tuple(self.intern_type_list(&ts))))
}

#[inline]
Expand Down Expand Up @@ -2611,7 +2624,19 @@ impl<'tcx> TyCtxt<'tcx> {
}

pub fn intern_type_list(self, ts: &[Ty<'tcx>]) -> &'tcx List<Ty<'tcx>> {
if ts.is_empty() { List::empty() } else { self._intern_type_list(ts) }
if ts.is_empty() {
List::empty()
} else {
// Actually intern type lists as lists of `GenericArg`s.
//
// Transmuting from `Ty<'tcx>` to `GenericArg<'tcx>` is sound
// as explained in ty_slice_as_generic_arg`. With this,
// we guarantee that even when transmuting between `List<Ty<'tcx>>`
// and `List<GenericArg<'tcx>>`, the uniqueness requirement for
// lists is upheld.
let substs = self._intern_substs(ty::subst::ty_slice_as_generic_args(ts));
substs.try_as_type_list().unwrap()
}
}

pub fn intern_substs(self, ts: &[GenericArg<'tcx>]) -> &'tcx List<GenericArg<'tcx>> {
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/ty/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,9 +116,10 @@ impl<'tcx> Ty<'tcx> {
}
_ => true,
}),
Projection(ProjectionTy { substs: args, .. }) | Adt(_, args) | Tuple(args) => {
Projection(ProjectionTy { substs: args, .. }) | Adt(_, args) => {
args.iter().all(generic_arg_is_suggestible)
}
Tuple(args) => args.iter().all(|ty| ty.is_suggestable()),
Slice(ty) | RawPtr(TypeAndMut { ty, .. }) | Ref(_, ty, _) => ty.is_suggestable(),
Array(ty, c) => ty.is_suggestable() && const_is_suggestable(c.val()),
_ => true,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_middle/src/ty/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -201,8 +201,8 @@ impl FlagComputation {
self.add_ty(ty);
}

&ty::Tuple(ref substs) => {
self.add_substs(substs);
&ty::Tuple(types) => {
self.add_tys(types);
}

&ty::FnDef(_, substs) => {
Expand Down
30 changes: 30 additions & 0 deletions compiler/rustc_middle/src/ty/impls_ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,36 @@ impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for ty::subst::GenericArg<'t
}
}

impl<'a, 'tcx> HashStable<StableHashingContext<'a>> for ty::subst::GenericArgKind<'tcx> {
fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) {
match self {
// WARNING: We dedup cache the `HashStable` results for `List`
// while ignoring types and freely transmute
// between `List<Ty<'tcx>>` and `List<GenericArg<'tcx>>`.
// See `fn intern_type_list` for more details.
//
// We therefore hash types without adding a hash for their discriminant.
//
// In order to make it very unlikely for the sequence of bytes being hashed for
// a `GenericArgKind::Type` to be the same as the sequence of bytes being
// hashed for one of the other variants, we hash a `0xFF` byte before hashing
// their discriminant (since the discriminant of `TyKind` is unlikely to ever start
// with 0xFF).
ty::subst::GenericArgKind::Type(ty) => ty.hash_stable(hcx, hasher),
ty::subst::GenericArgKind::Const(ct) => {
0xFFu8.hash_stable(hcx, hasher);
mem::discriminant(self).hash_stable(hcx, hasher);
ct.hash_stable(hcx, hasher);
}
ty::subst::GenericArgKind::Lifetime(lt) => {
0xFFu8.hash_stable(hcx, hasher);
mem::discriminant(self).hash_stable(hcx, hasher);
lt.hash_stable(hcx, hasher);
}
}
}
}

impl<'a> HashStable<StableHashingContext<'a>> for ty::RegionKind {
fn hash_stable(&self, hcx: &mut StableHashingContext<'a>, hasher: &mut StableHasher) {
mem::discriminant(self).hash_stable(hcx, hasher);
Expand Down
Loading

0 comments on commit 03a8cc7

Please sign in to comment.