Auto merge of rust-lang#112294 - saethlin:inline-me-maybe, r=oli-obk

Ignore the always part of #[inline(always)] in MIR inlining `#[inline(always)]` is used in two cases: for functions that are so trivial it is always profitable to inline them, but also for functions which LLVM thinks are a bad inlining candidate, but which actually turn out to be profitable to inline. That second justification doesn't apply to the MIR inliner, so ignoring our cost estimation for these functions is not necessarily the right right thing to do. This is basically a wash on non-check runs and a perf benefit in check runs. There are some notable regressions, and I think we might be able to claw those back by turning `#[inline(always)]` into a stronger hint. But I think this PR stands decently on its own as a tidy simplification.
rust-lang-ci · Jun 16, 2023 · 6bba061 · 6bba061
2 parents 6a94e87 + c153f3a
commit 6bba061
Show file tree

Hide file tree

Showing 19 changed files with 349 additions and 934 deletions.
diff --git a/compiler/rustc_mir_transform/src/inline.rs b/compiler/rustc_mir_transform/src/inline.rs
@@ -479,11 +479,12 @@ impl<'tcx> Inliner<'tcx> {
         // Abort if type validation found anything fishy.
         checker.validation?;
 
+        // N.B. We still apply our cost threshold to #[inline(always)] functions.
+        // That attribute is often applied to very large functions that exceed LLVM's (very
+        // generous) inlining threshold. Such functions are very poor MIR inlining candidates.
+        // Always inlining #[inline(always)] functions in MIR, on net, slows down the compiler.
         let cost = checker.cost;
-        if let InlineAttr::Always = callee_attrs.inline {
-            debug!("INLINING {:?} because inline(always) [cost={}]", callsite, cost);
-            Ok(())
-        } else if cost <= threshold {
+        if cost <= threshold {
             debug!("INLINING {:?} [cost={} <= threshold={}]", callsite, cost, threshold);
             Ok(())
         } else {

diff --git a/src/tools/miri/tests/fail/terminate-terminator.rs b/src/tools/miri/tests/fail/terminate-terminator.rs
@@ -1,4 +1,4 @@
-//@compile-flags: -Zmir-opt-level=3
+//@compile-flags: -Zmir-opt-level=3 -Zinline-mir-hint-threshold=1000
 // Enable MIR inlining to ensure that `TerminatorKind::Terminate` is generated
 // instead of just `UnwindAction::Terminate`.
 

diff --git a/tests/mir-opt/inline/asm_unwind.rs b/tests/mir-opt/inline/asm_unwind.rs
@@ -2,6 +2,7 @@
 //
 // EMIT_MIR_FOR_EACH_PANIC_STRATEGY
 // needs-asm-support
+// compile-flags: -Zinline-mir-hint-threshold=1000
 #![feature(asm_unwind)]
 
 struct D;
@@ -10,7 +11,7 @@ impl Drop for D {
     fn drop(&mut self) {}
 }
 
-#[inline(always)]
+#[inline]
 fn foo() {
     let _d = D;
     unsafe { std::arch::asm!("", options(may_unwind)) };

diff --git a/tests/mir-opt/inline/cycle.rs b/tests/mir-opt/inline/cycle.rs
@@ -1,4 +1,5 @@
 // EMIT_MIR_FOR_EACH_PANIC_STRATEGY
+// compile-flags: -Zinline-mir-hint-threshold=1000
 
 // EMIT_MIR cycle.f.Inline.diff
 #[inline(always)]

diff --git a/tests/mir-opt/inline/inline_diverging.rs b/tests/mir-opt/inline/inline_diverging.rs
@@ -1,6 +1,7 @@
 // Tests inlining of diverging calls.
 //
 // EMIT_MIR_FOR_EACH_PANIC_STRATEGY
+// compile-flags: -Zinline-mir-hint-threshold=1000
 #![crate_type = "lib"]
 
 // EMIT_MIR inline_diverging.f.Inline.diff

diff --git a/tests/mir-opt/inline/inline_generator.main.Inline.panic-abort.diff b/tests/mir-opt/inline/inline_generator.main.Inline.panic-abort.diff
@@ -4,30 +4,30 @@
   fn main() -> () {
       let mut _0: ();
       let _1: std::ops::GeneratorState<i32, bool>;
-      let mut _2: std::pin::Pin<&mut [generator@$DIR/inline_generator.rs:15:5: 15:8]>;
-      let mut _3: &mut [generator@$DIR/inline_generator.rs:15:5: 15:8];
-      let mut _4: [generator@$DIR/inline_generator.rs:15:5: 15:8];
+      let mut _2: std::pin::Pin<&mut [generator@$DIR/inline_generator.rs:16:5: 16:8]>;
+      let mut _3: &mut [generator@$DIR/inline_generator.rs:16:5: 16:8];
+      let mut _4: [generator@$DIR/inline_generator.rs:16:5: 16:8];
 +     let mut _5: bool;
       scope 1 {
           debug _r => _1;
       }
 +     scope 2 (inlined g) {
 +     }
-+     scope 3 (inlined Pin::<&mut [generator@$DIR/inline_generator.rs:15:5: 15:8]>::new) {
++     scope 3 (inlined Pin::<&mut [generator@$DIR/inline_generator.rs:16:5: 16:8]>::new) {
 +         debug pointer => _3;
 +         scope 4 {
-+             scope 5 (inlined Pin::<&mut [generator@$DIR/inline_generator.rs:15:5: 15:8]>::new_unchecked) {
++             scope 5 (inlined Pin::<&mut [generator@$DIR/inline_generator.rs:16:5: 16:8]>::new_unchecked) {
 +                 debug pointer => _3;
 +             }
 +         }
 +     }
 +     scope 6 (inlined g::{closure#0}) {
 +         debug a => _5;
-+         let mut _6: &mut [generator@$DIR/inline_generator.rs:15:5: 15:8];
++         let mut _6: &mut [generator@$DIR/inline_generator.rs:16:5: 16:8];
 +         let mut _7: u32;
 +         let mut _8: i32;
-+         let mut _9: &mut [generator@$DIR/inline_generator.rs:15:5: 15:8];
-+         let mut _10: &mut [generator@$DIR/inline_generator.rs:15:5: 15:8];
++         let mut _9: &mut [generator@$DIR/inline_generator.rs:16:5: 16:8];
++         let mut _10: &mut [generator@$DIR/inline_generator.rs:16:5: 16:8];
 +     }
 
       bb0: {
@@ -39,18 +39,18 @@
 -     }
 - 
 -     bb1: {
-+         _4 = [generator@$DIR/inline_generator.rs:15:5: 15:8 (#0)];
++         _4 = [generator@$DIR/inline_generator.rs:16:5: 16:8 (#0)];
           _3 = &mut _4;
--         _2 = Pin::<&mut [generator@$DIR/inline_generator.rs:15:5: 15:8]>::new(move _3) -> [return: bb2, unwind unreachable];
+-         _2 = Pin::<&mut [generator@$DIR/inline_generator.rs:16:5: 16:8]>::new(move _3) -> [return: bb2, unwind unreachable];
 -     }
 - 
 -     bb2: {
-+         _2 = Pin::<&mut [generator@$DIR/inline_generator.rs:15:5: 15:8]> { pointer: move _3 };
++         _2 = Pin::<&mut [generator@$DIR/inline_generator.rs:16:5: 16:8]> { pointer: move _3 };
           StorageDead(_3);
--         _1 = <[generator@$DIR/inline_generator.rs:15:5: 15:8] as Generator<bool>>::resume(move _2, const false) -> [return: bb3, unwind unreachable];
+-         _1 = <[generator@$DIR/inline_generator.rs:16:5: 16:8] as Generator<bool>>::resume(move _2, const false) -> [return: bb3, unwind unreachable];
 +         StorageLive(_5);
 +         _5 = const false;
-+         _6 = deref_copy (_2.0: &mut [generator@$DIR/inline_generator.rs:15:5: 15:8]);
++         _6 = deref_copy (_2.0: &mut [generator@$DIR/inline_generator.rs:16:5: 16:8]);
 +         _7 = discriminant((*_6));
 +         switchInt(move _7) -> [0: bb2, 1: bb6, 3: bb7, otherwise: bb8];
       }
@@ -82,7 +82,7 @@
 + 
 +     bb5: {
 +         _1 = GeneratorState::<i32, bool>::Yielded(move _8);
-+         _9 = deref_copy (_2.0: &mut [generator@$DIR/inline_generator.rs:15:5: 15:8]);
++         _9 = deref_copy (_2.0: &mut [generator@$DIR/inline_generator.rs:16:5: 16:8]);
 +         discriminant((*_9)) = 3;
 +         goto -> bb1;
 +     }
@@ -95,7 +95,7 @@
 +         StorageLive(_8);
 +         StorageDead(_8);
 +         _1 = GeneratorState::<i32, bool>::Complete(_5);
-+         _10 = deref_copy (_2.0: &mut [generator@$DIR/inline_generator.rs:15:5: 15:8]);
++         _10 = deref_copy (_2.0: &mut [generator@$DIR/inline_generator.rs:16:5: 16:8]);
 +         discriminant((*_10)) = 1;
 +         goto -> bb1;
 +     }

diff --git a/tests/mir-opt/inline/inline_generator.main.Inline.panic-unwind.diff b/tests/mir-opt/inline/inline_generator.main.Inline.panic-unwind.diff
@@ -4,30 +4,30 @@
   fn main() -> () {
       let mut _0: ();
       let _1: std::ops::GeneratorState<i32, bool>;
-      let mut _2: std::pin::Pin<&mut [generator@$DIR/inline_generator.rs:15:5: 15:8]>;
-      let mut _3: &mut [generator@$DIR/inline_generator.rs:15:5: 15:8];
-      let mut _4: [generator@$DIR/inline_generator.rs:15:5: 15:8];
+      let mut _2: std::pin::Pin<&mut [generator@$DIR/inline_generator.rs:16:5: 16:8]>;
+      let mut _3: &mut [generator@$DIR/inline_generator.rs:16:5: 16:8];
+      let mut _4: [generator@$DIR/inline_generator.rs:16:5: 16:8];
 +     let mut _5: bool;
       scope 1 {
           debug _r => _1;
       }
 +     scope 2 (inlined g) {
 +     }
-+     scope 3 (inlined Pin::<&mut [generator@$DIR/inline_generator.rs:15:5: 15:8]>::new) {
++     scope 3 (inlined Pin::<&mut [generator@$DIR/inline_generator.rs:16:5: 16:8]>::new) {
 +         debug pointer => _3;
 +         scope 4 {
-+             scope 5 (inlined Pin::<&mut [generator@$DIR/inline_generator.rs:15:5: 15:8]>::new_unchecked) {
++             scope 5 (inlined Pin::<&mut [generator@$DIR/inline_generator.rs:16:5: 16:8]>::new_unchecked) {
 +                 debug pointer => _3;
 +             }
 +         }
 +     }
 +     scope 6 (inlined g::{closure#0}) {
 +         debug a => _5;
-+         let mut _6: &mut [generator@$DIR/inline_generator.rs:15:5: 15:8];
++         let mut _6: &mut [generator@$DIR/inline_generator.rs:16:5: 16:8];
 +         let mut _7: u32;
 +         let mut _8: i32;
-+         let mut _9: &mut [generator@$DIR/inline_generator.rs:15:5: 15:8];
-+         let mut _10: &mut [generator@$DIR/inline_generator.rs:15:5: 15:8];
++         let mut _9: &mut [generator@$DIR/inline_generator.rs:16:5: 16:8];
++         let mut _10: &mut [generator@$DIR/inline_generator.rs:16:5: 16:8];
 +     }
 
       bb0: {
@@ -39,18 +39,18 @@
 -     }
 - 
 -     bb1: {
-+         _4 = [generator@$DIR/inline_generator.rs:15:5: 15:8 (#0)];
++         _4 = [generator@$DIR/inline_generator.rs:16:5: 16:8 (#0)];
           _3 = &mut _4;
--         _2 = Pin::<&mut [generator@$DIR/inline_generator.rs:15:5: 15:8]>::new(move _3) -> [return: bb2, unwind: bb4];
+-         _2 = Pin::<&mut [generator@$DIR/inline_generator.rs:16:5: 16:8]>::new(move _3) -> [return: bb2, unwind: bb4];
 -     }
 - 
 -     bb2: {
-+         _2 = Pin::<&mut [generator@$DIR/inline_generator.rs:15:5: 15:8]> { pointer: move _3 };
++         _2 = Pin::<&mut [generator@$DIR/inline_generator.rs:16:5: 16:8]> { pointer: move _3 };
           StorageDead(_3);
--         _1 = <[generator@$DIR/inline_generator.rs:15:5: 15:8] as Generator<bool>>::resume(move _2, const false) -> [return: bb3, unwind: bb4];
+-         _1 = <[generator@$DIR/inline_generator.rs:16:5: 16:8] as Generator<bool>>::resume(move _2, const false) -> [return: bb3, unwind: bb4];
 +         StorageLive(_5);
 +         _5 = const false;
-+         _6 = deref_copy (_2.0: &mut [generator@$DIR/inline_generator.rs:15:5: 15:8]);
++         _6 = deref_copy (_2.0: &mut [generator@$DIR/inline_generator.rs:16:5: 16:8]);
 +         _7 = discriminant((*_6));
 +         switchInt(move _7) -> [0: bb3, 1: bb7, 3: bb8, otherwise: bb9];
       }
@@ -87,7 +87,7 @@
 + 
 +     bb6: {
 +         _1 = GeneratorState::<i32, bool>::Yielded(move _8);
-+         _9 = deref_copy (_2.0: &mut [generator@$DIR/inline_generator.rs:15:5: 15:8]);
++         _9 = deref_copy (_2.0: &mut [generator@$DIR/inline_generator.rs:16:5: 16:8]);
 +         discriminant((*_9)) = 3;
 +         goto -> bb1;
 +     }
@@ -100,7 +100,7 @@
 +         StorageLive(_8);
 +         StorageDead(_8);
 +         _1 = GeneratorState::<i32, bool>::Complete(_5);
-+         _10 = deref_copy (_2.0: &mut [generator@$DIR/inline_generator.rs:15:5: 15:8]);
++         _10 = deref_copy (_2.0: &mut [generator@$DIR/inline_generator.rs:16:5: 16:8]);
 +         discriminant((*_10)) = 1;
 +         goto -> bb1;
 +     }

diff --git a/tests/mir-opt/inline/inline_generator.rs b/tests/mir-opt/inline/inline_generator.rs
@@ -1,4 +1,5 @@
 // EMIT_MIR_FOR_EACH_PANIC_STRATEGY
+// compile-flags: -Zinline-mir-hint-threshold=1000
 #![feature(generators, generator_trait)]
 
 use std::ops::Generator;
@@ -9,8 +10,8 @@ fn main() {
     let _r = Pin::new(&mut g()).resume(false);
 }
 
-#[inline(always)]
+#[inline]
 pub fn g() -> impl Generator<bool> {
-    #[inline(always)]
+    #[inline]
     |a| { yield if a { 7 } else { 13 } }
 }