Skip to content

Commit

Permalink
Auto merge of rust-lang#105119 - JakobDegen:inline-experiments, r=cjg…
Browse files Browse the repository at this point in the history
…illot

Disable top down MIR inlining

The current MIR inliner has exponential behavior in some cases: <https://godbolt.org/z/7jnWah4fE>. The cause of this is top-down inlining, where we repeatedly do inlining like `call_a() => { call_b(); call_b(); }`. Each decision on its own seems to make sense, but the result is exponential.

Disabling top-down inlining fundamentally prevents this. Each call site in the original, unoptimized source code is now considered for inlining exactly one time, which means that the total growth in MIR size is limited to number of call sites * inlining threshold.

Top down inlining may be worth re-introducing at some point, but it needs to be accompanied with a principled way to prevent this kind of behavior.
  • Loading branch information
bors committed Dec 6, 2022
2 parents 8e440b0 + f4f7777 commit 226202d
Show file tree
Hide file tree
Showing 10 changed files with 243 additions and 115 deletions.
28 changes: 6 additions & 22 deletions compiler/rustc_mir_transform/src/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ use rustc_middle::mir::visit::*;
use rustc_middle::mir::*;
use rustc_middle::ty::{self, Instance, InstanceDef, ParamEnv, Ty, TyCtxt};
use rustc_session::config::OptLevel;
use rustc_span::def_id::DefId;
use rustc_span::{hygiene::ExpnKind, ExpnData, LocalExpnId, Span};
use rustc_target::abi::VariantIdx;
use rustc_target::spec::abi::Abi;
Expand Down Expand Up @@ -87,13 +86,8 @@ fn inline<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) -> bool {

let param_env = tcx.param_env_reveal_all_normalized(def_id);

let mut this = Inliner {
tcx,
param_env,
codegen_fn_attrs: tcx.codegen_fn_attrs(def_id),
history: Vec::new(),
changed: false,
};
let mut this =
Inliner { tcx, param_env, codegen_fn_attrs: tcx.codegen_fn_attrs(def_id), changed: false };
let blocks = BasicBlock::new(0)..body.basic_blocks.next_index();
this.process_blocks(body, blocks);
this.changed
Expand All @@ -104,12 +98,6 @@ struct Inliner<'tcx> {
param_env: ParamEnv<'tcx>,
/// Caller codegen attributes.
codegen_fn_attrs: &'tcx CodegenFnAttrs,
/// Stack of inlined instances.
/// We only check the `DefId` and not the substs because we want to
/// avoid inlining cases of polymorphic recursion.
/// The number of `DefId`s is finite, so checking history is enough
/// to ensure that we do not loop endlessly while inlining.
history: Vec<DefId>,
/// Indicates that the caller body has been modified.
changed: bool,
}
Expand All @@ -134,12 +122,12 @@ impl<'tcx> Inliner<'tcx> {
debug!("not-inlined {} [{}]", callsite.callee, reason);
continue;
}
Ok(new_blocks) => {
Ok(_) => {
debug!("inlined {}", callsite.callee);
self.changed = true;
self.history.push(callsite.callee.def_id());
self.process_blocks(caller_body, new_blocks);
self.history.pop();
// We could process the blocks returned by `try_inlining` here. However, that
// leads to exponential compile times due to the top-down nature of this kind
// of inlining.
}
}
}
Expand Down Expand Up @@ -313,10 +301,6 @@ impl<'tcx> Inliner<'tcx> {
return None;
}

if self.history.contains(&callee.def_id()) {
return None;
}

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

return Some(CallSite {
Expand Down
23 changes: 12 additions & 11 deletions src/test/mir-opt/inline/cycle.g.Inline.diff
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
+ let _3: (); // in scope 1 at $DIR/cycle.rs:6:5: 6:8
+ let mut _4: &fn() {main}; // in scope 1 at $DIR/cycle.rs:6:5: 6:6
+ let mut _5: (); // in scope 1 at $DIR/cycle.rs:6:5: 6:8
+ scope 2 (inlined <fn() {main} as Fn<()>>::call - shim(fn() {main})) { // at $DIR/cycle.rs:6:5: 6:8
+ }
+ }

bb0: {
Expand All @@ -29,7 +27,10 @@
+ StorageLive(_4); // scope 1 at $DIR/cycle.rs:6:5: 6:6
+ _4 = &_2; // scope 1 at $DIR/cycle.rs:6:5: 6:6
+ StorageLive(_5); // scope 1 at $DIR/cycle.rs:6:5: 6:8
+ _3 = move (*_4)() -> [return: bb4, unwind: bb2]; // scope 2 at $SRC_DIR/core/src/ops/function.rs:LL:COL
+ _3 = <fn() {main} as Fn<()>>::call(move _4, move _5) -> [return: bb2, unwind: bb3]; // scope 1 at $DIR/cycle.rs:6:5: 6:8
+ // mir::Constant
+ // + span: $DIR/cycle.rs:6:5: 6:6
+ // + literal: Const { ty: for<'a> extern "rust-call" fn(&'a fn() {main}, ()) -> <fn() {main} as FnOnce<()>>::Output {<fn() {main} as Fn<()>>::call}, val: Value(<ZST>) }
}

bb1: {
Expand All @@ -39,19 +40,19 @@
return; // scope 0 at $DIR/cycle.rs:+2:2: +2:2
+ }
+
+ bb2 (cleanup): {
+ drop(_2) -> bb3; // scope 1 at $DIR/cycle.rs:7:1: 7:2
+ bb2: {
+ StorageDead(_5); // scope 1 at $DIR/cycle.rs:6:7: 6:8
+ StorageDead(_4); // scope 1 at $DIR/cycle.rs:6:7: 6:8
+ StorageDead(_3); // scope 1 at $DIR/cycle.rs:6:8: 6:9
+ drop(_2) -> bb1; // scope 1 at $DIR/cycle.rs:7:1: 7:2
+ }
+
+ bb3 (cleanup): {
+ resume; // scope 1 at $DIR/cycle.rs:5:1: 7:2
+ drop(_2) -> bb4; // scope 1 at $DIR/cycle.rs:7:1: 7:2
+ }
+
+ bb4: {
+ StorageDead(_5); // scope 1 at $DIR/cycle.rs:6:7: 6:8
+ StorageDead(_4); // scope 1 at $DIR/cycle.rs:6:7: 6:8
+ StorageDead(_3); // scope 1 at $DIR/cycle.rs:6:8: 6:9
+ drop(_2) -> bb1; // scope 1 at $DIR/cycle.rs:7:1: 7:2
+ bb4 (cleanup): {
+ resume; // scope 1 at $DIR/cycle.rs:5:1: 7:2
}
}

40 changes: 12 additions & 28 deletions src/test/mir-opt/inline/cycle.main.Inline.diff
Original file line number Diff line number Diff line change
Expand Up @@ -10,18 +10,6 @@
+ let _3: (); // in scope 1 at $DIR/cycle.rs:6:5: 6:8
+ let mut _4: &fn() {g}; // in scope 1 at $DIR/cycle.rs:6:5: 6:6
+ let mut _5: (); // in scope 1 at $DIR/cycle.rs:6:5: 6:8
+ scope 2 (inlined <fn() {g} as Fn<()>>::call - shim(fn() {g})) { // at $DIR/cycle.rs:6:5: 6:8
+ scope 3 (inlined g) { // at $SRC_DIR/core/src/ops/function.rs:LL:COL
+ let mut _6: fn() {main}; // in scope 3 at $DIR/cycle.rs:12:5: 12:12
+ scope 4 (inlined f::<fn() {main}>) { // at $DIR/cycle.rs:12:5: 12:12
+ debug g => _6; // in scope 4 at $DIR/cycle.rs:5:6: 5:7
+ let _7: (); // in scope 4 at $DIR/cycle.rs:6:5: 6:8
+ let mut _8: &fn() {main}; // in scope 4 at $DIR/cycle.rs:6:5: 6:6
+ scope 5 (inlined <fn() {main} as Fn<()>>::call - shim(fn() {main})) { // at $DIR/cycle.rs:6:5: 6:8
+ }
+ }
+ }
+ }
+ }

bb0: {
Expand All @@ -39,11 +27,10 @@
+ StorageLive(_4); // scope 1 at $DIR/cycle.rs:6:5: 6:6
+ _4 = &_2; // scope 1 at $DIR/cycle.rs:6:5: 6:6
+ StorageLive(_5); // scope 1 at $DIR/cycle.rs:6:5: 6:8
+ StorageLive(_6); // scope 3 at $DIR/cycle.rs:12:5: 12:12
+ StorageLive(_7); // scope 4 at $DIR/cycle.rs:6:5: 6:8
+ StorageLive(_8); // scope 4 at $DIR/cycle.rs:6:5: 6:6
+ _8 = &_6; // scope 4 at $DIR/cycle.rs:6:5: 6:6
+ _7 = move (*_8)() -> [return: bb4, unwind: bb2]; // scope 5 at $SRC_DIR/core/src/ops/function.rs:LL:COL
+ _3 = <fn() {g} as Fn<()>>::call(move _4, move _5) -> [return: bb2, unwind: bb3]; // scope 1 at $DIR/cycle.rs:6:5: 6:8
+ // mir::Constant
+ // + span: $DIR/cycle.rs:6:5: 6:6
+ // + literal: Const { ty: for<'a> extern "rust-call" fn(&'a fn() {g}, ()) -> <fn() {g} as FnOnce<()>>::Output {<fn() {g} as Fn<()>>::call}, val: Value(<ZST>) }
}

bb1: {
Expand All @@ -53,22 +40,19 @@
return; // scope 0 at $DIR/cycle.rs:+2:2: +2:2
+ }
+
+ bb2 (cleanup): {
+ drop(_2) -> bb3; // scope 1 at $DIR/cycle.rs:7:1: 7:2
+ bb2: {
+ StorageDead(_5); // scope 1 at $DIR/cycle.rs:6:7: 6:8
+ StorageDead(_4); // scope 1 at $DIR/cycle.rs:6:7: 6:8
+ StorageDead(_3); // scope 1 at $DIR/cycle.rs:6:8: 6:9
+ drop(_2) -> bb1; // scope 1 at $DIR/cycle.rs:7:1: 7:2
+ }
+
+ bb3 (cleanup): {
+ resume; // scope 1 at $DIR/cycle.rs:5:1: 7:2
+ drop(_2) -> bb4; // scope 1 at $DIR/cycle.rs:7:1: 7:2
+ }
+
+ bb4: {
+ StorageDead(_8); // scope 4 at $DIR/cycle.rs:6:7: 6:8
+ StorageDead(_7); // scope 4 at $DIR/cycle.rs:6:8: 6:9
+ StorageDead(_6); // scope 3 at $DIR/cycle.rs:12:5: 12:12
+ StorageDead(_5); // scope 1 at $DIR/cycle.rs:6:7: 6:8
+ StorageDead(_4); // scope 1 at $DIR/cycle.rs:6:7: 6:8
+ StorageDead(_3); // scope 1 at $DIR/cycle.rs:6:8: 6:9
+ drop(_2) -> bb1; // scope 1 at $DIR/cycle.rs:7:1: 7:2
+ bb4 (cleanup): {
+ resume; // scope 1 at $DIR/cycle.rs:5:1: 7:2
}
}

50 changes: 50 additions & 0 deletions src/test/mir-opt/inline/exponential_runtime.main.Inline.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
- // MIR for `main` before Inline
+ // MIR for `main` after Inline

fn main() -> () {
let mut _0: (); // return place in scope 0 at $DIR/exponential_runtime.rs:+0:11: +0:11
let _1: (); // in scope 0 at $DIR/exponential_runtime.rs:+1:5: +1:22
+ scope 1 (inlined <() as G>::call) { // at $DIR/exponential_runtime.rs:86:5: 86:22
+ let _2: (); // in scope 1 at $DIR/exponential_runtime.rs:73:9: 73:25
+ let _3: (); // in scope 1 at $DIR/exponential_runtime.rs:74:9: 74:25
+ let _4: (); // in scope 1 at $DIR/exponential_runtime.rs:75:9: 75:25
+ }

bb0: {
StorageLive(_1); // scope 0 at $DIR/exponential_runtime.rs:+1:5: +1:22
- _1 = <() as G>::call() -> bb1; // scope 0 at $DIR/exponential_runtime.rs:+1:5: +1:22
+ StorageLive(_2); // scope 1 at $DIR/exponential_runtime.rs:73:9: 73:25
+ _2 = <() as F>::call() -> bb1; // scope 1 at $DIR/exponential_runtime.rs:73:9: 73:25
// mir::Constant
- // + span: $DIR/exponential_runtime.rs:86:5: 86:20
- // + literal: Const { ty: fn() {<() as G>::call}, val: Value(<ZST>) }
+ // + span: $DIR/exponential_runtime.rs:73:9: 73:23
+ // + literal: Const { ty: fn() {<() as F>::call}, val: Value(<ZST>) }
}

bb1: {
+ StorageDead(_2); // scope 1 at $DIR/exponential_runtime.rs:73:25: 73:26
+ StorageLive(_3); // scope 1 at $DIR/exponential_runtime.rs:74:9: 74:25
+ _3 = <() as F>::call() -> bb2; // scope 1 at $DIR/exponential_runtime.rs:74:9: 74:25
+ // mir::Constant
+ // + span: $DIR/exponential_runtime.rs:74:9: 74:23
+ // + literal: Const { ty: fn() {<() as F>::call}, val: Value(<ZST>) }
+ }
+
+ bb2: {
+ StorageDead(_3); // scope 1 at $DIR/exponential_runtime.rs:74:25: 74:26
+ StorageLive(_4); // scope 1 at $DIR/exponential_runtime.rs:75:9: 75:25
+ _4 = <() as F>::call() -> bb3; // scope 1 at $DIR/exponential_runtime.rs:75:9: 75:25
+ // mir::Constant
+ // + span: $DIR/exponential_runtime.rs:75:9: 75:23
+ // + literal: Const { ty: fn() {<() as F>::call}, val: Value(<ZST>) }
+ }
+
+ bb3: {
+ StorageDead(_4); // scope 1 at $DIR/exponential_runtime.rs:75:25: 75:26
StorageDead(_1); // scope 0 at $DIR/exponential_runtime.rs:+1:22: +1:23
_0 = const (); // scope 0 at $DIR/exponential_runtime.rs:+0:11: +2:2
return; // scope 0 at $DIR/exponential_runtime.rs:+2:2: +2:2
}
}

87 changes: 87 additions & 0 deletions src/test/mir-opt/inline/exponential_runtime.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
// Checks that code with exponential runtime does not have exponential behavior in inlining.

trait A {
fn call();
}

trait B {
fn call();
}
impl<T: A> B for T {
#[inline]
fn call() {
<T as A>::call();
<T as A>::call();
<T as A>::call();
}
}

trait C {
fn call();
}
impl<T: B> C for T {
#[inline]
fn call() {
<T as B>::call();
<T as B>::call();
<T as B>::call();
}
}

trait D {
fn call();
}
impl<T: C> D for T {
#[inline]
fn call() {
<T as C>::call();
<T as C>::call();
<T as C>::call();
}
}

trait E {
fn call();
}
impl<T: D> E for T {
#[inline]
fn call() {
<T as D>::call();
<T as D>::call();
<T as D>::call();
}
}

trait F {
fn call();
}
impl<T: E> F for T {
#[inline]
fn call() {
<T as E>::call();
<T as E>::call();
<T as E>::call();
}
}

trait G {
fn call();
}
impl<T: F> G for T {
#[inline]
fn call() {
<T as F>::call();
<T as F>::call();
<T as F>::call();
}
}

impl A for () {
#[inline(never)]
fn call() {}
}

// EMIT_MIR exponential_runtime.main.Inline.diff
fn main() {
<() as G>::call();
}
11 changes: 4 additions & 7 deletions src/test/mir-opt/inline/inline_cycle.one.Inline.diff
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,17 @@
let mut _0: (); // return place in scope 0 at $DIR/inline_cycle.rs:+0:10: +0:10
let _1: (); // in scope 0 at $DIR/inline_cycle.rs:+1:5: +1:24
+ scope 1 (inlined <C as Call>::call) { // at $DIR/inline_cycle.rs:14:5: 14:24
+ scope 2 (inlined <A<C> as Call>::call) { // at $DIR/inline_cycle.rs:43:9: 43:23
+ scope 3 (inlined <B<C> as Call>::call) { // at $DIR/inline_cycle.rs:28:9: 28:31
+ }
+ }
+ }

bb0: {
StorageLive(_1); // scope 0 at $DIR/inline_cycle.rs:+1:5: +1:24
- _1 = <C as Call>::call() -> bb1; // scope 0 at $DIR/inline_cycle.rs:+1:5: +1:24
+ _1 = <C as Call>::call() -> bb1; // scope 3 at $DIR/inline_cycle.rs:36:9: 36:28
+ _1 = <A<C> as Call>::call() -> bb1; // scope 1 at $DIR/inline_cycle.rs:43:9: 43:23
// mir::Constant
- // + span: $DIR/inline_cycle.rs:14:5: 14:22
+ // + span: $DIR/inline_cycle.rs:36:9: 36:26
// + literal: Const { ty: fn() {<C as Call>::call}, val: Value(<ZST>) }
- // + literal: Const { ty: fn() {<C as Call>::call}, val: Value(<ZST>) }
+ // + span: $DIR/inline_cycle.rs:43:9: 43:21
+ // + literal: Const { ty: fn() {<A<C> as Call>::call}, val: Value(<ZST>) }
}

bb1: {
Expand Down
23 changes: 7 additions & 16 deletions src/test/mir-opt/inline/inline_cycle.two.Inline.diff
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,6 @@
+ debug f => _2; // in scope 1 at $DIR/inline_cycle.rs:53:22: 53:23
+ let _3: (); // in scope 1 at $DIR/inline_cycle.rs:54:5: 54:8
+ let mut _4: (); // in scope 1 at $DIR/inline_cycle.rs:54:5: 54:8
+ scope 2 (inlined <fn() {f} as FnOnce<()>>::call_once - shim(fn() {f})) { // at $DIR/inline_cycle.rs:54:5: 54:8
+ scope 3 (inlined f) { // at $SRC_DIR/core/src/ops/function.rs:LL:COL
+ let _5: (); // in scope 3 at $DIR/inline_cycle.rs:59:5: 59:12
+ }
+ }
+ }

bb0: {
Expand All @@ -23,23 +18,19 @@
+ _2 = f; // scope 0 at $DIR/inline_cycle.rs:+1:5: +1:12
// mir::Constant
- // + span: $DIR/inline_cycle.rs:49:5: 49:9
+ // + span: $DIR/inline_cycle.rs:49:10: 49:11
+ // + literal: Const { ty: fn() {f}, val: Value(<ZST>) }
- // + literal: Const { ty: fn(fn() {f}) {call::<fn() {f}>}, val: Value(<ZST>) }
- // mir::Constant
// + span: $DIR/inline_cycle.rs:49:10: 49:11
// + literal: Const { ty: fn() {f}, val: Value(<ZST>) }
+ StorageLive(_3); // scope 1 at $DIR/inline_cycle.rs:54:5: 54:8
+ StorageLive(_4); // scope 1 at $DIR/inline_cycle.rs:54:5: 54:8
+ StorageLive(_5); // scope 3 at $DIR/inline_cycle.rs:59:5: 59:12
+ _5 = call::<fn() {f}>(f) -> bb1; // scope 3 at $DIR/inline_cycle.rs:59:5: 59:12
+ _3 = <fn() {f} as FnOnce<()>>::call_once(move _2, move _4) -> bb1; // scope 1 at $DIR/inline_cycle.rs:54:5: 54:8
+ // mir::Constant
+ // + span: $DIR/inline_cycle.rs:59:5: 59:9
// + literal: Const { ty: fn(fn() {f}) {call::<fn() {f}>}, val: Value(<ZST>) }
// mir::Constant
- // + span: $DIR/inline_cycle.rs:49:10: 49:11
+ // + span: $DIR/inline_cycle.rs:59:10: 59:11
// + literal: Const { ty: fn() {f}, val: Value(<ZST>) }
+ // + span: $DIR/inline_cycle.rs:54:5: 54:6
+ // + literal: Const { ty: extern "rust-call" fn(fn() {f}, ()) -> <fn() {f} as FnOnce<()>>::Output {<fn() {f} as FnOnce<()>>::call_once}, val: Value(<ZST>) }
}

bb1: {
+ StorageDead(_5); // scope 3 at $DIR/inline_cycle.rs:59:12: 59:13
+ StorageDead(_4); // scope 1 at $DIR/inline_cycle.rs:54:7: 54:8
+ StorageDead(_3); // scope 1 at $DIR/inline_cycle.rs:54:8: 54:9
+ StorageDead(_2); // scope 0 at $DIR/inline_cycle.rs:+1:5: +1:12
Expand Down
Loading

0 comments on commit 226202d

Please sign in to comment.