Skip to content

Commit 4236da5

Browse files
scottmcmscottmcm
committedJun 20, 2024
Give inlining bonuses to things that optimize out
1 parent f334951 commit 4236da5

7 files changed

+355
-64
lines changed
 

‎compiler/rustc_mir_transform/src/cost_checker.rs

+63-20
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use rustc_middle::bug;
12
use rustc_middle::mir::visit::*;
23
use rustc_middle::mir::*;
34
use rustc_middle::ty::{self, ParamEnv, Ty, TyCtxt};
@@ -6,6 +7,8 @@ const INSTR_COST: usize = 5;
67
const CALL_PENALTY: usize = 25;
78
const LANDINGPAD_PENALTY: usize = 50;
89
const RESUME_PENALTY: usize = 45;
10+
const LARGE_SWITCH_PENALTY: usize = 20;
11+
const CONST_SWITCH_BONUS: usize = 10;
912

1013
/// Verify that the callee body is compatible with the caller.
1114
#[derive(Clone)]
@@ -42,36 +45,49 @@ impl<'b, 'tcx> CostChecker<'b, 'tcx> {
4245
}
4346

4447
impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> {
45-
fn visit_statement(&mut self, statement: &Statement<'tcx>, _: Location) {
46-
// Don't count StorageLive/StorageDead in the inlining cost.
48+
fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) {
49+
// Most costs are in rvalues and terminators, not in statements.
4750
match statement.kind {
48-
StatementKind::StorageLive(_)
49-
| StatementKind::StorageDead(_)
50-
| StatementKind::Deinit(_)
51-
| StatementKind::Nop => {}
51+
StatementKind::Intrinsic(ref ndi) => {
52+
self.penalty += match **ndi {
53+
NonDivergingIntrinsic::Assume(..) => INSTR_COST,
54+
NonDivergingIntrinsic::CopyNonOverlapping(..) => CALL_PENALTY,
55+
};
56+
}
57+
_ => self.super_statement(statement, location),
58+
}
59+
}
60+
61+
fn visit_rvalue(&mut self, rvalue: &Rvalue<'tcx>, _location: Location) {
62+
match rvalue {
63+
Rvalue::NullaryOp(NullOp::UbChecks, ..) if !self.tcx.sess.ub_checks() => {
64+
// If this is in optimized MIR it's because it's used later,
65+
// so if we don't need UB checks this session, give a bonus
66+
// here to offset the cost of the call later.
67+
self.bonus += CALL_PENALTY;
68+
}
69+
// These are essentially constants that didn't end up in an Operand,
70+
// so treat them as also being free.
71+
Rvalue::NullaryOp(..) => {}
5272
_ => self.penalty += INSTR_COST,
5373
}
5474
}
5575

5676
fn visit_terminator(&mut self, terminator: &Terminator<'tcx>, _: Location) {
57-
let tcx = self.tcx;
58-
match terminator.kind {
59-
TerminatorKind::Drop { ref place, unwind, .. } => {
77+
match &terminator.kind {
78+
TerminatorKind::Drop { place, unwind, .. } => {
6079
// If the place doesn't actually need dropping, treat it like a regular goto.
61-
let ty = self.instantiate_ty(place.ty(self.callee_body, tcx).ty);
62-
if ty.needs_drop(tcx, self.param_env) {
80+
let ty = self.instantiate_ty(place.ty(self.callee_body, self.tcx).ty);
81+
if ty.needs_drop(self.tcx, self.param_env) {
6382
self.penalty += CALL_PENALTY;
6483
if let UnwindAction::Cleanup(_) = unwind {
6584
self.penalty += LANDINGPAD_PENALTY;
6685
}
67-
} else {
68-
self.penalty += INSTR_COST;
6986
}
7087
}
71-
TerminatorKind::Call { func: Operand::Constant(ref f), unwind, .. } => {
72-
let fn_ty = self.instantiate_ty(f.const_.ty());
73-
self.penalty += if let ty::FnDef(def_id, _) = *fn_ty.kind()
74-
&& tcx.intrinsic(def_id).is_some()
88+
TerminatorKind::Call { func, unwind, .. } => {
89+
self.penalty += if let Some((def_id, ..)) = func.const_fn_def()
90+
&& self.tcx.intrinsic(def_id).is_some()
7591
{
7692
// Don't give intrinsics the extra penalty for calls
7793
INSTR_COST
@@ -82,8 +98,25 @@ impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> {
8298
self.penalty += LANDINGPAD_PENALTY;
8399
}
84100
}
85-
TerminatorKind::Assert { unwind, .. } => {
86-
self.penalty += CALL_PENALTY;
101+
TerminatorKind::SwitchInt { discr, targets } => {
102+
if discr.constant().is_some() {
103+
// Not only will this become a `Goto`, but likely other
104+
// things will be removable as unreachable.
105+
self.bonus += CONST_SWITCH_BONUS;
106+
} else if targets.all_targets().len() > 3 {
107+
// More than false/true/unreachable gets extra cost.
108+
self.penalty += LARGE_SWITCH_PENALTY;
109+
} else {
110+
self.penalty += INSTR_COST;
111+
}
112+
}
113+
TerminatorKind::Assert { unwind, msg, .. } => {
114+
self.penalty +=
115+
if msg.is_optional_overflow_check() && !self.tcx.sess.overflow_checks() {
116+
INSTR_COST
117+
} else {
118+
CALL_PENALTY
119+
};
87120
if let UnwindAction::Cleanup(_) = unwind {
88121
self.penalty += LANDINGPAD_PENALTY;
89122
}
@@ -95,7 +128,17 @@ impl<'tcx> Visitor<'tcx> for CostChecker<'_, 'tcx> {
95128
self.penalty += LANDINGPAD_PENALTY;
96129
}
97130
}
98-
_ => self.penalty += INSTR_COST,
131+
TerminatorKind::Unreachable => {
132+
self.bonus += INSTR_COST;
133+
}
134+
TerminatorKind::Goto { .. } | TerminatorKind::Return => {}
135+
TerminatorKind::UnwindTerminate(..) => {}
136+
kind @ (TerminatorKind::FalseUnwind { .. }
137+
| TerminatorKind::FalseEdge { .. }
138+
| TerminatorKind::Yield { .. }
139+
| TerminatorKind::CoroutineDrop) => {
140+
bug!("{kind:?} should not be in runtime MIR");
141+
}
99142
}
100143
}
101144
}

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

+5-1
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,12 @@
33

44
// CHECK-LABEL: @write_u8_variant_a
55
// CHECK-NEXT: {{.*}}:
6-
// CHECK-NEXT: getelementptr
76
// CHECK-NEXT: icmp ugt
7+
// CHECK-NEXT: getelementptr
8+
// CHECK-NEXT: select i1 {{.+}} null
9+
// CHECK-NEXT: insertvalue
10+
// CHECK-NEXT: insertvalue
11+
// CHECK-NEXT: ret
812
#[no_mangle]
913
pub fn write_u8_variant_a(bytes: &mut [u8], buf: u8, offset: usize) -> Option<&mut [u8]> {
1014
let buf = buf.to_le_bytes();

‎tests/crashes/123893.rs

+4
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,14 @@ fn generic_impl<T>() {
1111
impl<T> MagicTrait for T {
1212
const IS_BIG: bool = true;
1313
}
14+
more_cost();
1415
if T::IS_BIG {
1516
big_impl::<i32>();
1617
}
1718
}
1819

1920
#[inline(never)]
2021
fn big_impl<T>() {}
22+
23+
#[inline(never)]
24+
fn more_cost() {}

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

+76-1
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,87 @@ fn step_forward(_1: u16, _2: usize) -> u16 {
44
debug x => _1;
55
debug n => _2;
66
let mut _0: u16;
7+
scope 1 (inlined <u16 as Step>::forward) {
8+
let mut _8: u16;
9+
scope 2 {
10+
}
11+
scope 3 (inlined <u16 as Step>::forward_checked) {
12+
scope 4 {
13+
scope 6 (inlined core::num::<impl u16>::checked_add) {
14+
let mut _7: bool;
15+
scope 7 {
16+
}
17+
scope 8 (inlined core::num::<impl u16>::overflowing_add) {
18+
let mut _5: (u16, bool);
19+
let _6: bool;
20+
scope 9 {
21+
}
22+
}
23+
}
24+
}
25+
scope 5 (inlined convert::num::ptr_try_from_impls::<impl TryFrom<usize> for u16>::try_from) {
26+
let mut _3: bool;
27+
let mut _4: u16;
28+
}
29+
}
30+
scope 10 (inlined Option::<u16>::is_none) {
31+
scope 11 (inlined Option::<u16>::is_some) {
32+
}
33+
}
34+
scope 12 (inlined core::num::<impl u16>::wrapping_add) {
35+
}
36+
}
737

838
bb0: {
9-
_0 = <u16 as Step>::forward(move _1, move _2) -> [return: bb1, unwind unreachable];
39+
StorageLive(_4);
40+
StorageLive(_3);
41+
_3 = Gt(_2, const 65535_usize);
42+
switchInt(move _3) -> [0: bb1, otherwise: bb5];
1043
}
1144

1245
bb1: {
46+
_4 = _2 as u16 (IntToInt);
47+
StorageDead(_3);
48+
StorageLive(_6);
49+
StorageLive(_5);
50+
_5 = AddWithOverflow(_1, _4);
51+
_6 = (_5.1: bool);
52+
StorageDead(_5);
53+
StorageLive(_7);
54+
_7 = unlikely(move _6) -> [return: bb2, unwind unreachable];
55+
}
56+
57+
bb2: {
58+
switchInt(move _7) -> [0: bb3, otherwise: bb4];
59+
}
60+
61+
bb3: {
62+
StorageDead(_7);
63+
StorageDead(_6);
64+
goto -> bb7;
65+
}
66+
67+
bb4: {
68+
StorageDead(_7);
69+
StorageDead(_6);
70+
goto -> bb6;
71+
}
72+
73+
bb5: {
74+
StorageDead(_3);
75+
goto -> bb6;
76+
}
77+
78+
bb6: {
79+
assert(!const true, "attempt to compute `{} + {}`, which would overflow", const core::num::<impl u16>::MAX, const 1_u16) -> [success: bb7, unwind unreachable];
80+
}
81+
82+
bb7: {
83+
StorageLive(_8);
84+
_8 = _2 as u16 (IntToInt);
85+
_0 = Add(_1, _8);
86+
StorageDead(_8);
87+
StorageDead(_4);
1388
return;
1489
}
1590
}

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

+76-1
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,87 @@ fn step_forward(_1: u16, _2: usize) -> u16 {
44
debug x => _1;
55
debug n => _2;
66
let mut _0: u16;
7+
scope 1 (inlined <u16 as Step>::forward) {
8+
let mut _8: u16;
9+
scope 2 {
10+
}
11+
scope 3 (inlined <u16 as Step>::forward_checked) {
12+
scope 4 {
13+
scope 6 (inlined core::num::<impl u16>::checked_add) {
14+
let mut _7: bool;
15+
scope 7 {
16+
}
17+
scope 8 (inlined core::num::<impl u16>::overflowing_add) {
18+
let mut _5: (u16, bool);
19+
let _6: bool;
20+
scope 9 {
21+
}
22+
}
23+
}
24+
}
25+
scope 5 (inlined convert::num::ptr_try_from_impls::<impl TryFrom<usize> for u16>::try_from) {
26+
let mut _3: bool;
27+
let mut _4: u16;
28+
}
29+
}
30+
scope 10 (inlined Option::<u16>::is_none) {
31+
scope 11 (inlined Option::<u16>::is_some) {
32+
}
33+
}
34+
scope 12 (inlined core::num::<impl u16>::wrapping_add) {
35+
}
36+
}
737

838
bb0: {
9-
_0 = <u16 as Step>::forward(move _1, move _2) -> [return: bb1, unwind continue];
39+
StorageLive(_4);
40+
StorageLive(_3);
41+
_3 = Gt(_2, const 65535_usize);
42+
switchInt(move _3) -> [0: bb1, otherwise: bb5];
1043
}
1144

1245
bb1: {
46+
_4 = _2 as u16 (IntToInt);
47+
StorageDead(_3);
48+
StorageLive(_6);
49+
StorageLive(_5);
50+
_5 = AddWithOverflow(_1, _4);
51+
_6 = (_5.1: bool);
52+
StorageDead(_5);
53+
StorageLive(_7);
54+
_7 = unlikely(move _6) -> [return: bb2, unwind unreachable];
55+
}
56+
57+
bb2: {
58+
switchInt(move _7) -> [0: bb3, otherwise: bb4];
59+
}
60+
61+
bb3: {
62+
StorageDead(_7);
63+
StorageDead(_6);
64+
goto -> bb7;
65+
}
66+
67+
bb4: {
68+
StorageDead(_7);
69+
StorageDead(_6);
70+
goto -> bb6;
71+
}
72+
73+
bb5: {
74+
StorageDead(_3);
75+
goto -> bb6;
76+
}
77+
78+
bb6: {
79+
assert(!const true, "attempt to compute `{} + {}`, which would overflow", const core::num::<impl u16>::MAX, const 1_u16) -> [success: bb7, unwind continue];
80+
}
81+
82+
bb7: {
83+
StorageLive(_8);
84+
_8 = _2 as u16 (IntToInt);
85+
_0 = Add(_1, _8);
86+
StorageDead(_8);
87+
StorageDead(_4);
1388
return;
1489
}
1590
}

0 commit comments

Comments
 (0)