Give inlining bonuses to things that optimize out

compiler/rustc_mir_transform/src/cost_checker.rs

@@ -1,3 +1,4 @@
+use rustc_middle::bug;
 use rustc_middle::mir::visit::*;
 use rustc_middle::mir::*;
 use rustc_middle::ty::{self, ParamEnv, Ty, TyCtxt};
@@ -6,6 +7,8 @@
 const CALL_PENALTY: usize = 25;
 const LANDINGPAD_PENALTY: usize = 50;
 const RESUME_PENALTY: usize = 45;
+const LARGE_SWITCH_PENALTY: usize = 20;
+const CONST_SWITCH_BONUS: usize = 10;
 
 /// Verify that the callee body is compatible with the caller.
 #[derive(Clone)]
@@ -42,36 +45,49 @@ fn instantiate_ty(&self, v: Ty<'tcx>) -> Ty<'tcx> {
 }
 
 impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> {
-    fn visit_statement(&mut self, statement: &Statement<'tcx>, _: Location) {
+    fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) {
-        // Don't count StorageLive/StorageDead in the inlining cost.
+        // Most costs are in rvalues and terminators, not in statements.
         match statement.kind {
-            StatementKind::StorageLive(_)
+            StatementKind::Intrinsic(ref ndi) => {
-            | StatementKind::StorageDead(_)
+                self.penalty += match **ndi {
-            | StatementKind::Deinit(_)
+                    NonDivergingIntrinsic::Assume(..) => INSTR_COST,
-            | StatementKind::Nop => {}
+                    NonDivergingIntrinsic::CopyNonOverlapping(..) => CALL_PENALTY,
+                };
+            }
+            _ => self.super_statement(statement, location),
+        }
+    }
+
+    fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, _location: Location) {
+        match rvalue {
+            Rvalue::NullaryOp(NullOp::UbChecks, ..) if !self.tcx.sess.ub_checks() => {
+                // If this is in optimized MIR it's because it's used later,
+                // so if we don't need UB checks this session, give a bonus
+                // here to offset the cost of the call later.
+                self.bonus += CALL_PENALTY;
+            }
+            // These are essentially constants that didn't end up in an Operand,
+            // so treat them as also being free.
+            Rvalue::NullaryOp(..) => {}
             _ => self.penalty += INSTR_COST,
         }
     }
 
     fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, _: Location) {
-        let tcx = self.tcx;
+        match &terminator.kind {
-        match terminator.kind {
+            TerminatorKind::Drop { place, unwind, .. } => {
-            TerminatorKind::Drop { ref place, unwind, .. } => {
                 // If the place doesn't actually need dropping, treat it like a regular goto.
-                let ty = self.instantiate_ty(place.ty(self.callee_body, tcx).ty);
+                let ty = self.instantiate_ty(place.ty(self.callee_body, self.tcx).ty);
-                if ty.needs_drop(tcx, self.param_env) {
+                if ty.needs_drop(self.tcx, self.param_env) {
                     self.penalty += CALL_PENALTY;
                     if let UnwindAction::Cleanup(_) = unwind {
                         self.penalty += LANDINGPAD_PENALTY;
                     }
-                } else {
-                    self.penalty += INSTR_COST;
                 }
             }
-            TerminatorKind::Call { func: Operand::Constant(ref f), unwind, .. } => {
+            TerminatorKind::Call { func, unwind, .. } => {
-                let fn_ty = self.instantiate_ty(f.const_.ty());
+                self.penalty += if let Some((def_id, ..)) = func.const_fn_def()
-                self.penalty += if let ty::FnDef(def_id, _) = *fn_ty.kind()
+                    && self.tcx.intrinsic(def_id).is_some()
-                    && tcx.intrinsic(def_id).is_some()
                 {
                     // Don't give intrinsics the extra penalty for calls
                     INSTR_COST
@@ -82,8 +98,25 @@ fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, _: Location) {
                     self.penalty += LANDINGPAD_PENALTY;
                 }
             }
-            TerminatorKind::Assert { unwind, .. } => {
+            TerminatorKind::SwitchInt { discr, targets } => {
-                self.penalty += CALL_PENALTY;
+                if discr.constant().is_some() {
+                    // Not only will this become a `Goto`, but likely other
+                    // things will be removable as unreachable.
+                    self.bonus += CONST_SWITCH_BONUS;
+                } else if targets.all_targets().len() > 3 {
+                    // More than false/true/unreachable gets extra cost.
+                    self.penalty += LARGE_SWITCH_PENALTY;
+                } else {
+                    self.penalty += INSTR_COST;
+                }
+            }
+            TerminatorKind::Assert { unwind, msg, .. } => {
+                self.penalty +=
+                    if msg.is_optional_overflow_check() && !self.tcx.sess.overflow_checks() {
+                        INSTR_COST
+                    } else {
+                        CALL_PENALTY
+                    };
                 if let UnwindAction::Cleanup(_) = unwind {
                     self.penalty += LANDINGPAD_PENALTY;
                 }
@@ -95,7 +128,17 @@ fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, _: Location) {
                     self.penalty += LANDINGPAD_PENALTY;
                 }
             }
-            _ => self.penalty += INSTR_COST,
+            TerminatorKind::Unreachable => {
+                self.bonus += INSTR_COST;
+            }
+            TerminatorKind::Goto { .. } | TerminatorKind::Return => {}
+            TerminatorKind::UnwindTerminate(..) => {}
+            kind @ (TerminatorKind::FalseUnwind { .. }
+            | TerminatorKind::FalseEdge { .. }
+            | TerminatorKind::Yield { .. }
+            | TerminatorKind::CoroutineDrop) => {
+                bug!("{kind:?} should not be in runtime MIR");
+            }
         }
     }
 }

tests/codegen/issues/issue-112509-slice-get-andthen-get.rs

@@ -3,8 +3,12 @@
 
 // CHECK-LABEL: @write_u8_variant_a
 // CHECK-NEXT: {{.*}}:
-// CHECK-NEXT: getelementptr
 // CHECK-NEXT: icmp ugt
+// CHECK-NEXT: getelementptr
+// CHECK-NEXT: select i1 {{.+}} null
+// CHECK-NEXT: insertvalue
+// CHECK-NEXT: insertvalue
+// CHECK-NEXT: ret
 #[no_mangle]
 pub fn write_u8_variant_a(bytes: &mut [u8], buf: u8, offset: usize) -> Option<&mut [u8]> {
     let buf = buf.to_le_bytes();

tests/crashes/123893.rs

@@ -11,6 +11,7 @@ trait MagicTrait {
     impl<T> MagicTrait for T {
         const IS_BIG: bool = true;
     }
+    more_cost();
     if T::IS_BIG {
         big_impl::<i32>();
     }
@@ -18,3 +19,6 @@ impl<T> MagicTrait for T {
 
 #[inline(never)]
 fn big_impl<T>() {}
+
+#[inline(never)]
+fn more_cost() {}

tests/mir-opt/pre-codegen/checked_ops.step_forward.PreCodegen.after.panic-abort.mir

@@ -4,12 +4,87 @@ fn step_forward(_1: u16, _2: usize) -> u16 {
     debug x => _1;
     debug n => _2;
     let mut _0: u16;
+    scope 1 (inlined <u16 as Step>::forward) {
+        let mut _8: u16;
+        scope 2 {
+        }
+        scope 3 (inlined <u16 as Step>::forward_checked) {
+            scope 4 {
+                scope 6 (inlined core::num::<impl u16>::checked_add) {
+                    let mut _7: bool;
+                    scope 7 {
+                    }
+                    scope 8 (inlined core::num::<impl u16>::overflowing_add) {
+                        let mut _5: (u16, bool);
+                        let _6: bool;
+                        scope 9 {
+                        }
+                    }
+                }
+            }
+            scope 5 (inlined convert::num::ptr_try_from_impls::<impl TryFrom<usize> for u16>::try_from) {
+                let mut _3: bool;
+                let mut _4: u16;
+            }
+        }
+        scope 10 (inlined Option::<u16>::is_none) {
+            scope 11 (inlined Option::<u16>::is_some) {
+            }
+        }
+        scope 12 (inlined core::num::<impl u16>::wrapping_add) {
+        }
+    }
 
     bb0: {
-        _0 = <u16 as Step>::forward(move _1, move _2) -> [return: bb1, unwind unreachable];
+        StorageLive(_4);
+        StorageLive(_3);
+        _3 = Gt(_2, const 65535_usize);
+        switchInt(move _3) -> [0: bb1, otherwise: bb5];
     }
 
     bb1: {
+        _4 = _2 as u16 (IntToInt);
+        StorageDead(_3);
+        StorageLive(_6);
+        StorageLive(_5);
+        _5 = AddWithOverflow(_1, _4);
+        _6 = (_5.1: bool);
+        StorageDead(_5);
+        StorageLive(_7);
+        _7 = unlikely(move _6) -> [return: bb2, unwind unreachable];
+    }
+
+    bb2: {
+        switchInt(move _7) -> [0: bb3, otherwise: bb4];
+    }
+
+    bb3: {
+        StorageDead(_7);
+        StorageDead(_6);
+        goto -> bb7;
+    }
+
+    bb4: {
+        StorageDead(_7);
+        StorageDead(_6);
+        goto -> bb6;
+    }
+
+    bb5: {
+        StorageDead(_3);
+        goto -> bb6;
+    }
+
+    bb6: {
+        assert(!const true, "attempt to compute `{} + {}`, which would overflow", const core::num::<impl u16>::MAX, const 1_u16) -> [success: bb7, unwind unreachable];
+    }
+
+    bb7: {
+        StorageLive(_8);
+        _8 = _2 as u16 (IntToInt);
+        _0 = Add(_1, _8);
+        StorageDead(_8);
+        StorageDead(_4);
         return;
     }
 }

tests/mir-opt/pre-codegen/checked_ops.step_forward.PreCodegen.after.panic-unwind.mir

@@ -4,12 +4,87 @@ fn step_forward(_1: u16, _2: usize) -> u16 {
     debug x => _1;
     debug n => _2;
     let mut _0: u16;
+    scope 1 (inlined <u16 as Step>::forward) {
+        let mut _8: u16;
+        scope 2 {
+        }
+        scope 3 (inlined <u16 as Step>::forward_checked) {
+            scope 4 {
+                scope 6 (inlined core::num::<impl u16>::checked_add) {
+                    let mut _7: bool;
+                    scope 7 {
+                    }
+                    scope 8 (inlined core::num::<impl u16>::overflowing_add) {
+                        let mut _5: (u16, bool);
+                        let _6: bool;
+                        scope 9 {
+                        }
+                    }
+                }
+            }
+            scope 5 (inlined convert::num::ptr_try_from_impls::<impl TryFrom<usize> for u16>::try_from) {
+                let mut _3: bool;
+                let mut _4: u16;
+            }
+        }
+        scope 10 (inlined Option::<u16>::is_none) {
+            scope 11 (inlined Option::<u16>::is_some) {
+            }
+        }
+        scope 12 (inlined core::num::<impl u16>::wrapping_add) {
+        }
+    }
 
     bb0: {
-        _0 = <u16 as Step>::forward(move _1, move _2) -> [return: bb1, unwind continue];
+        StorageLive(_4);
+        StorageLive(_3);
+        _3 = Gt(_2, const 65535_usize);
+        switchInt(move _3) -> [0: bb1, otherwise: bb5];
     }
 
     bb1: {
+        _4 = _2 as u16 (IntToInt);
+        StorageDead(_3);
+        StorageLive(_6);
+        StorageLive(_5);
+        _5 = AddWithOverflow(_1, _4);
+        _6 = (_5.1: bool);
+        StorageDead(_5);
+        StorageLive(_7);
+        _7 = unlikely(move _6) -> [return: bb2, unwind unreachable];
+    }
+
+    bb2: {
+        switchInt(move _7) -> [0: bb3, otherwise: bb4];
+    }
+
+    bb3: {
+        StorageDead(_7);
+        StorageDead(_6);
+        goto -> bb7;
+    }
+
+    bb4: {
+        StorageDead(_7);
+        StorageDead(_6);
+        goto -> bb6;
+    }
+
+    bb5: {
+        StorageDead(_3);
+        goto -> bb6;
+    }
+
+    bb6: {
+        assert(!const true, "attempt to compute `{} + {}`, which would overflow", const core::num::<impl u16>::MAX, const 1_u16) -> [success: bb7, unwind continue];
+    }
+
+    bb7: {
+        StorageLive(_8);
+        _8 = _2 as u16 (IntToInt);
+        _0 = Add(_1, _8);
+        StorageDead(_8);
+        StorageDead(_4);
         return;
     }
 }

tests/mir-opt/pre-codegen/loops.mapped.PreCodegen.after.mir

@@ -7,14 +7,30 @@ fn mapped(_1: impl Iterator<Item = T>, _2: impl Fn(T) -> U) -> () {
     let mut _3: std::iter::Map<impl Iterator<Item = T>, impl Fn(T) -> U>;
     let mut _4: std::iter::Map<impl Iterator<Item = T>, impl Fn(T) -> U>;
     let mut _5: &mut std::iter::Map<impl Iterator<Item = T>, impl Fn(T) -> U>;
-    let mut _6: std::option::Option<U>;
+    let mut _13: std::option::Option<U>;
-    let mut _7: isize;
+    let _15: ();
-    let _9: ();
     scope 1 {
         debug iter => _4;
-        let _8: U;
+        let _14: U;
         scope 2 {
-            debug x => _8;
+            debug x => _14;
+        }
+        scope 4 (inlined <Map<impl Iterator<Item = T>, impl Fn(T) -> U> as Iterator>::next) {
+            debug self => _5;
+            let mut _6: &mut impl Iterator<Item = T>;
+            let mut _7: std::option::Option<T>;
+            let mut _8: &mut impl Fn(T) -> U;
+            scope 5 (inlined Option::<T>::map::<U, &mut impl Fn(T) -> U>) {
+                debug self => _7;
+                debug f => _8;
+                let mut _9: isize;
+                let _10: T;
+                let mut _11: (T,);
+                let mut _12: U;
+                scope 6 {
+                    debug x => _10;
+                }
+            }
         }
     }
     scope 3 (inlined <Map<impl Iterator<Item = T>, impl Fn(T) -> U> as IntoIterator>::into_iter) {
@@ -32,20 +48,30 @@ fn mapped(_1: impl Iterator<Item = T>, _2: impl Fn(T) -> U) -> () {
     }
 
     bb2: {
-        StorageLive(_6);
+        StorageLive(_13);
-        StorageLive(_5);
         _5 = &mut _4;
-        _6 = <Map<impl Iterator<Item = T>, impl Fn(T) -> U> as Iterator>::next(move _5) -> [return: bb3, unwind: bb9];
+        StorageLive(_8);
+        StorageLive(_7);
+        StorageLive(_6);
+        _6 = &mut (_4.0: impl Iterator<Item = T>);
+        _7 = <impl Iterator<Item = T> as Iterator>::next(move _6) -> [return: bb3, unwind: bb10];
     }
 
     bb3: {
-        StorageDead(_5);
+        StorageDead(_6);
-        _7 = discriminant(_6);
+        _8 = &mut (_4.1: impl Fn(T) -> U);
-        switchInt(move _7) -> [0: bb4, 1: bb6, otherwise: bb8];
+        StorageLive(_9);
+        StorageLive(_10);
+        _9 = discriminant(_7);
+        switchInt(move _9) -> [0: bb4, 1: bb6, otherwise: bb9];
     }
 
     bb4: {
-        StorageDead(_6);
+        StorageDead(_10);
+        StorageDead(_9);
+        StorageDead(_7);
+        StorageDead(_8);
+        StorageDead(_13);
         drop(_4) -> [return: bb5, unwind continue];
     }
 
@@ -55,24 +81,39 @@ fn mapped(_1: impl Iterator<Item = T>, _2: impl Fn(T) -> U) -> () {
     }
 
     bb6: {
-        _8 = move ((_6 as Some).0: U);
+        _10 = move ((_7 as Some).0: T);
-        _9 = opaque::<U>(move _8) -> [return: bb7, unwind: bb9];
+        StorageLive(_12);
+        StorageLive(_11);
+        _11 = (_10,);
+        _12 = <&mut impl Fn(T) -> U as FnOnce<(T,)>>::call_once(move _8, move _11) -> [return: bb7, unwind: bb10];
     }
 
     bb7: {
-        StorageDead(_6);
+        StorageDead(_11);
-        goto -> bb2;
+        _13 = Option::<U>::Some(move _12);
+        StorageDead(_12);
+        StorageDead(_10);
+        StorageDead(_9);
+        StorageDead(_7);
+        StorageDead(_8);
+        _14 = move ((_13 as Some).0: U);
+        _15 = opaque::<U>(move _14) -> [return: bb8, unwind: bb10];
     }
 
     bb8: {
+        StorageDead(_13);
+        goto -> bb2;
+    }
+
+    bb9: {
         unreachable;
     }
 
-    bb9 (cleanup): {
+    bb10 (cleanup): {
-        drop(_4) -> [return: bb10, unwind terminate(cleanup)];
+        drop(_4) -> [return: bb11, unwind terminate(cleanup)];
     }
 
-    bb10 (cleanup): {
+    bb11 (cleanup): {
         resume;
     }
 }

tests/mir-opt/pre-codegen/slice_iter.enumerated_loop.PreCodegen.after.panic-abort.mir

@@ -7,19 +7,44 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () {
     let mut _11: std::slice::Iter<'_, T>;
     let mut _12: std::iter::Enumerate<std::slice::Iter<'_, T>>;
     let mut _13: std::iter::Enumerate<std::slice::Iter<'_, T>>;
-    let mut _14: &mut std::iter::Enumerate<std::slice::Iter<'_, T>>;
+    let mut _21: std::option::Option<(usize, &T)>;
-    let mut _15: std::option::Option<(usize, &T)>;
+    let mut _24: &impl Fn(usize, &T);
-    let mut _16: isize;
+    let mut _25: (usize, &T);
-    let mut _19: &impl Fn(usize, &T);
+    let _26: ();
-    let mut _20: (usize, &T);
-    let _21: ();
     scope 1 {
         debug iter => _13;
-        let _17: usize;
+        let _22: usize;
-        let _18: &T;
+        let _23: &T;
         scope 2 {
-            debug i => _17;
+            debug i => _22;
-            debug x => _18;
+            debug x => _23;
+        }
+        scope 17 (inlined <Enumerate<std::slice::Iter<'_, T>> as Iterator>::next) {
+            let mut _14: &mut std::slice::Iter<'_, T>;
+            let mut _15: std::option::Option<&T>;
+            let mut _19: (usize, bool);
+            let mut _20: (usize, &T);
+            scope 18 {
+                let _18: usize;
+                scope 23 {
+                }
+            }
+            scope 19 {
+                scope 20 {
+                    scope 26 (inlined <Option<(usize, &T)> as FromResidual>::from_residual) {
+                    }
+                }
+            }
+            scope 21 {
+                scope 22 {
+                }
+            }
+            scope 24 (inlined <Option<&T> as Try>::branch) {
+                let mut _16: isize;
+                let _17: &T;
+                scope 25 {
+                }
+            }
         }
     }
     scope 3 (inlined core::slice::<impl [T]>::iter) {
@@ -107,20 +132,28 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () {
     }
 
     bb4: {
+        StorageLive(_21);
+        StorageLive(_18);
+        StorageLive(_19);
         StorageLive(_15);
         StorageLive(_14);
-        _14 = &mut _13;
+        _14 = &mut (_13.0: std::slice::Iter<'_, T>);
-        _15 = <Enumerate<std::slice::Iter<'_, T>> as Iterator>::next(move _14) -> [return: bb5, unwind unreachable];
+        _15 = <std::slice::Iter<'_, T> as Iterator>::next(move _14) -> [return: bb5, unwind unreachable];
     }
 
     bb5: {
         StorageDead(_14);
+        StorageLive(_16);
         _16 = discriminant(_15);
-        switchInt(move _16) -> [0: bb6, 1: bb8, otherwise: bb10];
+        switchInt(move _16) -> [0: bb6, 1: bb8, otherwise: bb11];
     }
 
     bb6: {
+        StorageDead(_16);
         StorageDead(_15);
+        StorageDead(_19);
+        StorageDead(_18);
+        StorageDead(_21);
         StorageDead(_13);
         drop(_2) -> [return: bb7, unwind unreachable];
     }
@@ -130,23 +163,39 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () {
     }
 
     bb8: {
-        _17 = (((_15 as Some).0: (usize, &T)).0: usize);
+        _17 = move ((_15 as Some).0: &T);
-        _18 = (((_15 as Some).0: (usize, &T)).1: &T);
+        StorageDead(_16);
-        StorageLive(_19);
+        StorageDead(_15);
-        _19 = &_2;
+        _18 = (_13.1: usize);
-        StorageLive(_20);
+        _19 = AddWithOverflow((_13.1: usize), const 1_usize);
-        _20 = (_17, _18);
+        assert(!move (_19.1: bool), "attempt to compute `{} + {}`, which would overflow", (_13.1: usize), const 1_usize) -> [success: bb9, unwind unreachable];
-        _21 = <impl Fn(usize, &T) as Fn<(usize, &T)>>::call(move _19, move _20) -> [return: bb9, unwind unreachable];
     }
 
     bb9: {
+        (_13.1: usize) = move (_19.0: usize);
+        StorageLive(_20);
+        _20 = (_18, _17);
+        _21 = Option::<(usize, &T)>::Some(move _20);
         StorageDead(_20);
         StorageDead(_19);
-        StorageDead(_15);
+        StorageDead(_18);
-        goto -> bb4;
+        _22 = (((_21 as Some).0: (usize, &T)).0: usize);
+        _23 = (((_21 as Some).0: (usize, &T)).1: &T);
+        StorageLive(_24);
+        _24 = &_2;
+        StorageLive(_25);
+        _25 = (_22, _23);
+        _26 = <impl Fn(usize, &T) as Fn<(usize, &T)>>::call(move _24, move _25) -> [return: bb10, unwind unreachable];
     }
 
     bb10: {
+        StorageDead(_25);
+        StorageDead(_24);
+        StorageDead(_21);
+        goto -> bb4;
+    }
+
+    bb11: {
         unreachable;
     }
 }