Auto merge of #123011 - scottmcm:inline-debuginfo-cost, r=<try>

Add a debug-info cost to MIR inlining

Inspired by all the MIR I keep looking at that's inlined away like 7 different function calls so the body is 2 statements but 30 debug-infos.

r? ghost

Author: bors

compiler/rustc_mir_transform/src/cost_checker.rs

@@ -7,6 +7,13 @@ const CALL_PENALTY: usize = 25;
 const LANDINGPAD_PENALTY: usize = 50;
 const RESUME_PENALTY: usize = 45;
 
+// While debug info has no runtime cost, when inlining we do need to copy it
+// into the caller MIR, which means we should give it a small cost to represent
+// the compile-time impact of doing that, since just because something has few
+// instructions doesn't necessarily mean it's cheap for us to inline.
+// The backend can, of course, still inline such things later should it so wish.
+const DEBUG_INFO_COST: usize = 2;
+
 /// Verify that the callee body is compatible with the caller.
 #[derive(Clone)]
 pub(crate) struct CostChecker<'b, 'tcx> {
@@ -31,6 +38,12 @@ impl<'b, 'tcx> CostChecker<'b, 'tcx> {
         self.cost
     }
 
+    // The MIR inliner doesn't actually call `visit_body`, so it doesn't work
+    // to put this in the visitor below.
+    pub fn before_body(&mut self, body: &Body<'tcx>) {
+        self.cost += body.var_debug_info.len() * DEBUG_INFO_COST;
+    }
+
     fn instantiate_ty(&self, v: Ty<'tcx>) -> Ty<'tcx> {
         if let Some(instance) = self.instance {
             instance.instantiate_mir(self.tcx, ty::EarlyBinder::bind(&v))

compiler/rustc_mir_transform/src/inline.rs

@@ -506,6 +506,8 @@ impl<'tcx> Inliner<'tcx> {
         let mut checker =
             CostChecker::new(self.tcx, self.param_env, Some(callsite.callee), callee_body);
 
+        checker.before_body(callee_body);
+
         // Traverse the MIR manually so we can account for the effects of inlining on the CFG.
         let mut work_list = vec![START_BLOCK];
         let mut visited = BitSet::new_empty(callee_body.basic_blocks.len());

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

@@ -7,35 +7,17 @@ fn int_range(_1: usize, _2: usize) -> () {
     let mut _3: std::ops::Range<usize>;
     let mut _4: std::ops::Range<usize>;
     let mut _5: &mut std::ops::Range<usize>;
-    let mut _13: std::option::Option<usize>;
-    let _15: ();
+    let mut _6: std::option::Option<usize>;
+    let mut _7: isize;
+    let _9: ();
     scope 1 {
         debug iter => _4;
-        let _14: usize;
+        let _8: usize;
         scope 2 {
-            debug i => _14;
+            debug i => _8;
         }
         scope 4 (inlined iter::range::<impl Iterator for std::ops::Range<usize>>::next) {
             debug self => _5;
-            scope 5 (inlined <std::ops::Range<usize> as iter::range::RangeIteratorImpl>::spec_next) {
-                debug self => _5;
-                let mut _6: &usize;
-                let mut _7: &usize;
-                let mut _10: bool;
-                let _11: usize;
-                let mut _12: usize;
-                scope 6 {
-                    debug old => _11;
-                    scope 7 {
-                    }
-                }
-                scope 8 (inlined std::cmp::impls::<impl PartialOrd for usize>::lt) {
-                    debug self => _6;
-                    debug other => _7;
-                    let mut _8: usize;
-                    let mut _9: usize;
-                }
-            }
         }
     }
     scope 3 (inlined <std::ops::Range<usize> as IntoIterator>::into_iter) {
@@ -50,54 +32,35 @@ fn int_range(_1: usize, _2: usize) -> () {
     }
 
     bb1: {
-        StorageLive(_13);
-        _5 = &mut _4;
-        StorageLive(_11);
-        StorageLive(_10);
         StorageLive(_6);
-        _6 = &(_4.0: usize);
-        StorageLive(_7);
-        _7 = &(_4.1: usize);
-        StorageLive(_8);
-        _8 = (_4.0: usize);
-        StorageLive(_9);
-        _9 = (_4.1: usize);
-        _10 = Lt(move _8, move _9);
-        StorageDead(_9);
-        StorageDead(_8);
-        switchInt(move _10) -> [0: bb2, otherwise: bb3];
+        StorageLive(_5);
+        _5 = &mut _4;
+        _6 = <std::ops::Range<usize> as iter::range::RangeIteratorImpl>::spec_next(move _5) -> [return: bb2, unwind continue];
     }
 
     bb2: {
-        StorageDead(_7);
-        StorageDead(_6);
-        StorageDead(_10);
-        StorageDead(_11);
-        StorageDead(_13);
-        StorageDead(_4);
-        return;
+        StorageDead(_5);
+        _7 = discriminant(_6);
+        switchInt(move _7) -> [0: bb3, 1: bb4, otherwise: bb6];
     }
 
     bb3: {
-        StorageDead(_7);
         StorageDead(_6);
-        _11 = (_4.0: usize);
-        StorageLive(_12);
-        _12 = <usize as Step>::forward_unchecked(_11, const 1_usize) -> [return: bb4, unwind continue];
+        StorageDead(_4);
+        return;
     }
 
     bb4: {
-        (_4.0: usize) = move _12;
-        StorageDead(_12);
-        _13 = Option::<usize>::Some(_11);
-        StorageDead(_10);
-        StorageDead(_11);
-        _14 = ((_13 as Some).0: usize);
-        _15 = opaque::<usize>(move _14) -> [return: bb5, unwind continue];
+        _8 = ((_6 as Some).0: usize);
+        _9 = opaque::<usize>(move _8) -> [return: bb5, unwind continue];
     }
 
     bb5: {
-        StorageDead(_13);
+        StorageDead(_6);
         goto -> bb1;
     }
+
+    bb6: {
+        unreachable;
+    }
 }

tests/mir-opt/pre-codegen/range_iter.forward_loop.PreCodegen.after.panic-unwind.mir

@@ -8,37 +8,19 @@ fn forward_loop(_1: u32, _2: u32, _3: impl Fn(u32)) -> () {
     let mut _4: std::ops::Range<u32>;
     let mut _5: std::ops::Range<u32>;
     let mut _6: &mut std::ops::Range<u32>;
-    let mut _14: std::option::Option<u32>;
-    let mut _16: &impl Fn(u32);
-    let mut _17: (u32,);
-    let _18: ();
+    let mut _7: std::option::Option<u32>;
+    let mut _8: isize;
+    let mut _10: &impl Fn(u32);
+    let mut _11: (u32,);
+    let _12: ();
     scope 1 {
         debug iter => _5;
-        let _15: u32;
+        let _9: u32;
         scope 2 {
-            debug x => _15;
+            debug x => _9;
         }
         scope 4 (inlined iter::range::<impl Iterator for std::ops::Range<u32>>::next) {
             debug self => _6;
-            scope 5 (inlined <std::ops::Range<u32> as iter::range::RangeIteratorImpl>::spec_next) {
-                debug self => _6;
-                let mut _7: &u32;
-                let mut _8: &u32;
-                let mut _11: bool;
-                let _12: u32;
-                let mut _13: u32;
-                scope 6 {
-                    debug old => _12;
-                    scope 7 {
-                    }
-                }
-                scope 8 (inlined std::cmp::impls::<impl PartialOrd for u32>::lt) {
-                    debug self => _7;
-                    debug other => _8;
-                    let mut _9: u32;
-                    let mut _10: u32;
-                }
-            }
         }
     }
     scope 3 (inlined <std::ops::Range<u32> as IntoIterator>::into_iter) {
@@ -53,72 +35,53 @@ fn forward_loop(_1: u32, _2: u32, _3: impl Fn(u32)) -> () {
     }
 
     bb1: {
-        StorageLive(_14);
-        _6 = &mut _5;
-        StorageLive(_12);
-        StorageLive(_11);
         StorageLive(_7);
-        _7 = &(_5.0: u32);
-        StorageLive(_8);
-        _8 = &(_5.1: u32);
-        StorageLive(_9);
-        _9 = (_5.0: u32);
-        StorageLive(_10);
-        _10 = (_5.1: u32);
-        _11 = Lt(move _9, move _10);
-        StorageDead(_10);
-        StorageDead(_9);
-        switchInt(move _11) -> [0: bb2, otherwise: bb4];
+        StorageLive(_6);
+        _6 = &mut _5;
+        _7 = <std::ops::Range<u32> as iter::range::RangeIteratorImpl>::spec_next(move _6) -> [return: bb2, unwind: bb8];
     }
 
     bb2: {
-        StorageDead(_8);
-        StorageDead(_7);
-        StorageDead(_11);
-        StorageDead(_12);
-        StorageDead(_14);
-        StorageDead(_5);
-        drop(_3) -> [return: bb3, unwind continue];
+        StorageDead(_6);
+        _8 = discriminant(_7);
+        switchInt(move _8) -> [0: bb3, 1: bb5, otherwise: bb7];
     }
 
     bb3: {
-        return;
+        StorageDead(_7);
+        StorageDead(_5);
+        drop(_3) -> [return: bb4, unwind continue];
     }
 
     bb4: {
-        StorageDead(_8);
-        StorageDead(_7);
-        _12 = (_5.0: u32);
-        StorageLive(_13);
-        _13 = <u32 as Step>::forward_unchecked(_12, const 1_usize) -> [return: bb5, unwind: bb7];
+        return;
     }
 
     bb5: {
-        (_5.0: u32) = move _13;
-        StorageDead(_13);
-        _14 = Option::<u32>::Some(_12);
-        StorageDead(_11);
-        StorageDead(_12);
-        _15 = ((_14 as Some).0: u32);
-        StorageLive(_16);
-        _16 = &_3;
-        StorageLive(_17);
-        _17 = (_15,);
-        _18 = <impl Fn(u32) as Fn<(u32,)>>::call(move _16, move _17) -> [return: bb6, unwind: bb7];
+        _9 = ((_7 as Some).0: u32);
+        StorageLive(_10);
+        _10 = &_3;
+        StorageLive(_11);
+        _11 = (_9,);
+        _12 = <impl Fn(u32) as Fn<(u32,)>>::call(move _10, move _11) -> [return: bb6, unwind: bb8];
     }
 
     bb6: {
-        StorageDead(_17);
-        StorageDead(_16);
-        StorageDead(_14);
+        StorageDead(_11);
+        StorageDead(_10);
+        StorageDead(_7);
         goto -> bb1;
     }
 
-    bb7 (cleanup): {
-        drop(_3) -> [return: bb8, unwind terminate(cleanup)];
+    bb7: {
+        unreachable;
     }
 
     bb8 (cleanup): {
+        drop(_3) -> [return: bb9, unwind terminate(cleanup)];
+    }
+
+    bb9 (cleanup): {
         resume;
     }
 }

tests/mir-opt/pre-codegen/range_iter.range_iter_next.PreCodegen.after.panic-unwind.mir

@@ -5,73 +5,13 @@ fn range_iter_next(_1: &mut std::ops::Range<u32>) -> Option<u32> {
     let mut _0: std::option::Option<u32>;
     scope 1 (inlined iter::range::<impl Iterator for std::ops::Range<u32>>::next) {
         debug self => _1;
-        scope 2 (inlined <std::ops::Range<u32> as iter::range::RangeIteratorImpl>::spec_next) {
-            debug self => _1;
-            let mut _2: &u32;
-            let mut _3: &u32;
-            let mut _6: bool;
-            let _7: u32;
-            let mut _8: u32;
-            scope 3 {
-                debug old => _7;
-                scope 4 {
-                }
-            }
-            scope 5 (inlined std::cmp::impls::<impl PartialOrd for u32>::lt) {
-                debug self => _2;
-                debug other => _3;
-                let mut _4: u32;
-                let mut _5: u32;
-            }
-        }
     }
 
     bb0: {
-        StorageLive(_7);
-        StorageLive(_6);
-        StorageLive(_2);
-        _2 = &((*_1).0: u32);
-        StorageLive(_3);
-        _3 = &((*_1).1: u32);
-        StorageLive(_4);
-        _4 = ((*_1).0: u32);
-        StorageLive(_5);
-        _5 = ((*_1).1: u32);
-        _6 = Lt(move _4, move _5);
-        StorageDead(_5);
-        StorageDead(_4);
-        switchInt(move _6) -> [0: bb1, otherwise: bb2];
+        _0 = <std::ops::Range<u32> as iter::range::RangeIteratorImpl>::spec_next(move _1) -> [return: bb1, unwind continue];
     }
 
     bb1: {
-        StorageDead(_3);
-        StorageDead(_2);
-        _0 = const Option::<u32>::None;
-        goto -> bb4;
-    }
-
-    bb2: {
-        StorageDead(_3);
-        StorageDead(_2);
-        _7 = ((*_1).0: u32);
-        StorageLive(_8);
-        _8 = <u32 as Step>::forward_unchecked(_7, const 1_usize) -> [return: bb3, unwind continue];
-    }
-
-    bb3: {
-        ((*_1).0: u32) = move _8;
-        StorageDead(_8);
-        _0 = Option::<u32>::Some(_7);
-        goto -> bb4;
-    }
-
-    bb4: {
-        StorageDead(_6);
-        StorageDead(_7);
         return;
     }
 }
-
-ALLOC0 (size: 8, align: 4) {
-    00 00 00 00 __ __ __ __                         │ ....░░░░
-}