Skip to content

Commit

Permalink
Fix setting inline hint based on InstanceDef::requires_inline
Browse files Browse the repository at this point in the history
For instances where `InstanceDef::requires_inline` is true, an attempt
is made to set an inline hint though a call to the `inline` function.
The attempt is ineffective, since all attributes will be usually removed
by the second call.

Fix the issue by applying the attributes only once, with user provided
attributes having a priority when provided.
  • Loading branch information
tmiasko committed Nov 17, 2020
1 parent f5230fb commit 4ea25da
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 12 deletions.
16 changes: 4 additions & 12 deletions compiler/rustc_codegen_llvm/src/attributes.rs
Expand Up @@ -25,7 +25,7 @@ use crate::value::Value;

/// Mark LLVM function to use provided inline heuristic.
#[inline]
fn inline(cx: &CodegenCx<'ll, '_>, val: &'ll Value, inline: InlineAttr) {
fn inline(cx: &CodegenCx<'ll, '_>, val: &'ll Value, inline: InlineAttr, requires_inline: bool) {
use self::InlineAttr::*;
match inline {
Hint => Attribute::InlineHint.apply_llfn(Function, val),
Expand All @@ -35,11 +35,8 @@ fn inline(cx: &CodegenCx<'ll, '_>, val: &'ll Value, inline: InlineAttr) {
Attribute::NoInline.apply_llfn(Function, val);
}
}
None => {
Attribute::InlineHint.unapply_llfn(Function, val);
Attribute::AlwaysInline.unapply_llfn(Function, val);
Attribute::NoInline.unapply_llfn(Function, val);
}
None if requires_inline => Attribute::InlineHint.apply_llfn(Function, val),
None => {}
};
}

Expand Down Expand Up @@ -229,12 +226,7 @@ pub fn from_fn_attrs(cx: &CodegenCx<'ll, 'tcx>, llfn: &'ll Value, instance: ty::
}
}

// FIXME(eddyb) consolidate these two `inline` calls (and avoid overwrites).
if instance.def.requires_inline(cx.tcx) {
inline(cx, llfn, attributes::InlineAttr::Hint);
}

inline(cx, llfn, codegen_fn_attrs.inline.clone());
inline(cx, llfn, codegen_fn_attrs.inline.clone(), instance.def.requires_inline(cx.tcx));

// The `uwtable` attribute according to LLVM is:
//
Expand Down
31 changes: 31 additions & 0 deletions src/test/codegen/inline-hint.rs
@@ -0,0 +1,31 @@
// Checks that closures, constructors, and shims except
// for a drop glue receive inline hint by default.
//
// compile-flags: -Cno-prepopulate-passes -Zsymbol-mangling-version=v0
#![crate_type = "lib"]

pub fn f() {
let a = A;
let b = (0i32, 1i32, 2i32, 3i32);
let c = || {};

a(String::new(), String::new());
b.clone();
c();
}

struct A(String, String);

// CHECK: ; core::ptr::drop_in_place::<inline_hint::A>
// CHECK-NEXT: ; Function Attrs:
// CHECK-NOT: inlinehint
// CHECK-SAME: {{$}}

// CHECK: ; <(i32, i32, i32, i32) as core::clone::Clone>::clone
// CHECK-NEXT: ; Function Attrs: inlinehint

// CHECK: ; inline_hint::f::{closure#0}
// CHECK-NEXT: ; Function Attrs: inlinehint

// CHECK: ; inline_hint::A
// CHECK-NEXT: ; Function Attrs: inlinehint

0 comments on commit 4ea25da

Please sign in to comment.