Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Omit non-needs_drop drop_in_place in vtables #122662

Merged
merged 1 commit into from
May 28, 2024
Merged

Omit non-needs_drop drop_in_place in vtables #122662

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions compiler/rustc_codegen_cranelift/src/abi/mod.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading