Skip to content

Commit

Permalink
Auto merge of #122662 - Mark-Simulacrum:optional-drop, r=bjorn3
Browse files Browse the repository at this point in the history 
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 (11%) 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.

Accepted MCP: rust-lang/compiler-team#730
  • Loading branch information
@bors
bors committed May 28, 2024
2 parents 7717a30 + 4c002fc commit 8c4db85
Show file tree
Hide file tree
Showing 7 changed files with 203 additions and 116 deletions.
16 changes: 16 additions & 0 deletions compiler/rustc_codegen_cranelift/src/abi/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,7 @@ pub(crate) fn codegen_drop<'tcx>(
fx: &mut FunctionCx<'_, '_, 'tcx>,
source_info: mir::SourceInfo,
drop_place: CPlace<'tcx>,
target: BasicBlock,
) {
let ty = drop_place.layout().ty;
let drop_instance = Instance::resolve_drop_in_place(fx.tcx, ty).polymorphize(fx.tcx);
Expand Down Expand Up @@ -620,6 +621,12 @@ pub(crate) fn codegen_drop<'tcx>(
let ptr = ptr.get_addr(fx);
let drop_fn = crate::vtable::drop_fn_of_obj(fx, vtable);

let is_null = fx.bcx.ins().icmp_imm(IntCC::Equal, drop_fn, 0);
let target_block = fx.get_block(target);
let continued = fx.bcx.create_block();
fx.bcx.ins().brif(is_null, target_block, &[], continued, &[]);
fx.bcx.switch_to_block(continued);

// FIXME(eddyb) perhaps move some of this logic into
// `Instance::resolve_drop_in_place`?
let virtual_drop = Instance {
Expand Down Expand Up @@ -659,6 +666,12 @@ pub(crate) fn codegen_drop<'tcx>(
let (data, vtable) = drop_place.to_cvalue(fx).dyn_star_force_data_on_stack(fx);
let drop_fn = crate::vtable::drop_fn_of_obj(fx, vtable);

let is_null = fx.bcx.ins().icmp_imm(IntCC::Equal, drop_fn, 0);
let target_block = fx.get_block(target);
let continued = fx.bcx.create_block();
fx.bcx.ins().brif(is_null, target_block, &[], continued, &[]);
fx.bcx.switch_to_block(continued);

let virtual_drop = Instance {
def: ty::InstanceDef::Virtual(drop_instance.def_id(), 0),
args: drop_instance.args,
Expand Down Expand Up @@ -697,4 +710,7 @@ pub(crate) fn codegen_drop<'tcx>(
}
}
}

let target_block = fx.get_block(target);
fx.bcx.ins().jump(target_block, &[]);
}
5 changes: 1 addition & 4 deletions compiler/rustc_codegen_cranelift/src/base.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -548,10 +548,7 @@ fn codegen_fn_body(fx: &mut FunctionCx<'_, '_, '_>, start_block: Block) {
}
TerminatorKind::Drop { place, target, unwind: _, replace: _ } => {
let drop_place = codegen_place(fx, *place);
crate::abi::codegen_drop(fx, source_info, drop_place);

let target_block = fx.get_block(*target);
fx.bcx.ins().jump(target_block, &[]);
crate::abi::codegen_drop(fx, source_info, drop_place, *target);
}
};
}
Expand Down
27 changes: 25 additions & 2 deletions compiler/rustc_codegen_ssa/src/meth.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,13 @@ impl<'a, 'tcx> VirtualIndex {
VirtualIndex(index as u64)
}

pub fn get_fn<Bx: BuilderMethods<'a, 'tcx>>(
fn get_fn_inner<Bx: BuilderMethods<'a, 'tcx>>(
self,
bx: &mut Bx,
llvtable: Bx::Value,
ty: Ty<'tcx>,
fn_abi: &FnAbi<'tcx, Ty<'tcx>>,
nonnull: bool,
) -> Bx::Value {
// Load the function pointer from the object.
debug!("get_fn({llvtable:?}, {ty:?}, {self:?})");
Expand All @@ -41,13 +42,35 @@ impl<'a, 'tcx> VirtualIndex {
} else {
let gep = bx.inbounds_ptradd(llvtable, bx.const_usize(vtable_byte_offset));
let ptr = bx.load(llty, gep, ptr_align);
bx.nonnull_metadata(ptr);
// VTable loads are invariant.
bx.set_invariant_load(ptr);
if nonnull {
bx.nonnull_metadata(ptr);
}
ptr
}
}

pub fn get_optional_fn<Bx: BuilderMethods<'a, 'tcx>>(
self,
bx: &mut Bx,
llvtable: Bx::Value,
ty: Ty<'tcx>,
fn_abi: &FnAbi<'tcx, Ty<'tcx>>,
) -> Bx::Value {
self.get_fn_inner(bx, llvtable, ty, fn_abi, false)
}

pub fn get_fn<Bx: BuilderMethods<'a, 'tcx>>(
self,
bx: &mut Bx,
llvtable: Bx::Value,
ty: Ty<'tcx>,
fn_abi: &FnAbi<'tcx, Ty<'tcx>>,
) -> Bx::Value {
self.get_fn_inner(bx, llvtable, ty, fn_abi, true)
}

pub fn get_usize<Bx: BuilderMethods<'a, 'tcx>>(
self,
bx: &mut Bx,
Expand Down
200 changes: 112 additions & 88 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
&mut self,
helper: TerminatorCodegenHelper<'tcx>,
bx: &mut Bx,
source_info: &mir::SourceInfo,
location: mir::Place<'tcx>,
target: mir::BasicBlock,
unwind: mir::UnwindAction,
Expand All @@ -523,90 +524,106 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
args1 = [place.val.llval];
&args1[..]
};
let (drop_fn, fn_abi, drop_instance) =
match ty.kind() {
// FIXME(eddyb) perhaps move some of this logic into
// `Instance::resolve_drop_in_place`?
ty::Dynamic(_, _, ty::Dyn) => {
// IN THIS ARM, WE HAVE:
// ty = *mut (dyn Trait)
// which is: exists<T> ( *mut T, Vtable<T: Trait> )
// args[0] args[1]
//
// args = ( Data, Vtable )
// |
// v
// /-------\
// | ... |
// \-------/
//
let virtual_drop = Instance {
def: ty::InstanceDef::Virtual(drop_fn.def_id(), 0), // idx 0: the drop function
args: drop_fn.args,
};
debug!("ty = {:?}", ty);
debug!("drop_fn = {:?}", drop_fn);
debug!("args = {:?}", args);
let fn_abi = bx.fn_abi_of_instance(virtual_drop, ty::List::empty());
let vtable = args[1];
// Truncate vtable off of args list
args = &args[..1];
(
meth::VirtualIndex::from_index(ty::COMMON_VTABLE_ENTRIES_DROPINPLACE)
.get_fn(bx, vtable, ty, fn_abi),
fn_abi,
virtual_drop,
)
}
ty::Dynamic(_, _, ty::DynStar) => {
// IN THIS ARM, WE HAVE:
// ty = *mut (dyn* Trait)
// which is: *mut exists<T: sizeof(T) == sizeof(usize)> (T, Vtable<T: Trait>)
//
// args = [ * ]
// |
// v
// ( Data, Vtable )
// |
// v
// /-------\
// | ... |
// \-------/
//
//
// WE CAN CONVERT THIS INTO THE ABOVE LOGIC BY DOING
//
// data = &(*args[0]).0 // gives a pointer to Data above (really the same pointer)
// vtable = (*args[0]).1 // loads the vtable out
// (data, vtable) // an equivalent Rust `*mut dyn Trait`
//
// SO THEN WE CAN USE THE ABOVE CODE.
let virtual_drop = Instance {
def: ty::InstanceDef::Virtual(drop_fn.def_id(), 0), // idx 0: the drop function
args: drop_fn.args,
};
debug!("ty = {:?}", ty);
debug!("drop_fn = {:?}", drop_fn);
debug!("args = {:?}", args);
let fn_abi = bx.fn_abi_of_instance(virtual_drop, ty::List::empty());
let meta_ptr = place.project_field(bx, 1);
let meta = bx.load_operand(meta_ptr);
// Truncate vtable off of args list
args = &args[..1];
debug!("args' = {:?}", args);
(
meth::VirtualIndex::from_index(ty::COMMON_VTABLE_ENTRIES_DROPINPLACE)
.get_fn(bx, meta.immediate(), ty, fn_abi),
fn_abi,
virtual_drop,
)
}
_ => (
bx.get_fn_addr(drop_fn),
bx.fn_abi_of_instance(drop_fn, ty::List::empty()),
drop_fn,
),
};
let (maybe_null, drop_fn, fn_abi, drop_instance) = match ty.kind() {
// FIXME(eddyb) perhaps move some of this logic into
// `Instance::resolve_drop_in_place`?
ty::Dynamic(_, _, ty::Dyn) => {
// IN THIS ARM, WE HAVE:
// ty = *mut (dyn Trait)
// which is: exists<T> ( *mut T, Vtable<T: Trait> )
// args[0] args[1]
//
// args = ( Data, Vtable )
// |
// v
// /-------\
// | ... |
// \-------/
//
let virtual_drop = Instance {
def: ty::InstanceDef::Virtual(drop_fn.def_id(), 0), // idx 0: the drop function
args: drop_fn.args,
};
debug!("ty = {:?}", ty);
debug!("drop_fn = {:?}", drop_fn);
debug!("args = {:?}", args);
let fn_abi = bx.fn_abi_of_instance(virtual_drop, ty::List::empty());
let vtable = args[1];
// Truncate vtable off of args list
args = &args[..1];
(
true,
meth::VirtualIndex::from_index(ty::COMMON_VTABLE_ENTRIES_DROPINPLACE)
.get_optional_fn(bx, vtable, ty, fn_abi),
fn_abi,
virtual_drop,
)
}
ty::Dynamic(_, _, ty::DynStar) => {
// IN THIS ARM, WE HAVE:
// ty = *mut (dyn* Trait)
// which is: *mut exists<T: sizeof(T) == sizeof(usize)> (T, Vtable<T: Trait>)
//
// args = [ * ]
// |
// v
// ( Data, Vtable )
// |
// v
// /-------\
// | ... |
// \-------/
//
//
// WE CAN CONVERT THIS INTO THE ABOVE LOGIC BY DOING
//
// data = &(*args[0]).0 // gives a pointer to Data above (really the same pointer)
// vtable = (*args[0]).1 // loads the vtable out
// (data, vtable) // an equivalent Rust `*mut dyn Trait`
//
// SO THEN WE CAN USE THE ABOVE CODE.
let virtual_drop = Instance {
def: ty::InstanceDef::Virtual(drop_fn.def_id(), 0), // idx 0: the drop function
args: drop_fn.args,
};
debug!("ty = {:?}", ty);
debug!("drop_fn = {:?}", drop_fn);
debug!("args = {:?}", args);
let fn_abi = bx.fn_abi_of_instance(virtual_drop, ty::List::empty());
let meta_ptr = place.project_field(bx, 1);
let meta = bx.load_operand(meta_ptr);
// Truncate vtable off of args list
args = &args[..1];
debug!("args' = {:?}", args);
(
true,
meth::VirtualIndex::from_index(ty::COMMON_VTABLE_ENTRIES_DROPINPLACE)
.get_optional_fn(bx, meta.immediate(), ty, fn_abi),
fn_abi,
virtual_drop,
)
}
_ => (
false,
bx.get_fn_addr(drop_fn),
bx.fn_abi_of_instance(drop_fn, ty::List::empty()),
drop_fn,
),
};

// We generate a null check for the drop_fn. This saves a bunch of relocations being
// generated for no-op drops.
if maybe_null {
let is_not_null = bx.append_sibling_block("is_not_null");
let llty = bx.fn_ptr_backend_type(fn_abi);
let null = bx.const_null(llty);
let non_null =
bx.icmp(base::bin_op_to_icmp_predicate(mir::BinOp::Ne, false), drop_fn, null);
bx.cond_br(non_null, is_not_null, helper.llbb_with_cleanup(self, target));
bx.switch_to_block(is_not_null);
self.set_debug_loc(bx, *source_info);
}

helper.do_call(
self,
bx,
Expand All @@ -617,7 +634,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
unwind,
&[],
Some(drop_instance),
mergeable_succ,
!maybe_null && mergeable_succ,
)
}

Expand Down Expand Up @@ -1346,9 +1363,16 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
MergingSucc::False
}

mir::TerminatorKind::Drop { place, target, unwind, replace: _ } => {
self.codegen_drop_terminator(helper, bx, place, target, unwind, mergeable_succ())
}
mir::TerminatorKind::Drop { place, target, unwind, replace: _ } => self
.codegen_drop_terminator(
helper,
bx,
&terminator.source_info,
place,
target,
unwind,
mergeable_succ(),
),

mir::TerminatorKind::Assert { ref cond, expected, ref msg, target, unwind } => self
.codegen_assert_terminator(
Expand Down
12 changes: 8 additions & 4 deletions compiler/rustc_middle/src/ty/vtable.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,14 @@ pub(super) fn vtable_allocation_provider<'tcx>(
let idx: u64 = u64::try_from(idx).unwrap();
let scalar = match entry {
VtblEntry::MetadataDropInPlace => {
let instance = ty::Instance::resolve_drop_in_place(tcx, ty);
let fn_alloc_id = tcx.reserve_and_set_fn_alloc(instance);
let fn_ptr = Pointer::from(fn_alloc_id);
Scalar::from_pointer(fn_ptr, &tcx)
if ty.needs_drop(tcx, ty::ParamEnv::reveal_all()) {
let instance = ty::Instance::resolve_drop_in_place(tcx, ty);
let fn_alloc_id = tcx.reserve_and_set_fn_alloc(instance);
let fn_ptr = Pointer::from(fn_alloc_id);
Scalar::from_pointer(fn_ptr, &tcx)
} else {
Scalar::from_maybe_pointer(Pointer::null(), &tcx)
}
}
VtblEntry::MetadataSize => Scalar::from_uint(size, ptr_size),
VtblEntry::MetadataAlign => Scalar::from_uint(align, ptr_size),
Expand Down
Loading

0 comments on commit 8c4db85

Please sign in to comment.