Skip to content

Commit 915aa06

Browse files
borsbors
committed
Auto merge of #110705 - saethlin:ignore-locals-cost, r=cjgillot
Remove the size of locals heuristic in MIR inlining This heuristic doesn't necessarily correlate to complexity of the MIR Body. In particular, a lot of straight-line code in MIR tends to never reuse a local, even though any optimizer would effectively reuse the storage or just put everything in registers. So it doesn't even necessarily make sense that this would be a stack size heuristic. So... what happens if we just delete the heuristic? The benchmark suite improves significantly. Less heuristics better? r? `@cjgillot`
2 parents 3462f79 + 173845c commit 915aa06

5 files changed

+450
-82
lines changed

‎compiler/rustc_mir_transform/src/inline.rs‎

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@ const CALL_PENALTY: usize = 25;
2626
const LANDINGPAD_PENALTY: usize = 50;
2727
const RESUME_PENALTY: usize = 45;
2828

29-
const UNKNOWN_SIZE_COST: usize = 10;
30-
3129
const TOP_DOWN_DEPTH_LIMIT: usize = 5;
3230

3331
pub struct Inline;
@@ -464,12 +462,6 @@ impl<'tcx> Inliner<'tcx> {
464462
}
465463
}
466464

467-
// Count up the cost of local variables and temps, if we know the size
468-
// use that, otherwise we use a moderately-large dummy cost.
469-
for v in callee_body.vars_and_temps_iter() {
470-
checker.visit_local_decl(v, &callee_body.local_decls[v]);
471-
}
472-
473465
// Abort if type validation found anything fishy.
474466
checker.validation?;
475467

@@ -764,14 +756,6 @@ impl<'tcx> Inliner<'tcx> {
764756
}
765757
}
766758

767-
fn type_size_of<'tcx>(
768-
tcx: TyCtxt<'tcx>,
769-
param_env: ty::ParamEnv<'tcx>,
770-
ty: Ty<'tcx>,
771-
) -> Option<u64> {
772-
tcx.layout_of(param_env.and(ty)).ok().map(|layout| layout.size.bytes())
773-
}
774-
775759
/// Verify that the callee body is compatible with the caller.
776760
///
777761
/// This visitor mostly computes the inlining cost,
@@ -845,24 +829,6 @@ impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> {
845829
self.super_terminator(terminator, location);
846830
}
847831

848-
/// Count up the cost of local variables and temps, if we know the size
849-
/// use that, otherwise we use a moderately-large dummy cost.
850-
fn visit_local_decl(&mut self, local: Local, local_decl: &LocalDecl<'tcx>) {
851-
let tcx = self.tcx;
852-
let ptr_size = tcx.data_layout.pointer_size.bytes();
853-
854-
let ty = self.instance.subst_mir(tcx, &local_decl.ty);
855-
// Cost of the var is the size in machine-words, if we know
856-
// it.
857-
if let Some(size) = type_size_of(tcx, self.param_env, ty) {
858-
self.cost += ((size + ptr_size - 1) / ptr_size) as usize;
859-
} else {
860-
self.cost += UNKNOWN_SIZE_COST;
861-
}
862-
863-
self.super_local_decl(local, local_decl)
864-
}
865-
866832
/// This method duplicates code from MIR validation in an attempt to detect type mismatches due
867833
/// to normalization failure.
868834
fn visit_projection_elem(

‎tests/mir-opt/inline/unchecked_shifts.unchecked_shl_unsigned_smaller.Inline.diff‎

Lines changed: 115 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,51 @@
1212
+ debug rhs => _4; // in scope 1 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
1313
+ let mut _5: u16; // in scope 1 at $SRC_DIR/core/src/num/mod.rs:LL:COL
1414
+ let mut _6: (u32,); // in scope 1 at $SRC_DIR/core/src/num/mod.rs:LL:COL
15+
+ let mut _7: u32; // in scope 1 at $SRC_DIR/core/src/num/mod.rs:LL:COL
1516
+ scope 2 {
17+
+ scope 3 (inlined core::num::<impl u16>::unchecked_shl::conv) { // at $SRC_DIR/core/src/num/mod.rs:LL:COL
18+
+ debug x => _7; // in scope 3 at $SRC_DIR/core/src/num/mod.rs:LL:COL
19+
+ let mut _8: std::option::Option<u16>; // in scope 3 at $SRC_DIR/core/src/num/mod.rs:LL:COL
20+
+ let mut _9: std::result::Result<u16, std::num::TryFromIntError>; // in scope 3 at $SRC_DIR/core/src/num/mod.rs:LL:COL
21+
+ scope 4 {
22+
+ scope 5 (inlined <u32 as TryInto<u16>>::try_into) { // at $SRC_DIR/core/src/num/mod.rs:LL:COL
23+
+ debug self => _7; // in scope 5 at $SRC_DIR/core/src/convert/mod.rs:LL:COL
24+
+ scope 6 (inlined convert::num::<impl TryFrom<u32> for u16>::try_from) { // at $SRC_DIR/core/src/convert/mod.rs:LL:COL
25+
+ debug u => _7; // in scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
26+
+ let mut _10: bool; // in scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
27+
+ let mut _11: u32; // in scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
28+
+ let mut _12: u16; // in scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
29+
+ }
30+
+ }
31+
+ scope 7 (inlined Result::<u16, TryFromIntError>::ok) { // at $SRC_DIR/core/src/num/mod.rs:LL:COL
32+
+ debug self => _9; // in scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
33+
+ let mut _13: isize; // in scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
34+
+ let _14: u16; // in scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
35+
+ scope 8 {
36+
+ debug x => _14; // in scope 8 at $SRC_DIR/core/src/result.rs:LL:COL
37+
+ }
38+
+ }
39+
+ scope 9 (inlined #[track_caller] Option::<u16>::unwrap_unchecked) { // at $SRC_DIR/core/src/num/mod.rs:LL:COL
40+
+ debug self => _8; // in scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
41+
+ let mut _15: &std::option::Option<u16>; // in scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
42+
+ let mut _16: isize; // in scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
43+
+ scope 10 {
44+
+ debug val => _5; // in scope 10 at $SRC_DIR/core/src/option.rs:LL:COL
45+
+ }
46+
+ scope 11 {
47+
+ scope 13 (inlined unreachable_unchecked) { // at $SRC_DIR/core/src/option.rs:LL:COL
48+
+ scope 14 {
49+
+ scope 15 (inlined unreachable_unchecked::runtime) { // at $SRC_DIR/core/src/intrinsics.rs:LL:COL
50+
+ }
51+
+ }
52+
+ }
53+
+ }
54+
+ scope 12 (inlined Option::<u16>::is_some) { // at $SRC_DIR/core/src/option.rs:LL:COL
55+
+ debug self => _15; // in scope 12 at $SRC_DIR/core/src/option.rs:LL:COL
56+
+ }
57+
+ }
58+
+ }
59+
+ }
1660
+ }
1761
+ }
1862

@@ -22,30 +66,87 @@
2266
StorageLive(_4); // scope 0 at $DIR/unchecked_shifts.rs:+1:21: +1:22
2367
_4 = _2; // scope 0 at $DIR/unchecked_shifts.rs:+1:21: +1:22
2468
- _0 = core::num::<impl u16>::unchecked_shl(move _3, move _4) -> bb1; // scope 0 at $DIR/unchecked_shifts.rs:+1:5: +1:23
69+
- // mir::Constant
70+
- // + span: $DIR/unchecked_shifts.rs:11:7: 11:20
71+
- // + literal: Const { ty: unsafe fn(u16, u32) -> u16 {core::num::<impl u16>::unchecked_shl}, val: Value(<ZST>) }
2572
+ StorageLive(_5); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
2673
+ StorageLive(_6); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
2774
+ _6 = (_4,); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
28-
+ _5 = core::num::<impl u16>::unchecked_shl::conv(move (_6.0: u32)) -> bb1; // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
29-
// mir::Constant
30-
- // + span: $DIR/unchecked_shifts.rs:11:7: 11:20
31-
- // + literal: Const { ty: unsafe fn(u16, u32) -> u16 {core::num::<impl u16>::unchecked_shl}, val: Value(<ZST>) }
32-
+ // + span: $SRC_DIR/core/src/num/mod.rs:LL:COL
33-
+ // + literal: Const { ty: fn(u32) -> u16 {core::num::<impl u16>::unchecked_shl::conv}, val: Value(<ZST>) }
75+
+ StorageLive(_7); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
76+
+ _7 = move (_6.0: u32); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
77+
+ StorageLive(_8); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
78+
+ StorageLive(_9); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
79+
+ StorageLive(_10); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
80+
+ StorageLive(_11); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
81+
+ _11 = const 65535_u32; // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
82+
+ _10 = Gt(_7, move _11); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
83+
+ StorageDead(_11); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
84+
+ switchInt(move _10) -> [0: bb3, otherwise: bb2]; // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
3485
}
3586

3687
bb1: {
37-
+ StorageDead(_6); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
38-
+ _0 = unchecked_shl::<u16>(_3, move _5) -> [return: bb2, unwind unreachable]; // scope 2 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
39-
+ // mir::Constant
40-
+ // + span: $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
41-
+ // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(u16, u16) -> u16 {unchecked_shl::<u16>}, val: Value(<ZST>) }
42-
+ }
43-
+
44-
+ bb2: {
4588
+ StorageDead(_5); // scope 2 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
4689
StorageDead(_4); // scope 0 at $DIR/unchecked_shifts.rs:+1:22: +1:23
4790
StorageDead(_3); // scope 0 at $DIR/unchecked_shifts.rs:+1:22: +1:23
4891
return; // scope 0 at $DIR/unchecked_shifts.rs:+2:2: +2:2
92+
+ }
93+
+
94+
+ bb2: {
95+
+ _9 = Result::<u16, TryFromIntError>::Err(const TryFromIntError(())); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
96+
+ // mir::Constant
97+
+ // + span: no-location
98+
+ // + literal: Const { ty: TryFromIntError, val: Value(<ZST>) }
99+
+ goto -> bb4; // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
100+
+ }
101+
+
102+
+ bb3: {
103+
+ StorageLive(_12); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
104+
+ _12 = _7 as u16 (IntToInt); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
105+
+ _9 = Result::<u16, TryFromIntError>::Ok(move _12); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
106+
+ StorageDead(_12); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
107+
+ goto -> bb4; // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
108+
+ }
109+
+
110+
+ bb4: {
111+
+ StorageDead(_10); // scope 6 at $SRC_DIR/core/src/convert/num.rs:LL:COL
112+
+ StorageLive(_14); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
113+
+ _13 = discriminant(_9); // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
114+
+ switchInt(move _13) -> [0: bb7, 1: bb5, otherwise: bb6]; // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
115+
+ }
116+
+
117+
+ bb5: {
118+
+ _8 = Option::<u16>::None; // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
119+
+ goto -> bb8; // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
120+
+ }
121+
+
122+
+ bb6: {
123+
+ unreachable; // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
124+
+ }
125+
+
126+
+ bb7: {
127+
+ _14 = move ((_9 as Ok).0: u16); // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
128+
+ _8 = Option::<u16>::Some(move _14); // scope 8 at $SRC_DIR/core/src/result.rs:LL:COL
129+
+ goto -> bb8; // scope 7 at $SRC_DIR/core/src/result.rs:LL:COL
130+
+ }
131+
+
132+
+ bb8: {
133+
+ StorageDead(_14); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
134+
+ StorageDead(_9); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
135+
+ StorageLive(_15); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
136+
+ _16 = discriminant(_8); // scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
137+
+ switchInt(move _16) -> [1: bb9, otherwise: bb6]; // scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
138+
+ }
139+
+
140+
+ bb9: {
141+
+ _5 = move ((_8 as Some).0: u16); // scope 9 at $SRC_DIR/core/src/option.rs:LL:COL
142+
+ StorageDead(_15); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
143+
+ StorageDead(_8); // scope 4 at $SRC_DIR/core/src/num/mod.rs:LL:COL
144+
+ StorageDead(_7); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
145+
+ StorageDead(_6); // scope 2 at $SRC_DIR/core/src/num/mod.rs:LL:COL
146+
+ _0 = unchecked_shl::<u16>(_3, move _5) -> [return: bb1, unwind unreachable]; // scope 2 at $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
147+
+ // mir::Constant
148+
+ // + span: $SRC_DIR/core/src/num/uint_macros.rs:LL:COL
149+
+ // + literal: Const { ty: unsafe extern "rust-intrinsic" fn(u16, u16) -> u16 {unchecked_shl::<u16>}, val: Value(<ZST>) }
49150
}
50151
}
51152

0 commit comments

Comments
 (0)