Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Never inline C variadics, cold functions, functions with incompatible attributes ... #78966

Merged
merged 6 commits into from
Nov 15, 2020
Merged
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
41 changes: 19 additions & 22 deletions compiler/rustc_mir/src/transform/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use rustc_index::vec::Idx;
use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
use rustc_middle::mir::visit::*;
use rustc_middle::mir::*;
use rustc_middle::ty::subst::Subst;
use rustc_middle::ty::{self, ConstKind, Instance, InstanceDef, ParamEnv, Ty, TyCtxt};
use rustc_span::{hygiene::ExpnKind, ExpnData, Span};
use rustc_target::spec::abi::Abi;
Expand All @@ -28,6 +29,7 @@ pub struct Inline;
#[derive(Copy, Clone, Debug)]
struct CallSite<'tcx> {
callee: Instance<'tcx>,
fn_sig: ty::PolyFnSig<'tcx>,
block: BasicBlock,
target: Option<BasicBlock>,
source_info: SourceInfo,
Expand Down Expand Up @@ -173,22 +175,23 @@ impl Inliner<'tcx> {

// Only consider direct calls to functions
let terminator = bb_data.terminator();
if let TerminatorKind::Call { func: ref op, ref destination, .. } = terminator.kind {
if let ty::FnDef(callee_def_id, substs) = *op.ty(caller_body, self.tcx).kind() {
// To resolve an instance its substs have to be fully normalized, so
// we do this here.
let normalized_substs = self.tcx.normalize_erasing_regions(self.param_env, substs);
if let TerminatorKind::Call { ref func, ref destination, .. } = terminator.kind {
let func_ty = func.ty(caller_body, self.tcx);
if let ty::FnDef(def_id, substs) = *func_ty.kind() {
// To resolve an instance its substs have to be fully normalized.
let substs = self.tcx.normalize_erasing_regions(self.param_env, substs);
let callee =
Instance::resolve(self.tcx, self.param_env, callee_def_id, normalized_substs)
.ok()
.flatten()?;
Instance::resolve(self.tcx, self.param_env, def_id, substs).ok().flatten()?;

if let InstanceDef::Virtual(..) | InstanceDef::Intrinsic(_) = callee.def {
return None;
}

let fn_sig = self.tcx.fn_sig(def_id).subst(self.tcx, substs);

return Some(CallSite {
callee,
fn_sig,
block: bb,
target: destination.map(|(_, target)| target),
source_info: terminator.source_info,
Expand All @@ -203,9 +206,8 @@ impl Inliner<'tcx> {
debug!("should_inline({:?})", callsite);
let tcx = self.tcx;

// Cannot inline generators which haven't been transformed yet
if callee_body.yield_ty.is_some() {
debug!(" yield ty present - not inlining");
if callsite.fn_sig.c_variadic() {
debug!("callee is variadic - not inlining");
return false;
}

Expand All @@ -218,11 +220,7 @@ impl Inliner<'tcx> {
return false;
}

let self_no_sanitize =
self.codegen_fn_attrs.no_sanitize & self.tcx.sess.opts.debugging_opts.sanitizer;
let callee_no_sanitize =
codegen_fn_attrs.no_sanitize & self.tcx.sess.opts.debugging_opts.sanitizer;
if self_no_sanitize != callee_no_sanitize {
if self.codegen_fn_attrs.no_sanitize != codegen_fn_attrs.no_sanitize {
debug!("`callee has incompatible no_sanitize attribute - not inlining");
return false;
}
Expand Down Expand Up @@ -256,9 +254,9 @@ impl Inliner<'tcx> {
self.tcx.sess.opts.debugging_opts.inline_mir_threshold
};

// Significantly lower the threshold for inlining cold functions
if codegen_fn_attrs.flags.contains(CodegenFnAttrFlags::COLD) {
threshold /= 5;
debug!("#[cold] present - not inlining");
return false;
}

// Give a bonus functions with a small number of blocks,
Expand Down Expand Up @@ -447,7 +445,7 @@ impl Inliner<'tcx> {
};

// Copy the arguments if needed.
let args: Vec<_> = self.make_call_args(args, &callsite, caller_body);
let args: Vec<_> = self.make_call_args(args, &callsite, caller_body, &callee_body);

let mut integrator = Integrator {
args: &args,
Expand Down Expand Up @@ -528,6 +526,7 @@ impl Inliner<'tcx> {
args: Vec<Operand<'tcx>>,
callsite: &CallSite<'tcx>,
caller_body: &mut Body<'tcx>,
callee_body: &Body<'tcx>,
) -> Vec<Local> {
let tcx = self.tcx;

Expand All @@ -554,9 +553,7 @@ impl Inliner<'tcx> {
// tmp2 = tuple_tmp.2
//
// and the vector is `[closure_ref, tmp0, tmp1, tmp2]`.
// FIXME(eddyb) make this check for `"rust-call"` ABI combined with
// `callee_body.spread_arg == None`, instead of special-casing closures.
if tcx.is_closure(callsite.callee.def_id()) {
if callsite.fn_sig.abi() == Abi::RustCall && callee_body.spread_arg.is_none() {
let mut args = args.into_iter();
let self_ = self.create_temp_if_necessary(args.next().unwrap(), callsite, caller_body);
let tuple = self.create_temp_if_necessary(args.next().unwrap(), callsite, caller_body);
Expand Down
22 changes: 19 additions & 3 deletions src/test/mir-opt/inline/inline-compatibility.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
// Checks that only functions with compatible attributes are inlined.
//
// only-x86_64
// needs-sanitizer-address
// compile-flags: -Zsanitizer=address

#![crate_type = "lib"]
#![feature(no_sanitize)]
#![feature(target_feature_11)]
#![feature(c_variadic)]

// EMIT_MIR inline_compatibility.inlined_target_feature.Inline.diff
#[target_feature(enable = "sse2")]
Expand Down Expand Up @@ -35,5 +34,22 @@ pub unsafe fn not_inlined_no_sanitize() {
pub unsafe fn target_feature() {}

#[inline]
#[no_sanitize(address, memory)]
#[no_sanitize(address)]
pub unsafe fn no_sanitize() {}

// EMIT_MIR inline_compatibility.not_inlined_c_variadic.Inline.diff
pub unsafe fn not_inlined_c_variadic() {
let s = sum(4u32, 4u32, 30u32, 200u32, 1000u32);
}

#[no_mangle]
#[inline(always)]
unsafe extern "C" fn sum(n: u32, mut vs: ...) -> u32 {
let mut s = 0;
let mut i = 0;
while i != n {
s += vs.arg::<u32>();
i += 1;
}
s
}
16 changes: 16 additions & 0 deletions src/test/mir-opt/inline/inline-generator.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// ignore-wasm32-bare compiled with panic=abort by default
#![feature(generators, generator_trait)]

use std::ops::Generator;
use std::pin::Pin;

// EMIT_MIR inline_generator.main.Inline.diff
fn main() {
let _r = Pin::new(&mut g()).resume(false);
}

#[inline(always)]
pub fn g() -> impl Generator<bool> {
#[inline(always)]
|a| { yield if a { 7 } else { 13 } }
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,24 @@
+ // MIR for `inlined_no_sanitize` after Inline

fn inlined_no_sanitize() -> () {
let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:24:37: 24:37
let _1: (); // in scope 0 at $DIR/inline-compatibility.rs:25:5: 25:18
+ scope 1 (inlined no_sanitize) { // at $DIR/inline-compatibility.rs:25:5: 25:18
let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:23:37: 23:37
let _1: (); // in scope 0 at $DIR/inline-compatibility.rs:24:5: 24:18
+ scope 1 (inlined no_sanitize) { // at $DIR/inline-compatibility.rs:24:5: 24:18
+ }

bb0: {
StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:25:5: 25:18
- _1 = no_sanitize() -> bb1; // scope 0 at $DIR/inline-compatibility.rs:25:5: 25:18
StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:24:5: 24:18
- _1 = no_sanitize() -> bb1; // scope 0 at $DIR/inline-compatibility.rs:24:5: 24:18
- // mir::Constant
- // + span: $DIR/inline-compatibility.rs:25:5: 25:16
- // + span: $DIR/inline-compatibility.rs:24:5: 24:16
- // + literal: Const { ty: unsafe fn() {no_sanitize}, val: Value(Scalar(<ZST>)) }
- }
-
- bb1: {
+ _1 = const (); // scope 1 at $DIR/inline-compatibility.rs:25:5: 25:18
StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:25:18: 25:19
_0 = const (); // scope 0 at $DIR/inline-compatibility.rs:24:37: 26:2
return; // scope 0 at $DIR/inline-compatibility.rs:26:2: 26:2
+ _1 = const (); // scope 1 at $DIR/inline-compatibility.rs:24:5: 24:18
StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:24:18: 24:19
_0 = const (); // scope 0 at $DIR/inline-compatibility.rs:23:37: 25:2
return; // scope 0 at $DIR/inline-compatibility.rs:25:2: 25:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,24 @@
+ // MIR for `inlined_target_feature` after Inline

fn inlined_target_feature() -> () {
let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:13:40: 13:40
let _1: (); // in scope 0 at $DIR/inline-compatibility.rs:14:5: 14:21
+ scope 1 (inlined target_feature) { // at $DIR/inline-compatibility.rs:14:5: 14:21
let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:12:40: 12:40
let _1: (); // in scope 0 at $DIR/inline-compatibility.rs:13:5: 13:21
+ scope 1 (inlined target_feature) { // at $DIR/inline-compatibility.rs:13:5: 13:21
+ }

bb0: {
StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:14:5: 14:21
- _1 = target_feature() -> bb1; // scope 0 at $DIR/inline-compatibility.rs:14:5: 14:21
StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:13:5: 13:21
- _1 = target_feature() -> bb1; // scope 0 at $DIR/inline-compatibility.rs:13:5: 13:21
- // mir::Constant
- // + span: $DIR/inline-compatibility.rs:14:5: 14:19
- // + span: $DIR/inline-compatibility.rs:13:5: 13:19
- // + literal: Const { ty: unsafe fn() {target_feature}, val: Value(Scalar(<ZST>)) }
- }
-
- bb1: {
+ _1 = const (); // scope 1 at $DIR/inline-compatibility.rs:14:5: 14:21
StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:14:21: 14:22
_0 = const (); // scope 0 at $DIR/inline-compatibility.rs:13:40: 15:2
return; // scope 0 at $DIR/inline-compatibility.rs:15:2: 15:2
+ _1 = const (); // scope 1 at $DIR/inline-compatibility.rs:13:5: 13:21
StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:13:21: 13:22
_0 = const (); // scope 0 at $DIR/inline-compatibility.rs:12:40: 14:2
return; // scope 0 at $DIR/inline-compatibility.rs:14:2: 14:2
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
- // MIR for `not_inlined_c_variadic` before Inline
+ // MIR for `not_inlined_c_variadic` after Inline

fn not_inlined_c_variadic() -> () {
let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:41:40: 41:40
let _1: u32; // in scope 0 at $DIR/inline-compatibility.rs:42:9: 42:10
scope 1 {
debug s => _1; // in scope 1 at $DIR/inline-compatibility.rs:42:9: 42:10
}

bb0: {
StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:42:9: 42:10
_1 = sum(const 4_u32, const 4_u32, const 30_u32, const 200_u32, const 1000_u32) -> bb1; // scope 0 at $DIR/inline-compatibility.rs:42:13: 42:52
// mir::Constant
// + span: $DIR/inline-compatibility.rs:42:13: 42:16
// + literal: Const { ty: unsafe extern "C" fn(u32, ...) -> u32 {sum}, val: Value(Scalar(<ZST>)) }
}

bb1: {
_0 = const (); // scope 0 at $DIR/inline-compatibility.rs:41:40: 43:2
StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:43:1: 43:2
return; // scope 0 at $DIR/inline-compatibility.rs:43:2: 43:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,21 @@
+ // MIR for `not_inlined_no_sanitize` after Inline

fn not_inlined_no_sanitize() -> () {
let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:29:41: 29:41
let _1: (); // in scope 0 at $DIR/inline-compatibility.rs:30:5: 30:18
let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:28:41: 28:41
let _1: (); // in scope 0 at $DIR/inline-compatibility.rs:29:5: 29:18

bb0: {
StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:30:5: 30:18
_1 = no_sanitize() -> bb1; // scope 0 at $DIR/inline-compatibility.rs:30:5: 30:18
StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:29:5: 29:18
_1 = no_sanitize() -> bb1; // scope 0 at $DIR/inline-compatibility.rs:29:5: 29:18
// mir::Constant
// + span: $DIR/inline-compatibility.rs:30:5: 30:16
// + span: $DIR/inline-compatibility.rs:29:5: 29:16
// + literal: Const { ty: unsafe fn() {no_sanitize}, val: Value(Scalar(<ZST>)) }
}

bb1: {
StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:30:18: 30:19
_0 = const (); // scope 0 at $DIR/inline-compatibility.rs:29:41: 31:2
return; // scope 0 at $DIR/inline-compatibility.rs:31:2: 31:2
StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:29:18: 29:19
_0 = const (); // scope 0 at $DIR/inline-compatibility.rs:28:41: 30:2
return; // scope 0 at $DIR/inline-compatibility.rs:30:2: 30:2
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,21 @@
+ // MIR for `not_inlined_target_feature` after Inline

fn not_inlined_target_feature() -> () {
let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:18:44: 18:44
let _1: (); // in scope 0 at $DIR/inline-compatibility.rs:19:5: 19:21
let mut _0: (); // return place in scope 0 at $DIR/inline-compatibility.rs:17:44: 17:44
let _1: (); // in scope 0 at $DIR/inline-compatibility.rs:18:5: 18:21

bb0: {
StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:19:5: 19:21
_1 = target_feature() -> bb1; // scope 0 at $DIR/inline-compatibility.rs:19:5: 19:21
StorageLive(_1); // scope 0 at $DIR/inline-compatibility.rs:18:5: 18:21
_1 = target_feature() -> bb1; // scope 0 at $DIR/inline-compatibility.rs:18:5: 18:21
// mir::Constant
// + span: $DIR/inline-compatibility.rs:19:5: 19:19
// + span: $DIR/inline-compatibility.rs:18:5: 18:19
// + literal: Const { ty: unsafe fn() {target_feature}, val: Value(Scalar(<ZST>)) }
}

bb1: {
StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:19:21: 19:22
_0 = const (); // scope 0 at $DIR/inline-compatibility.rs:18:44: 20:2
return; // scope 0 at $DIR/inline-compatibility.rs:20:2: 20:2
StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:18:21: 18:22
_0 = const (); // scope 0 at $DIR/inline-compatibility.rs:17:44: 19:2
return; // scope 0 at $DIR/inline-compatibility.rs:19:2: 19:2
}
}

Loading