Skip to content

Commit e4f3c16

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 61a1dbd commit e4f3c16

File tree

4 files changed

+152
-94
lines changed

4 files changed

+152
-94
lines changed

‎compiler/rustc_codegen_ssa/src/meth.rs

Lines changed: 13 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,
@@ -39,13 +39,24 @@ impl<'a, 'tcx> VirtualIndex {
3939
} else {
4040
let gep = bx.inbounds_ptradd(llvtable, bx.const_usize(vtable_byte_offset));
4141
let ptr = bx.load(llty, gep, ptr_align);
42-
bx.nonnull_metadata(ptr);
4342
// VTable loads are invariant.
4443
bx.set_invariant_load(ptr);
4544
ptr
4645
}
4746
}
4847

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

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

Lines changed: 115 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -498,6 +498,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
498498
&mut self,
499499
helper: TerminatorCodegenHelper<'tcx>,
500500
bx: &mut Bx,
501+
source_info: &mir::SourceInfo,
501502
location: mir::Place<'tcx>,
502503
target: mir::BasicBlock,
503504
unwind: mir::UnwindAction,
@@ -521,90 +522,109 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
521522
args1 = [place.val.llval];
522523
&args1[..]
523524
};
524-
let (drop_fn, fn_abi, drop_instance) =
525-
match ty.kind() {
526-
// FIXME(eddyb) perhaps move some of this logic into
527-
// `Instance::resolve_drop_in_place`?
528-
ty::Dynamic(_, _, ty::Dyn) => {
529-
// IN THIS ARM, WE HAVE:
530-
// ty = *mut (dyn Trait)
531-
// which is: exists<T> ( *mut T, Vtable<T: Trait> )
532-
// args[0] args[1]
533-
//
534-
// args = ( Data, Vtable )
535-
// |
536-
// v
537-
// /-------\
538-
// | ... |
539-
// \-------/
540-
//
541-
let virtual_drop = Instance {
542-
def: ty::InstanceDef::Virtual(drop_fn.def_id(), 0),
543-
args: drop_fn.args,
544-
};
545-
debug!("ty = {:?}", ty);
546-
debug!("drop_fn = {:?}", drop_fn);
547-
debug!("args = {:?}", args);
548-
let fn_abi = bx.fn_abi_of_instance(virtual_drop, ty::List::empty());
549-
let vtable = args[1];
550-
// Truncate vtable off of args list
551-
args = &args[..1];
552-
(
553-
meth::VirtualIndex::from_index(ty::COMMON_VTABLE_ENTRIES_DROPINPLACE)
554-
.get_fn(bx, vtable, ty, fn_abi),
555-
fn_abi,
556-
virtual_drop,
557-
)
558-
}
559-
ty::Dynamic(_, _, ty::DynStar) => {
560-
// IN THIS ARM, WE HAVE:
561-
// ty = *mut (dyn* Trait)
562-
// which is: *mut exists<T: sizeof(T) == sizeof(usize)> (T, Vtable<T: Trait>)
563-
//
564-
// args = [ * ]
565-
// |
566-
// v
567-
// ( Data, Vtable )
568-
// |
569-
// v
570-
// /-------\
571-
// | ... |
572-
// \-------/
573-
//
574-
//
575-
// WE CAN CONVERT THIS INTO THE ABOVE LOGIC BY DOING
576-
//
577-
// data = &(*args[0]).0 // gives a pointer to Data above (really the same pointer)
578-
// vtable = (*args[0]).1 // loads the vtable out
579-
// (data, vtable) // an equivalent Rust `*mut dyn Trait`
580-
//
581-
// SO THEN WE CAN USE THE ABOVE CODE.
582-
let virtual_drop = Instance {
583-
def: ty::InstanceDef::Virtual(drop_fn.def_id(), 0),
584-
args: drop_fn.args,
585-
};
586-
debug!("ty = {:?}", ty);
587-
debug!("drop_fn = {:?}", drop_fn);
588-
debug!("args = {:?}", args);
589-
let fn_abi = bx.fn_abi_of_instance(virtual_drop, ty::List::empty());
590-
let meta_ptr = place.project_field(bx, 1);
591-
let meta = bx.load_operand(meta_ptr);
592-
// Truncate vtable off of args list
593-
args = &args[..1];
594-
debug!("args' = {:?}", args);
595-
(
596-
meth::VirtualIndex::from_index(ty::COMMON_VTABLE_ENTRIES_DROPINPLACE)
597-
.get_fn(bx, meta.immediate(), ty, fn_abi),
598-
fn_abi,
599-
virtual_drop,
600-
)
601-
}
602-
_ => (
603-
bx.get_fn_addr(drop_fn),
604-
bx.fn_abi_of_instance(drop_fn, ty::List::empty()),
605-
drop_fn,
606-
),
607-
};
525+
let (maybe_null, drop_fn, fn_abi, drop_instance) = match ty.kind() {
526+
// FIXME(eddyb) perhaps move some of this logic into
527+
// `Instance::resolve_drop_in_place`?
528+
ty::Dynamic(_, _, ty::Dyn) => {
529+
// IN THIS ARM, WE HAVE:
530+
// ty = *mut (dyn Trait)
531+
// which is: exists<T> ( *mut T, Vtable<T: Trait> )
532+
// args[0] args[1]
533+
//
534+
// args = ( Data, Vtable )
535+
// |
536+
// v
537+
// /-------\
538+
// | ... |
539+
// \-------/
540+
//
541+
let virtual_drop = Instance {
542+
def: ty::InstanceDef::Virtual(drop_fn.def_id(), 0),
543+
args: drop_fn.args,
544+
};
545+
debug!("ty = {:?}", ty);
546+
debug!("drop_fn = {:?}", drop_fn);
547+
debug!("args = {:?}", args);
548+
let fn_abi = bx.fn_abi_of_instance(virtual_drop, ty::List::empty());
549+
let vtable = args[1];
550+
// Truncate vtable off of args list
551+
args = &args[..1];
552+
(
553+
true,
554+
meth::VirtualIndex::from_index(ty::COMMON_VTABLE_ENTRIES_DROPINPLACE)
555+
.get_optional_fn(bx, vtable, ty, fn_abi),
556+
fn_abi,
557+
virtual_drop,
558+
)
559+
}
560+
ty::Dynamic(_, _, ty::DynStar) => {
561+
// IN THIS ARM, WE HAVE:
562+
// ty = *mut (dyn* Trait)
563+
// which is: *mut exists<T: sizeof(T) == sizeof(usize)> (T, Vtable<T: Trait>)
564+
//
565+
// args = [ * ]
566+
// |
567+
// v
568+
// ( Data, Vtable )
569+
// |
570+
// v
571+
// /-------\
572+
// | ... |
573+
// \-------/
574+
//
575+
//
576+
// WE CAN CONVERT THIS INTO THE ABOVE LOGIC BY DOING
577+
//
578+
// data = &(*args[0]).0 // gives a pointer to Data above (really the same pointer)
579+
// vtable = (*args[0]).1 // loads the vtable out
580+
// (data, vtable) // an equivalent Rust `*mut dyn Trait`
581+
//
582+
// SO THEN WE CAN USE THE ABOVE CODE.
583+
let virtual_drop = Instance {
584+
def: ty::InstanceDef::Virtual(drop_fn.def_id(), 0),
585+
args: drop_fn.args,
586+
};
587+
debug!("ty = {:?}", ty);
588+
debug!("drop_fn = {:?}", drop_fn);
589+
debug!("args = {:?}", args);
590+
let fn_abi = bx.fn_abi_of_instance(virtual_drop, ty::List::empty());
591+
let meta_ptr = place.project_field(bx, 1);
592+
let meta = bx.load_operand(meta_ptr);
593+
// Truncate vtable off of args list
594+
args = &args[..1];
595+
debug!("args' = {:?}", args);
596+
(
597+
true,
598+
meth::VirtualIndex::from_index(ty::COMMON_VTABLE_ENTRIES_DROPINPLACE)
599+
.get_optional_fn(bx, meta.immediate(), ty, fn_abi),
600+
fn_abi,
601+
virtual_drop,
602+
)
603+
}
604+
_ => (
605+
false,
606+
bx.get_fn_addr(drop_fn),
607+
bx.fn_abi_of_instance(drop_fn, ty::List::empty()),
608+
drop_fn,
609+
),
610+
};
611+
612+
// We generate a null check for the drop_fn. This saves a bunch of relocations being
613+
// generated for no-op drops.
614+
if maybe_null {
615+
let is_not_null = bx.append_sibling_block("is_not_null");
616+
let llty = bx.fn_ptr_backend_type(fn_abi);
617+
let null = bx.const_null(llty);
618+
let non_null = bx.icmp(
619+
base::bin_op_to_icmp_predicate(mir::BinOp::Ne.to_hir_binop(), false),
620+
drop_fn,
621+
null,
622+
);
623+
bx.cond_br(non_null, is_not_null, self.llbb(target));
624+
bx.switch_to_block(is_not_null);
625+
self.set_debug_loc(bx, *source_info);
626+
}
627+
608628
helper.do_call(
609629
self,
610630
bx,
@@ -615,7 +635,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
615635
unwind,
616636
&[],
617637
Some(drop_instance),
618-
mergeable_succ,
638+
!maybe_null && mergeable_succ,
619639
)
620640
}
621641

@@ -1344,9 +1364,16 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
13441364
MergingSucc::False
13451365
}
13461366

1347-
mir::TerminatorKind::Drop { place, target, unwind, replace: _ } => {
1348-
self.codegen_drop_terminator(helper, bx, place, target, unwind, mergeable_succ())
1349-
}
1367+
mir::TerminatorKind::Drop { place, target, unwind, replace: _ } => self
1368+
.codegen_drop_terminator(
1369+
helper,
1370+
bx,
1371+
&terminator.source_info,
1372+
place,
1373+
target,
1374+
unwind,
1375+
mergeable_succ(),
1376+
),
13501377

13511378
mir::TerminatorKind::Assert { ref cond, expected, ref msg, target, unwind } => self
13521379
.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),

‎tests/codegen/vtable-loads.rs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
//@ compile-flags: -O
2+
3+
#![crate_type = "lib"]
4+
5+
// CHECK-LABEL: @loop_skips_vtable_load
6+
#[no_mangle]
7+
pub fn loop_skips_vtable_load(x: &dyn Fn()) {
8+
// CHECK: load ptr, ptr %0{{.*}}, !invariant.load
9+
// CHECK-NEXT: tail call void %1
10+
// CHECK-NOT: load ptr
11+
x();
12+
for _ in 0..100 {
13+
// CHECK: tail call void %1
14+
x();
15+
}
16+
}

0 commit comments

Comments
 (0)