Skip to content

Commit

Permalink
Rollup merge of rust-lang#102099 - InnovativeInventor:re-cold-land, r…
Browse files Browse the repository at this point in the history
…=nikic

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

I noticed that certain inlining optimizations were missing while staring at some compiled code output. I'd like to see this relanded, so I rebased the PR from ``@erikdesjardins`` (PR rust-lang#94823).

This PR reapplies rust-lang#92419, which was reverted in rust-lang#94402 due to rust-lang#94390.

Fixes rust-lang#46515, fixes rust-lang#87055.

Update: fixes rust-lang#97217.
  • Loading branch information
TaKO8Ki authored Jun 27, 2023
2 parents 448a75b + e5b84f3 commit 047dd55
Show file tree
Hide file tree
Showing 7 changed files with 83 additions and 12 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_gcc/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1421,7 +1421,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
}

Expand Down
8 changes: 5 additions & 3 deletions compiler/rustc_codegen_llvm/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1221,9 +1221,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]);
}
}

Expand Down
10 changes: 3 additions & 7 deletions compiler/rustc_codegen_ssa/src/mir/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,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 {
Expand All @@ -219,11 +219,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 {
Expand Down Expand Up @@ -1618,7 +1614,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();

Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/traits/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
21 changes: 21 additions & 0 deletions tests/codegen/issue-97217.rs
Original file line number Diff line number Diff line change
@@ -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
}
14 changes: 14 additions & 0 deletions tests/codegen/unwind-landingpad-cold.rs
Original file line number Diff line number Diff line change
@@ -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();
}
38 changes: 38 additions & 0 deletions tests/codegen/unwind-landingpad-inline.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// 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: 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();
}

0 comments on commit 047dd55

Please sign in to comment.