Skip to content

Commit 288ece0

Browse files
Mark-SimulacrumMark-Simulacrum
committed
Omit non-needs_drop drop_in_place in vtables
This replaces the drop_in_place reference with null in vtables. On librustc_driver.so, this drops about ~17k dynamic relocations from the output, since many vtables can now be placed in read-only memory, rather than having a relocated pointer included. This makes a tradeoff by adding a null check at vtable call sites. I'm not sure that's readily avoidable without changing the vtable format (e.g., so that we can use a pc-relative relocation instead of an absolute address, and avoid the dynamic relocation that way). But it seems likely that the check is cheap at runtime.
1 parent a0c20d5 commit 288ece0

File tree

3 files changed

+132
-88
lines changed

3 files changed

+132
-88
lines changed

‎compiler/rustc_codegen_ssa/src/meth.rs‎

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ impl<'a, 'tcx> VirtualIndex {
1313
VirtualIndex(index as u64)
1414
}
1515

16-
pub fn get_fn<Bx: BuilderMethods<'a, 'tcx>>(
16+
pub fn get_optional_fn<Bx: BuilderMethods<'a, 'tcx>>(
1717
self,
1818
bx: &mut Bx,
1919
llvtable: Bx::Value,
@@ -35,17 +35,29 @@ impl<'a, 'tcx> VirtualIndex {
3535
.typeid_metadata(typeid_for_trait_ref(bx.tcx(), expect_dyn_trait_in_self(ty)))
3636
.unwrap();
3737
let func = bx.type_checked_load(llvtable, vtable_byte_offset, typeid);
38+
// FIXME: can be null
3839
func
3940
} else {
4041
let gep = bx.inbounds_ptradd(llvtable, bx.const_usize(vtable_byte_offset));
4142
let ptr = bx.load(llty, gep, ptr_align);
42-
bx.nonnull_metadata(ptr);
4343
// VTable loads are invariant.
4444
bx.set_invariant_load(ptr);
4545
ptr
4646
}
4747
}
4848

49+
pub fn get_fn<Bx: BuilderMethods<'a, 'tcx>>(
50+
self,
51+
bx: &mut Bx,
52+
llvtable: Bx::Value,
53+
ty: Ty<'tcx>,
54+
fn_abi: &FnAbi<'tcx, Ty<'tcx>>,
55+
) -> Bx::Value {
56+
let ptr = self.get_optional_fn(bx, llvtable, ty, fn_abi);
57+
bx.nonnull_metadata(ptr);
58+
ptr
59+
}
60+
4961
pub fn get_usize<Bx: BuilderMethods<'a, 'tcx>>(
5062
self,
5163
bx: &mut Bx,

‎compiler/rustc_codegen_ssa/src/mir/block.rs‎

Lines changed: 110 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -472,6 +472,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
472472
&mut self,
473473
helper: TerminatorCodegenHelper<'tcx>,
474474
bx: &mut Bx,
475+
source_info: &mir::SourceInfo,
475476
location: mir::Place<'tcx>,
476477
target: mir::BasicBlock,
477478
unwind: mir::UnwindAction,
@@ -495,84 +496,104 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
495496
args1 = [place.llval];
496497
&args1[..]
497498
};
498-
let (drop_fn, fn_abi) =
499-
match ty.kind() {
500-
// FIXME(eddyb) perhaps move some of this logic into
501-
// `Instance::resolve_drop_in_place`?
502-
ty::Dynamic(_, _, ty::Dyn) => {
503-
// IN THIS ARM, WE HAVE:
504-
// ty = *mut (dyn Trait)
505-
// which is: exists<T> ( *mut T, Vtable<T: Trait> )
506-
// args[0] args[1]
507-
//
508-
// args = ( Data, Vtable )
509-
// |
510-
// v
511-
// /-------\
512-
// | ... |
513-
// \-------/
514-
//
515-
let virtual_drop = Instance {
516-
def: ty::InstanceDef::Virtual(drop_fn.def_id(), 0),
517-
args: drop_fn.args,
518-
};
519-
debug!("ty = {:?}", ty);
520-
debug!("drop_fn = {:?}", drop_fn);
521-
debug!("args = {:?}", args);
522-
let fn_abi = bx.fn_abi_of_instance(virtual_drop, ty::List::empty());
523-
let vtable = args[1];
524-
// Truncate vtable off of args list
525-
args = &args[..1];
526-
(
527-
meth::VirtualIndex::from_index(ty::COMMON_VTABLE_ENTRIES_DROPINPLACE)
528-
.get_fn(bx, vtable, ty, fn_abi),
529-
fn_abi,
530-
)
531-
}
532-
ty::Dynamic(_, _, ty::DynStar) => {
533-
// IN THIS ARM, WE HAVE:
534-
// ty = *mut (dyn* Trait)
535-
// which is: *mut exists<T: sizeof(T) == sizeof(usize)> (T, Vtable<T: Trait>)
536-
//
537-
// args = [ * ]
538-
// |
539-
// v
540-
// ( Data, Vtable )
541-
// |
542-
// v
543-
// /-------\
544-
// | ... |
545-
// \-------/
546-
//
547-
//
548-
// WE CAN CONVERT THIS INTO THE ABOVE LOGIC BY DOING
549-
//
550-
// data = &(*args[0]).0 // gives a pointer to Data above (really the same pointer)
551-
// vtable = (*args[0]).1 // loads the vtable out
552-
// (data, vtable) // an equivalent Rust `*mut dyn Trait`
553-
//
554-
// SO THEN WE CAN USE THE ABOVE CODE.
555-
let virtual_drop = Instance {
556-
def: ty::InstanceDef::Virtual(drop_fn.def_id(), 0),
557-
args: drop_fn.args,
558-
};
559-
debug!("ty = {:?}", ty);
560-
debug!("drop_fn = {:?}", drop_fn);
561-
debug!("args = {:?}", args);
562-
let fn_abi = bx.fn_abi_of_instance(virtual_drop, ty::List::empty());
563-
let meta_ptr = place.project_field(bx, 1);
564-
let meta = bx.load_operand(meta_ptr);
565-
// Truncate vtable off of args list
566-
args = &args[..1];
567-
debug!("args' = {:?}", args);
568-
(
569-
meth::VirtualIndex::from_index(ty::COMMON_VTABLE_ENTRIES_DROPINPLACE)
570-
.get_fn(bx, meta.immediate(), ty, fn_abi),
571-
fn_abi,
572-
)
573-
}
574-
_ => (bx.get_fn_addr(drop_fn), bx.fn_abi_of_instance(drop_fn, ty::List::empty())),
575-
};
499+
let (maybe_null, drop_fn, fn_abi) = match ty.kind() {
500+
// FIXME(eddyb) perhaps move some of this logic into
501+
// `Instance::resolve_drop_in_place`?
502+
ty::Dynamic(_, _, ty::Dyn) => {
503+
// IN THIS ARM, WE HAVE:
504+
// ty = *mut (dyn Trait)
505+
// which is: exists<T> ( *mut T, Vtable<T: Trait> )
506+
// args[0] args[1]
507+
//
508+
// args = ( Data, Vtable )
509+
// |
510+
// v
511+
// /-------\
512+
// | ... |
513+
// \-------/
514+
//
515+
let virtual_drop = Instance {
516+
def: ty::InstanceDef::Virtual(drop_fn.def_id(), 0),
517+
args: drop_fn.args,
518+
};
519+
debug!("ty = {:?}", ty);
520+
debug!("drop_fn = {:?}", drop_fn);
521+
debug!("args = {:?}", args);
522+
let fn_abi = bx.fn_abi_of_instance(virtual_drop, ty::List::empty());
523+
let vtable = args[1];
524+
// Truncate vtable off of args list
525+
args = &args[..1];
526+
(
527+
true,
528+
meth::VirtualIndex::from_index(ty::COMMON_VTABLE_ENTRIES_DROPINPLACE)
529+
.get_optional_fn(bx, vtable, ty, fn_abi),
530+
fn_abi,
531+
)
532+
}
533+
ty::Dynamic(_, _, ty::DynStar) => {
534+
// IN THIS ARM, WE HAVE:
535+
// ty = *mut (dyn* Trait)
536+
// which is: *mut exists<T: sizeof(T) == sizeof(usize)> (T, Vtable<T: Trait>)
537+
//
538+
// args = [ * ]
539+
// |
540+
// v
541+
// ( Data, Vtable )
542+
// |
543+
// v
544+
// /-------\
545+
// | ... |
546+
// \-------/
547+
//
548+
//
549+
// WE CAN CONVERT THIS INTO THE ABOVE LOGIC BY DOING
550+
//
551+
// data = &(*args[0]).0 // gives a pointer to Data above (really the same pointer)
552+
// vtable = (*args[0]).1 // loads the vtable out
553+
// (data, vtable) // an equivalent Rust `*mut dyn Trait`
554+
//
555+
// SO THEN WE CAN USE THE ABOVE CODE.
556+
let virtual_drop = Instance {
557+
def: ty::InstanceDef::Virtual(drop_fn.def_id(), 0),
558+
args: drop_fn.args,
559+
};
560+
debug!("ty = {:?}", ty);
561+
debug!("drop_fn = {:?}", drop_fn);
562+
debug!("args = {:?}", args);
563+
let fn_abi = bx.fn_abi_of_instance(virtual_drop, ty::List::empty());
564+
let meta_ptr = place.project_field(bx, 1);
565+
let meta = bx.load_operand(meta_ptr);
566+
// Truncate vtable off of args list
567+
args = &args[..1];
568+
debug!("args' = {:?}", args);
569+
(
570+
true,
571+
meth::VirtualIndex::from_index(ty::COMMON_VTABLE_ENTRIES_DROPINPLACE)
572+
.get_optional_fn(bx, meta.immediate(), ty, fn_abi),
573+
fn_abi,
574+
)
575+
}
576+
_ => {
577+
(false, bx.get_fn_addr(drop_fn), bx.fn_abi_of_instance(drop_fn, ty::List::empty()))
578+
}
579+
};
580+
581+
// We generate a null check for the drop_fn. This saves a bunch of relocations being
582+
// generated for no-op drops.
583+
if maybe_null {
584+
let is_not_null = bx.append_sibling_block("is_not_null");
585+
let llty = bx.fn_ptr_backend_type(fn_abi);
586+
let null = bx.const_null(llty);
587+
let non_null = bx.icmp(
588+
base::bin_op_to_icmp_predicate(mir::BinOp::Ne.to_hir_binop(), false),
589+
drop_fn,
590+
null,
591+
);
592+
bx.cond_br(non_null, is_not_null, self.llbb(target));
593+
bx.switch_to_block(is_not_null);
594+
self.set_debug_loc(bx, *source_info);
595+
}
596+
576597
helper.do_call(
577598
self,
578599
bx,
@@ -582,7 +603,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
582603
Some((ReturnDest::Nothing, target)),
583604
unwind,
584605
&[],
585-
mergeable_succ,
606+
!maybe_null && mergeable_succ,
586607
)
587608
}
588609

@@ -1305,9 +1326,16 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
13051326
MergingSucc::False
13061327
}
13071328

1308-
mir::TerminatorKind::Drop { place, target, unwind, replace: _ } => {
1309-
self.codegen_drop_terminator(helper, bx, place, target, unwind, mergeable_succ())
1310-
}
1329+
mir::TerminatorKind::Drop { place, target, unwind, replace: _ } => self
1330+
.codegen_drop_terminator(
1331+
helper,
1332+
bx,
1333+
&terminator.source_info,
1334+
place,
1335+
target,
1336+
unwind,
1337+
mergeable_succ(),
1338+
),
13111339

13121340
mir::TerminatorKind::Assert { ref cond, expected, ref msg, target, unwind } => self
13131341
.codegen_assert_terminator(

‎compiler/rustc_middle/src/ty/vtable.rs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -83,10 +83,14 @@ pub(super) fn vtable_allocation_provider<'tcx>(
8383
let idx: u64 = u64::try_from(idx).unwrap();
8484
let scalar = match entry {
8585
VtblEntry::MetadataDropInPlace => {
86-
let instance = ty::Instance::resolve_drop_in_place(tcx, ty);
87-
let fn_alloc_id = tcx.reserve_and_set_fn_alloc(instance);
88-
let fn_ptr = Pointer::from(fn_alloc_id);
89-
Scalar::from_pointer(fn_ptr, &tcx)
86+
if ty.needs_drop(tcx, ty::ParamEnv::reveal_all()) {
87+
let instance = ty::Instance::resolve_drop_in_place(tcx, ty);
88+
let fn_alloc_id = tcx.reserve_and_set_fn_alloc(instance);
89+
let fn_ptr = Pointer::from(fn_alloc_id);
90+
Scalar::from_pointer(fn_ptr, &tcx)
91+
} else {
92+
Scalar::from_maybe_pointer(Pointer::null(), &tcx)
93+
}
9094
}
9195
VtblEntry::MetadataSize => Scalar::from_uint(size, ptr_size),
9296
VtblEntry::MetadataAlign => Scalar::from_uint(align, ptr_size),

0 commit comments

Comments
 (0)