Reapply: Mark drop calls in landing pads cold instead of noinline

Co-authored-by: Max Fan <[email protected]>
Co-authored-by: Nikita Popov <[email protected]>

Author: 3 people

Diff for: compiler/rustc_codegen_gcc/src/builder.rs

@@ -1420,7 +1420,7 @@ impl<'a, 'gcc, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'gcc, 'tcx> {
         self.cx
     }
 
-    fn do_not_inline(&mut self, _llret: RValue<'gcc>) {
+    fn apply_attrs_to_cleanup_callsite(&mut self, _llret: RValue<'gcc>) {
         // FIXME(bjorn3): implement
     }
 

Diff for: compiler/rustc_codegen_llvm/src/builder.rs

@@ -1225,9 +1225,11 @@ impl<'a, 'll, 'tcx> BuilderMethods<'a, 'tcx> for Builder<'a, 'll, 'tcx> {
         unsafe { llvm::LLVMBuildZExt(self.llbuilder, val, dest_ty, UNNAMED) }
     }
 
-    fn do_not_inline(&mut self, llret: &'ll Value) {
-        let noinline = llvm::AttributeKind::NoInline.create_attr(self.llcx);
-        attributes::apply_to_callsite(llret, llvm::AttributePlace::Function, &[noinline]);
+    fn apply_attrs_to_cleanup_callsite(&mut self, llret: &'ll Value) {
+        // Cleanup is always the cold path.
+        let cold_inline = llvm::AttributeKind::Cold.create_attr(self.llcx);
+
+        attributes::apply_to_callsite(llret, llvm::AttributePlace::Function, &[cold_inline]);
     }
 }
 

Diff for: compiler/rustc_codegen_ssa/src/mir/block.rs

@@ -213,7 +213,7 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
                 self.funclet(fx),
             );
             if fx.mir[self.bb].is_cleanup {
-                bx.do_not_inline(invokeret);
+                bx.apply_attrs_to_cleanup_callsite(invokeret);
             }
 
             if let Some((ret_dest, target)) = destination {
@@ -228,11 +228,7 @@ impl<'a, 'tcx> TerminatorCodegenHelper<'tcx> {
         } else {
             let llret = bx.call(fn_ty, fn_attrs, Some(&fn_abi), fn_ptr, &llargs, self.funclet(fx));
             if fx.mir[self.bb].is_cleanup {
-                // Cleanup is always the cold path. Don't inline
-                // drop glue. Also, when there is a deeply-nested
-                // struct, there are "symmetry" issues that cause
-                // exponential inlining - see issue #41696.
-                bx.do_not_inline(llret);
+                bx.apply_attrs_to_cleanup_callsite(llret);
             }
 
             if let Some((ret_dest, target)) = destination {
@@ -1627,7 +1623,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
         let fn_ty = bx.fn_decl_backend_type(&fn_abi);
 
         let llret = bx.call(fn_ty, None, Some(&fn_abi), fn_ptr, &[], funclet.as_ref());
-        bx.do_not_inline(llret);
+        bx.apply_attrs_to_cleanup_callsite(llret);
 
         bx.unreachable();
 

Diff for: compiler/rustc_codegen_ssa/src/traits/builder.rs

@@ -332,5 +332,5 @@ pub trait BuilderMethods<'a, 'tcx>:
     ) -> Self::Value;
     fn zext(&mut self, val: Self::Value, dest_ty: Self::Type) -> Self::Value;
 
-    fn do_not_inline(&mut self, llret: Self::Value);
+    fn apply_attrs_to_cleanup_callsite(&mut self, llret: Self::Value);
 }

Diff for: tests/codegen/issue-97217.rs

@@ -0,0 +1,21 @@
+// compile-flags: -C opt-level=3
+// ignore-debug: the debug assertions get in the way
+#![crate_type = "lib"]
+
+// Regression test for issue 97217 (the following should result in no allocations)
+
+// CHECK-LABEL: @issue97217
+#[no_mangle]
+pub fn issue97217() -> i32 {
+    // drop_in_place should be inlined and never appear
+    // CHECK-NOT: drop_in_place
+
+    // __rust_alloc should be optimized out
+    // CHECK-NOT: __rust_alloc
+
+    let v1 = vec![5, 6, 7];
+    let v1_iter = v1.iter();
+    let total: i32 = v1_iter.sum();
+    println!("{}",total);
+    total
+}

Diff for: tests/codegen/unwind-landingpad-cold.rs

@@ -0,0 +1,14 @@
+// compile-flags: -Cno-prepopulate-passes
+#![crate_type = "lib"]
+
+// This test checks that drop calls in unwind landing pads
+// get the `cold` attribute.
+
+// CHECK-LABEL: @check_cold
+// CHECK: {{(call|invoke) void .+}}drop_in_place{{.+}} [[ATTRIBUTES:#[0-9]+]]
+// CHECK: attributes [[ATTRIBUTES]] = { cold }
+#[no_mangle]
+pub fn check_cold(f: fn(), x: Box<u32>) {
+    // this may unwind
+    f();
+}

Diff for: tests/codegen/unwind-landingpad-inline.rs

@@ -0,0 +1,40 @@
+// min-llvm-version: 15.0.0
+// compile-flags: -Copt-level=3
+// ignore-debug: the debug assertions get in the way
+#![crate_type = "lib"]
+
+// This test checks that we can inline drop_in_place in
+// unwind landing pads.
+
+// Without inlining, the box pointers escape via the call to drop_in_place,
+// and LLVM will not optimize out the pointer comparison.
+// With inlining, everything should be optimized out.
+// See https://github.com/rust-lang/rust/issues/46515
+// CHECK-LABEL: @check_no_escape_in_landingpad
+// CHECK: start:
+// CHECK-NEXT: __rust_no_alloc_shim_is_unstable
+// CHECK-NEXT: __rust_no_alloc_shim_is_unstable
+// CHECK-NEXT: ret void
+#[no_mangle]
+pub fn check_no_escape_in_landingpad(f: fn()) {
+    let x = &*Box::new(0);
+    let y = &*Box::new(0);
+
+    if x as *const _ == y as *const _ {
+        f();
+    }
+}
+
+// Without inlining, the compiler can't tell that
+// dropping an empty string (in a landing pad) does nothing.
+// With inlining, the landing pad should be optimized out.
+// See https://github.com/rust-lang/rust/issues/87055
+// CHECK-LABEL: @check_eliminate_noop_drop
+// CHECK: start:
+// CHECK-NEXT: call void %g()
+// CHECK-NEXT: ret void
+#[no_mangle]
+pub fn check_eliminate_noop_drop(g: fn()) {
+    let _var = String::new();
+    g();
+}