Skip to content

Commit

Permalink
Rollup merge of #78966 - tmiasko:inline-never, r=oli-obk
Browse files Browse the repository at this point in the history
Never inline C variadics, cold functions, functions with incompatible attributes ...

... and fix generator inlining.

Closes #67863.
Closes #78859.
  • Loading branch information
Dylan-DPC authored Nov 15, 2020
2 parents 335a255 + 2a010dd commit 6be44ed
Show file tree
Hide file tree
Showing 9 changed files with 238 additions and 61 deletions.
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

0 comments on commit 6be44ed

Please sign in to comment.