Auto merge of #142508 - Mark-Simulacrum:skip-noop-drop-glue, r=fee1-dead

Skip no-op drop glue

Since rust-lang/rust#122662 this no longer gets used in vtables, so we're safe to fully
drop generating functions from vtables. Those are eventually cleaned up
by LLVM, but it's wasteful to produce them in the first place.

This doesn't appear to be a significant win (and shows some slight regressions) but
seems like the right thing to do. At minimum it reduces noise in the LLVM IR we generate,
which seems like a good thing.

compiler/rustc_monomorphize/src/collector.rs

@@ -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));
             }
@@ -1177,8 +1180,13 @@ fn create_mono_items_for_vtable_methods<'tcx>(
         output.extend(methods);
     }
 
-    // Also add the destructor.
+    // Also add the destructor, if it's necessary.
-    visit_drop_use(tcx, impl_ty, false, source, output);
+    //
+    // 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.

tests/codegen-units/item-collection/drop-glue-noop.rs

@@ -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
+}

tests/codegen-units/item-collection/instantiation-through-vtable.rs

@@ -24,7 +24,6 @@ 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;
@@ -32,7 +31,6 @@ pub fn start(_: isize, _: *const *const u8) -> isize {
     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;

tests/codegen-units/item-collection/non-generic-closures.rs

@@ -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"]
@@ -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:27:13: 27: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 <{closure@TEST_PATH:27:13: 27: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]
     let f = |a: i32| {
         let _ = a + 2;
     };
@@ -40,6 +39,20 @@ 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 {
@@ -47,6 +60,7 @@ pub fn start(_: isize, _: *const *const u8) -> isize {
     assigned_to_variable_but_not_executed();
     assigned_to_variable_executed_directly();
     assigned_to_variable_executed_indirectly();
+    with_drop(PresentDrop);
 
     0
 }
@@ -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) {}
+}

tests/codegen-units/item-collection/unsizing.rs

@@ -1,4 +1,4 @@
-//@ compile-flags:-Zmir-opt-level=0
+//@ compile-flags:-Zmir-opt-level=0 -O
 
 #![deny(dead_code)]
 #![feature(coerce_unsized)]
@@ -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

tests/codegen/sanitizer/cfi/emit-type-metadata-id-itanium-cxx-abi-drop-in-place.rs

@@ -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
 
@@ -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"}