inliner: Break inlining cycles

When examining candidates for inlining, reject those that are determined
to be recursive either because of self-recursive calls or calls to any
instances already inlined.
This commit is contained in:
Tomasz Miąsko 2020-11-09 00:00:00 +00:00
parent b7f16c56d1
commit dc4d74d149
4 changed files with 243 additions and 107 deletions

View file

@ -1,6 +1,7 @@
//! Inlining pass for MIR functions
use rustc_attr as attr;
use rustc_hir as hir;
use rustc_index::bit_set::BitSet;
use rustc_index::vec::Idx;
use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs};
@ -12,9 +13,8 @@ use rustc_target::spec::abi::Abi;
use super::simplify::{remove_dead_blocks, CfgSimplifier};
use crate::transform::MirPass;
use std::collections::VecDeque;
use std::iter;
use std::ops::RangeFrom;
use std::ops::{Range, RangeFrom};
const DEFAULT_THRESHOLD: usize = 50;
const HINT_THRESHOLD: usize = 100;
@ -37,127 +37,128 @@ struct CallSite<'tcx> {
impl<'tcx> MirPass<'tcx> for Inline {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
if tcx.sess.opts.debugging_opts.mir_opt_level >= 2 {
if tcx.sess.opts.debugging_opts.instrument_coverage {
// The current implementation of source code coverage injects code region counters
// into the MIR, and assumes a 1-to-1 correspondence between MIR and source-code-
// based function.
debug!("function inlining is disabled when compiling with `instrument_coverage`");
} else {
Inliner {
tcx,
param_env: tcx.param_env_reveal_all_normalized(body.source.def_id()),
codegen_fn_attrs: tcx.codegen_fn_attrs(body.source.def_id()),
}
.run_pass(body);
}
if tcx.sess.opts.debugging_opts.mir_opt_level < 2 {
return;
}
if tcx.sess.opts.debugging_opts.instrument_coverage {
// The current implementation of source code coverage injects code region counters
// into the MIR, and assumes a 1-to-1 correspondence between MIR and source-code-
// based function.
debug!("function inlining is disabled when compiling with `instrument_coverage`");
return;
}
if inline(tcx, body) {
debug!("running simplify cfg on {:?}", body.source);
CfgSimplifier::new(body).simplify();
remove_dead_blocks(body);
}
}
}
fn inline(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) -> bool {
let def_id = body.source.def_id();
let hir_id = tcx.hir().local_def_id_to_hir_id(def_id.expect_local());
// Only do inlining into fn bodies.
if !tcx.hir().body_owner_kind(hir_id).is_fn_or_closure() {
return false;
}
if body.source.promoted.is_some() {
return false;
}
let mut this = Inliner {
tcx,
param_env: tcx.param_env_reveal_all_normalized(body.source.def_id()),
codegen_fn_attrs: tcx.codegen_fn_attrs(body.source.def_id()),
hir_id,
history: Vec::new(),
changed: false,
};
let blocks = BasicBlock::new(0)..body.basic_blocks().next_index();
this.process_blocks(body, blocks);
this.changed
}
struct Inliner<'tcx> {
tcx: TyCtxt<'tcx>,
param_env: ParamEnv<'tcx>,
/// Caller codegen attributes.
codegen_fn_attrs: &'tcx CodegenFnAttrs,
/// Caller HirID.
hir_id: hir::HirId,
/// Stack of inlined instances.
history: Vec<Instance<'tcx>>,
/// Indicates that the caller body has been modified.
changed: bool,
}
impl Inliner<'tcx> {
fn run_pass(&self, caller_body: &mut Body<'tcx>) {
// Keep a queue of callsites to try inlining on. We take
// advantage of the fact that queries detect cycles here to
// allow us to try and fetch the fully optimized MIR of a
// call; if it succeeds, we can inline it and we know that
// they do not call us. Otherwise, we just don't try to
// inline.
//
// We use a queue so that we inline "broadly" before we inline
// in depth. It is unclear if this is the best heuristic,
// really, but that's true of all the heuristics in this
// file. =)
let mut callsites = VecDeque::new();
let def_id = caller_body.source.def_id();
// Only do inlining into fn bodies.
let self_hir_id = self.tcx.hir().local_def_id_to_hir_id(def_id.expect_local());
if self.tcx.hir().body_owner_kind(self_hir_id).is_fn_or_closure()
&& caller_body.source.promoted.is_none()
{
for (bb, bb_data) in caller_body.basic_blocks().iter_enumerated() {
if let Some(callsite) = self.get_valid_function_call(bb, bb_data, caller_body) {
callsites.push_back(callsite);
}
}
} else {
return;
}
let mut changed = false;
while let Some(callsite) = callsites.pop_front() {
debug!("checking whether to inline callsite {:?}", callsite);
if let InstanceDef::Item(_) = callsite.callee.def {
if !self.tcx.is_mir_available(callsite.callee.def_id()) {
debug!("checking whether to inline callsite {:?} - MIR unavailable", callsite,);
continue;
}
}
let callee_body = if let Some(callee_def_id) = callsite.callee.def_id().as_local() {
let callee_hir_id = self.tcx.hir().local_def_id_to_hir_id(callee_def_id);
// Avoid a cycle here by only using `instance_mir` only if we have
// a lower `HirId` than the callee. This ensures that the callee will
// not inline us. This trick only works without incremental compilation.
// So don't do it if that is enabled. Also avoid inlining into generators,
// since their `optimized_mir` is used for layout computation, which can
// create a cycle, even when no attempt is made to inline the function
// in the other direction.
if !self.tcx.dep_graph.is_fully_enabled()
&& self_hir_id < callee_hir_id
&& caller_body.generator_kind.is_none()
{
self.tcx.instance_mir(callsite.callee.def)
} else {
continue;
}
} else {
// This cannot result in a cycle since the callee MIR is from another crate
// and is already optimized.
self.tcx.instance_mir(callsite.callee.def)
fn process_blocks(&mut self, caller_body: &mut Body<'tcx>, blocks: Range<BasicBlock>) {
for bb in blocks {
let callsite = match self.get_valid_function_call(bb, &caller_body[bb], caller_body) {
None => continue,
Some(it) => it,
};
if !self.consider_optimizing(callsite, &callee_body) {
if !self.is_mir_available(&callsite.callee, caller_body) {
debug!("MIR unavailable {}", callsite.callee);
continue;
}
let callee_body = self.tcx.instance_mir(callsite.callee.def);
if !self.should_inline(callsite, callee_body) {
continue;
}
if !self.tcx.consider_optimizing(|| {
format!("Inline {:?} into {}", callee_body.span, callsite.callee)
}) {
return;
}
let callee_body = callsite.callee.subst_mir_and_normalize_erasing_regions(
self.tcx,
self.param_env,
callee_body,
);
let start = caller_body.basic_blocks().len();
let old_blocks = caller_body.basic_blocks().next_index();
self.inline_call(callsite, caller_body, callee_body);
let new_blocks = old_blocks..caller_body.basic_blocks().next_index();
self.changed = true;
// Add callsites from inlined function
for (bb, bb_data) in caller_body.basic_blocks().iter_enumerated().skip(start) {
if let Some(new_callsite) = self.get_valid_function_call(bb, bb_data, caller_body) {
// Don't inline the same function multiple times.
if callsite.callee != new_callsite.callee {
callsites.push_back(new_callsite);
}
}
self.history.push(callsite.callee);
self.process_blocks(caller_body, new_blocks);
self.history.pop();
}
}
fn is_mir_available(&self, callee: &Instance<'tcx>, caller_body: &Body<'tcx>) -> bool {
if let InstanceDef::Item(_) = callee.def {
if !self.tcx.is_mir_available(callee.def_id()) {
return false;
}
changed = true;
}
// Simplify if we inlined anything.
if changed {
debug!("running simplify cfg on {:?}", caller_body.source);
CfgSimplifier::new(caller_body).simplify();
remove_dead_blocks(caller_body);
if let Some(callee_def_id) = callee.def_id().as_local() {
let callee_hir_id = self.tcx.hir().local_def_id_to_hir_id(callee_def_id);
// Avoid a cycle here by only using `instance_mir` only if we have
// a lower `HirId` than the callee. This ensures that the callee will
// not inline us. This trick only works without incremental compilation.
// So don't do it if that is enabled. Also avoid inlining into generators,
// since their `optimized_mir` is used for layout computation, which can
// create a cycle, even when no attempt is made to inline the function
// in the other direction.
!self.tcx.dep_graph.is_fully_enabled()
&& self.hir_id < callee_hir_id
&& caller_body.generator_kind.is_none()
} else {
// This cannot result in a cycle since the callee MIR is from another crate
// and is already optimized.
true
}
}
@ -196,14 +197,6 @@ impl Inliner<'tcx> {
None
}
fn consider_optimizing(&self, callsite: CallSite<'tcx>, callee_body: &Body<'tcx>) -> bool {
debug!("consider_optimizing({:?})", callsite);
self.should_inline(callsite, callee_body)
&& self.tcx.consider_optimizing(|| {
format!("Inline {:?} into {:?}", callee_body.span, callsite)
})
}
fn should_inline(&self, callsite: CallSite<'tcx>, callee_body: &Body<'tcx>) -> bool {
debug!("should_inline({:?})", callsite);
let tcx = self.tcx;
@ -323,7 +316,18 @@ impl Inliner<'tcx> {
}
TerminatorKind::Call { func: Operand::Constant(ref f), cleanup, .. } => {
if let ty::FnDef(def_id, _) = *f.literal.ty.kind() {
if let ty::FnDef(def_id, substs) =
*callsite.callee.subst_mir(self.tcx, &f.literal.ty).kind()
{
let substs = self.tcx.normalize_erasing_regions(self.param_env, substs);
if let Ok(Some(instance)) =
Instance::resolve(self.tcx, self.param_env, def_id, substs)
{
if callsite.callee == instance || self.history.contains(&instance) {
debug!("`callee is recursive - not inlining");
return false;
}
}
// Don't give intrinsics the extra penalty for calls
let f = tcx.fn_sig(def_id);
if f.abi() == Abi::RustIntrinsic || f.abi() == Abi::PlatformIntrinsic {
@ -397,8 +401,6 @@ impl Inliner<'tcx> {
let terminator = caller_body[callsite.bb].terminator.take().unwrap();
match terminator.kind {
TerminatorKind::Call { args, destination: Some(destination), cleanup, .. } => {
debug!("inlined {} into {:?}", callsite.callee, caller_body.source.instance);
// If the call is something like `a[*i] = f(i)`, where
// `i : &mut usize`, then just duplicating the `a[*i]`
// Place could result in two different locations if `f`

View file

@ -0,0 +1,60 @@
// Check that inliner handles various forms of recursion and doesn't fall into
// an infinite inlining cycle. The particular outcome of inlining is not
// crucial otherwise.
//
// Regression test for issue #78573.
fn main() {
one();
two();
}
// EMIT_MIR inline_cycle.one.Inline.diff
fn one() {
<C as Call>::call();
}
pub trait Call {
fn call();
}
pub struct A<T>(T);
pub struct B<T>(T);
pub struct C;
impl<T: Call> Call for A<T> {
#[inline]
fn call() {
<B<T> as Call>::call()
}
}
impl<T: Call> Call for B<T> {
#[inline]
fn call() {
<T as Call>::call()
}
}
impl Call for C {
#[inline]
fn call() {
A::<C>::call()
}
}
// EMIT_MIR inline_cycle.two.Inline.diff
fn two() {
call(f);
}
#[inline]
fn call<F: FnOnce()>(f: F) {
f();
}
#[inline]
fn f() {
call(f);
}

View file

@ -0,0 +1,27 @@
- // MIR for `one` before Inline
+ // MIR for `one` after Inline
fn one() -> () {
let mut _0: (); // return place in scope 0 at $DIR/inline-cycle.rs:13:10: 13:10
let _1: (); // in scope 0 at $DIR/inline-cycle.rs:14:5: 14:24
+ scope 1 (inlined <C as Call>::call) { // at $DIR/inline-cycle.rs:14:5: 14:24
+ }
bb0: {
StorageLive(_1); // scope 0 at $DIR/inline-cycle.rs:14:5: 14:24
- _1 = <C as Call>::call() -> bb1; // scope 0 at $DIR/inline-cycle.rs:14:5: 14:24
+ _1 = <A<C> as Call>::call() -> bb1; // scope 1 at $DIR/inline-cycle.rs:14:5: 14:24
// mir::Constant
- // + span: $DIR/inline-cycle.rs:14:5: 14:22
- // + literal: Const { ty: fn() {<C as Call>::call}, val: Value(Scalar(<ZST>)) }
+ // + span: $DIR/inline-cycle.rs:14:5: 14:24
+ // + literal: Const { ty: fn() {<A<C> as Call>::call}, val: Value(Scalar(<ZST>)) }
}
bb1: {
StorageDead(_1); // scope 0 at $DIR/inline-cycle.rs:14:24: 14:25
_0 = const (); // scope 0 at $DIR/inline-cycle.rs:13:10: 15:2
return; // scope 0 at $DIR/inline-cycle.rs:15:2: 15:2
}
}

View file

@ -0,0 +1,47 @@
- // MIR for `two` before Inline
+ // MIR for `two` after Inline
fn two() -> () {
let mut _0: (); // return place in scope 0 at $DIR/inline-cycle.rs:48:10: 48:10
let _1: (); // in scope 0 at $DIR/inline-cycle.rs:49:5: 49:12
+ let mut _2: fn() {f}; // in scope 0 at $DIR/inline-cycle.rs:49:5: 49:12
+ let mut _5: (); // in scope 0 at $DIR/inline-cycle.rs:49:5: 49:12
+ scope 1 (inlined call::<fn() {f}>) { // at $DIR/inline-cycle.rs:49:5: 49:12
+ debug f => _2; // in scope 1 at $DIR/inline-cycle.rs:49:5: 49:12
+ let _3: (); // in scope 1 at $DIR/inline-cycle.rs:49:5: 49:12
+ let mut _4: fn() {f}; // in scope 1 at $DIR/inline-cycle.rs:49:5: 49:12
+ scope 2 (inlined <fn() {f} as FnOnce<()>>::call_once - shim(fn() {f})) { // at $DIR/inline-cycle.rs:49:5: 49:12
+ }
+ }
bb0: {
StorageLive(_1); // scope 0 at $DIR/inline-cycle.rs:49:5: 49:12
- _1 = call::<fn() {f}>(f) -> bb1; // scope 0 at $DIR/inline-cycle.rs:49:5: 49:12
+ StorageLive(_2); // scope 0 at $DIR/inline-cycle.rs:49:5: 49:12
+ _2 = f; // scope 0 at $DIR/inline-cycle.rs:49:5: 49:12
// mir::Constant
- // + span: $DIR/inline-cycle.rs:49:5: 49:9
- // + literal: Const { ty: fn(fn() {f}) {call::<fn() {f}>}, val: Value(Scalar(<ZST>)) }
- // mir::Constant
// + span: $DIR/inline-cycle.rs:49:10: 49:11
// + literal: Const { ty: fn() {f}, val: Value(Scalar(<ZST>)) }
+ StorageLive(_3); // scope 1 at $DIR/inline-cycle.rs:49:5: 49:12
+ StorageLive(_4); // scope 1 at $DIR/inline-cycle.rs:49:5: 49:12
+ _4 = move _2; // scope 1 at $DIR/inline-cycle.rs:49:5: 49:12
+ StorageLive(_5); // scope 1 at $DIR/inline-cycle.rs:49:5: 49:12
+ _5 = const (); // scope 1 at $DIR/inline-cycle.rs:49:5: 49:12
+ _3 = move _4() -> bb1; // scope 2 at $DIR/inline-cycle.rs:49:5: 49:12
}
bb1: {
+ StorageDead(_5); // scope 1 at $DIR/inline-cycle.rs:49:5: 49:12
+ StorageDead(_4); // scope 1 at $DIR/inline-cycle.rs:49:5: 49:12
+ StorageDead(_3); // scope 1 at $DIR/inline-cycle.rs:49:5: 49:12
+ _1 = const (); // scope 1 at $DIR/inline-cycle.rs:49:5: 49:12
+ StorageDead(_2); // scope 0 at $DIR/inline-cycle.rs:49:5: 49:12
StorageDead(_1); // scope 0 at $DIR/inline-cycle.rs:49:12: 49:13
_0 = const (); // scope 0 at $DIR/inline-cycle.rs:48:10: 50:2
return; // scope 0 at $DIR/inline-cycle.rs:50:2: 50:2
}
}