Skip to content

Skip no-op drop glue #142508

New issue

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

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

Already on GitHub? # to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 10 additions & 2 deletions compiler/rustc_monomorphize/src/collector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -949,6 +949,9 @@ fn visit_instance_use<'tcx>(
}
ty::InstanceKind::DropGlue(_, None) => {
// Don't need to emit noop drop glue if we are calling directly.
//
// Note that we also optimize away the call to visit_instance_use in vtable construction
// (see create_mono_items_for_vtable_methods).
if !is_direct_call {
output.push(create_fn_mono_item(tcx, instance, source));
}
Expand Down Expand Up @@ -1177,8 +1180,13 @@ fn create_mono_items_for_vtable_methods<'tcx>(
output.extend(methods);
}

// Also add the destructor.
visit_drop_use(tcx, impl_ty, false, source, output);
// Also add the destructor, if it's necessary.
//
// This matches the check in vtable_allocation_provider in middle/ty/vtable.rs,
// if we don't need drop we're not adding an actual pointer to the vtable.
if impl_ty.needs_drop(tcx, ty::TypingEnv::fully_monomorphized()) {
visit_drop_use(tcx, impl_ty, false, source, output);
}
}

/// Scans the CTFE alloc in order to find function pointers and statics that must be monomorphized.
Expand Down
23 changes: 23 additions & 0 deletions tests/codegen-units/item-collection/drop-glue-noop.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
//@ compile-flags:-Clink-dead-code -Zmir-opt-level=0

#![deny(dead_code)]
#![crate_type = "lib"]

//~ MONO_ITEM fn start
#[no_mangle]
pub fn start(_: isize, _: *const *const u8) -> isize {
// No item produced for this, it's a no-op drop and so is removed.
unsafe {
std::ptr::drop_in_place::<u32>(&mut 0);
}

// No choice but to codegen for indirect drop as a function pointer, since we have to produce a
// function with the right signature. In vtables we can avoid that (tested in
// instantiation-through-vtable.rs) because we special case null pointer for drop glue since
// #122662.
//
//~ MONO_ITEM fn std::ptr::drop_in_place::<u64> - shim(None) @@ drop_glue_noop-cgu.0[External]
std::ptr::drop_in_place::<u64> as unsafe fn(*mut u64);

0
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,13 @@ impl<T> Trait for Struct<T> {
pub fn start(_: isize, _: *const *const u8) -> isize {
let s1 = Struct { _a: 0u32 };

//~ MONO_ITEM fn std::ptr::drop_in_place::<Struct<u32>> - shim(None) @@ instantiation_through_vtable-cgu.0[External]
//~ MONO_ITEM fn <Struct<u32> as Trait>::foo
//~ MONO_ITEM fn <Struct<u32> as Trait>::bar
let r1 = &s1 as &Trait;
r1.foo();
r1.bar();

let s1 = Struct { _a: 0u64 };
//~ MONO_ITEM fn std::ptr::drop_in_place::<Struct<u64>> - shim(None) @@ instantiation_through_vtable-cgu.0[External]
//~ MONO_ITEM fn <Struct<u64> as Trait>::foo
//~ MONO_ITEM fn <Struct<u64> as Trait>::bar
let _ = &s1 as &Trait;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to me this test is mostly a duplicate of the unsizing test, so I'm skipping updating it with its own drop glue variant -- seems like that's covered there. Maybe worth deleting one of these entirely in a follow-up PR though.

Expand Down
29 changes: 25 additions & 4 deletions tests/codegen-units/item-collection/non-generic-closures.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//@ compile-flags:-Clink-dead-code -Zinline-mir=no
//@ compile-flags:-Clink-dead-code -Zinline-mir=no -O

#![deny(dead_code)]
#![crate_type = "lib"]
Expand All @@ -22,9 +22,8 @@ fn assigned_to_variable_but_not_executed() {
//~ MONO_ITEM fn assigned_to_variable_executed_indirectly @@ non_generic_closures-cgu.0[External]
fn assigned_to_variable_executed_indirectly() {
//~ MONO_ITEM fn assigned_to_variable_executed_indirectly::{closure#0} @@ non_generic_closures-cgu.0[External]
//~ MONO_ITEM fn <{closure@TEST_PATH:28:13: 28:21} as std::ops::FnOnce<(i32,)>>::call_once - shim @@ non_generic_closures-cgu.0[External]
//~ MONO_ITEM fn <{closure@TEST_PATH:28:13: 28:21} as std::ops::FnOnce<(i32,)>>::call_once - shim(vtable) @@ non_generic_closures-cgu.0[External]
//~ MONO_ITEM fn std::ptr::drop_in_place::<{closure@TEST_PATH:28:13: 28:21}> - shim(None) @@ non_generic_closures-cgu.0[External]
//~ MONO_ITEM fn <{closure@TEST_PATH:27:13: 27:21} as std::ops::FnOnce<(i32,)>>::call_once - shim @@ non_generic_closures-cgu.0[External]
//~ MONO_ITEM fn <{closure@TEST_PATH:27:13: 27:21} as std::ops::FnOnce<(i32,)>>::call_once - shim(vtable) @@ non_generic_closures-cgu.0[External]
let f = |a: i32| {
let _ = a + 2;
};
Expand All @@ -40,13 +39,28 @@ fn assigned_to_variable_executed_directly() {
f(4);
}

// Make sure we generate mono items for stateful closures that need dropping
//~ MONO_ITEM fn with_drop @@ non_generic_closures-cgu.0[External]
fn with_drop(v: PresentDrop) {
//~ MONO_ITEM fn with_drop::{closure#0} @@ non_generic_closures-cgu.0[External]
//~ MONO_ITEM fn std::ptr::drop_in_place::<PresentDrop> - shim(Some(PresentDrop)) @@ non_generic_closures-cgu.0[Internal]
//~ MONO_ITEM fn std::ptr::drop_in_place::<{closure@TEST_PATH:49:14: 49:24}> - shim(Some({closure@TEST_PATH:49:14: 49:24})) @@ non_generic_closures-cgu.0[Internal]

let _f = |a: usize| {
let _ = a + 2;
//~ MONO_ITEM fn std::mem::drop::<PresentDrop> @@ non_generic_closures-cgu.0[External]
drop(v);
};
}

//~ MONO_ITEM fn start @@ non_generic_closures-cgu.0[External]
#[no_mangle]
pub fn start(_: isize, _: *const *const u8) -> isize {
temporary();
assigned_to_variable_but_not_executed();
assigned_to_variable_executed_directly();
assigned_to_variable_executed_indirectly();
with_drop(PresentDrop);

0
}
Expand All @@ -55,3 +69,10 @@ pub fn start(_: isize, _: *const *const u8) -> isize {
fn run_closure(f: &Fn(i32)) {
f(3);
}

struct PresentDrop;

impl Drop for PresentDrop {
//~ MONO_ITEM fn <PresentDrop as std::ops::Drop>::drop @@ non_generic_closures-cgu.0[External]
fn drop(&mut self) {}
}
24 changes: 19 additions & 5 deletions tests/codegen-units/item-collection/unsizing.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
//@ compile-flags:-Zmir-opt-level=0
//@ compile-flags:-Zmir-opt-level=0 -O

#![deny(dead_code)]
#![feature(coerce_unsized)]
Expand Down Expand Up @@ -42,33 +42,47 @@ struct Wrapper<T: ?Sized>(#[allow(dead_code)] *const T);

impl<T: ?Sized + Unsize<U>, U: ?Sized> CoerceUnsized<Wrapper<U>> for Wrapper<T> {}

struct PresentDrop;

impl Drop for PresentDrop {
fn drop(&mut self) {}
}

// Custom Coercion Case
impl Trait for PresentDrop {
fn foo(&self) {}
}

//~ MONO_ITEM fn start
#[no_mangle]
pub fn start(_: isize, _: *const *const u8) -> isize {
// simple case
let bool_sized = &true;
//~ MONO_ITEM fn std::ptr::drop_in_place::<bool> - shim(None) @@ unsizing-cgu.0[Internal]
//~ MONO_ITEM fn <bool as Trait>::foo
let _bool_unsized = bool_sized as &Trait;

let char_sized = &'a';

//~ MONO_ITEM fn std::ptr::drop_in_place::<char> - shim(None) @@ unsizing-cgu.0[Internal]
//~ MONO_ITEM fn <char as Trait>::foo
let _char_unsized = char_sized as &Trait;

// struct field
let struct_sized = &Struct { _a: 1, _b: 2, _c: 3.0f64 };
//~ MONO_ITEM fn std::ptr::drop_in_place::<f64> - shim(None) @@ unsizing-cgu.0[Internal]
//~ MONO_ITEM fn <f64 as Trait>::foo
let _struct_unsized = struct_sized as &Struct<Trait>;

// custom coercion
let wrapper_sized = Wrapper(&0u32);
//~ MONO_ITEM fn std::ptr::drop_in_place::<u32> - shim(None) @@ unsizing-cgu.0[Internal]
//~ MONO_ITEM fn <u32 as Trait>::foo
let _wrapper_sized = wrapper_sized as Wrapper<Trait>;

// with drop
let droppable = &PresentDrop;
//~ MONO_ITEM fn <PresentDrop as std::ops::Drop>::drop @@ unsizing-cgu.0[Internal]
//~ MONO_ITEM fn std::ptr::drop_in_place::<PresentDrop> - shim(Some(PresentDrop)) @@ unsizing-cgu.0[Internal]
//~ MONO_ITEM fn <PresentDrop as Trait>::foo
let droppable = droppable as &dyn Trait;

false.foo();

0
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
// Verifies that type metadata identifiers for drop functions are emitted correctly.
//
// Non needs_drop drop glue isn't codegen'd at all, so we don't try to check the IDs there. But we
// do check it's not emitted which should help catch bugs if we do start generating it again in the
// future.
//
//@ needs-sanitizer-cfi
//@ compile-flags: -Clto -Cno-prepopulate-passes -Copt-level=0 -Zsanitizer=cfi -Ctarget-feature=-crt-static

Expand All @@ -10,18 +14,18 @@
// CHECK: call i1 @llvm.type.test(ptr {{%.+}}, metadata !"_ZTSFvPu3dynIu{{[0-9]+}}NtNtNtC{{[[:print:]]+}}_4core3ops4drop4Dropu6regionEE")

struct EmptyDrop;
// CHECK: define{{.*}}4core3ptr{{[0-9]+}}drop_in_place$LT${{.*}}EmptyDrop$GT${{.*}}!type ![[TYPE1]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}}
// CHECK-NOT: define{{.*}}4core3ptr{{[0-9]+}}drop_in_place$LT${{.*}}EmptyDrop$GT${{.*}}!type ![[TYPE1]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}}

struct NonEmptyDrop;
struct PresentDrop;

impl Drop for NonEmptyDrop {
impl Drop for PresentDrop {
fn drop(&mut self) {}
// CHECK: define{{.*}}4core3ptr{{[0-9]+}}drop_in_place$LT${{.*}}NonEmptyDrop$GT${{.*}}!type ![[TYPE1]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}}
// CHECK: define{{.*}}4core3ptr{{[0-9]+}}drop_in_place$LT${{.*}}PresentDrop$GT${{.*}}!type ![[TYPE1]] !type !{{[0-9]+}} !type !{{[0-9]+}} !type !{{[0-9]+}}
}

pub fn foo() {
let _ = Box::new(EmptyDrop) as Box<dyn Send>;
let _ = Box::new(NonEmptyDrop) as Box<dyn Send>;
let _ = Box::new(PresentDrop) as Box<dyn Send>;
}

// CHECK: ![[TYPE1]] = !{i64 0, !"_ZTSFvPu3dynIu{{[0-9]+}}NtNtNtC{{[[:print:]]+}}_4core3ops4drop4Dropu6regionEE"}
Loading