Skip to content

Commit f47df31

Browse files
committed
Auto merge of #76683 - simonvandel:inst-combine-deref, r=oli-obk
Add optimization to avoid load of address Look for the sequence ```rust _2 = &_1; ... _5 = (*_2) ``` in which we can replace the last statement with `_5 = _1` to avoid the load of _2
2 parents fb1dc34 + dfc469d commit f47df31

10 files changed

+506
-12
lines changed

compiler/rustc_mir/src/transform/instcombine.rs

+115-2
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,14 @@ use crate::transform::{MirPass, MirSource};
44
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
55
use rustc_hir::Mutability;
66
use rustc_index::vec::Idx;
7-
use rustc_middle::mir::visit::{MutVisitor, Visitor};
87
use rustc_middle::mir::{
9-
BinOp, Body, Constant, Local, Location, Operand, Place, PlaceRef, ProjectionElem, Rvalue,
8+
visit::PlaceContext,
9+
visit::{MutVisitor, Visitor},
10+
Statement,
11+
};
12+
use rustc_middle::mir::{
13+
BinOp, Body, BorrowKind, Constant, Local, Location, Operand, Place, PlaceRef, ProjectionElem,
14+
Rvalue,
1015
};
1116
use rustc_middle::ty::{self, TyCtxt};
1217
use std::mem;
@@ -71,10 +76,36 @@ impl<'tcx> MutVisitor<'tcx> for InstCombineVisitor<'tcx> {
7176
*rvalue = Rvalue::Use(operand);
7277
}
7378

79+
if let Some(place) = self.optimizations.unneeded_deref.remove(&location) {
80+
debug!("unneeded_deref: replacing {:?} with {:?}", rvalue, place);
81+
*rvalue = Rvalue::Use(Operand::Copy(place));
82+
}
83+
7484
self.super_rvalue(rvalue, location)
7585
}
7686
}
7787

88+
struct MutatingUseVisitor {
89+
has_mutating_use: bool,
90+
local_to_look_for: Local,
91+
}
92+
93+
impl MutatingUseVisitor {
94+
fn has_mutating_use_in_stmt(local: Local, stmt: &Statement<'tcx>, location: Location) -> bool {
95+
let mut _self = Self { has_mutating_use: false, local_to_look_for: local };
96+
_self.visit_statement(stmt, location);
97+
_self.has_mutating_use
98+
}
99+
}
100+
101+
impl<'tcx> Visitor<'tcx> for MutatingUseVisitor {
102+
fn visit_local(&mut self, local: &Local, context: PlaceContext, _: Location) {
103+
if *local == self.local_to_look_for {
104+
self.has_mutating_use |= context.is_mutating_use();
105+
}
106+
}
107+
}
108+
78109
/// Finds optimization opportunities on the MIR.
79110
struct OptimizationFinder<'b, 'tcx> {
80111
body: &'b Body<'tcx>,
@@ -87,6 +118,85 @@ impl OptimizationFinder<'b, 'tcx> {
87118
OptimizationFinder { body, tcx, optimizations: OptimizationList::default() }
88119
}
89120

121+
fn find_deref_of_address(&mut self, rvalue: &Rvalue<'tcx>, location: Location) -> Option<()> {
122+
// Look for the sequence
123+
//
124+
// _2 = &_1;
125+
// ...
126+
// _5 = (*_2);
127+
//
128+
// which we can replace the last statement with `_5 = _1;` to avoid the load of `_2`.
129+
if let Rvalue::Use(op) = rvalue {
130+
let local_being_derefed = match op.place()?.as_ref() {
131+
PlaceRef { local, projection: [ProjectionElem::Deref] } => Some(local),
132+
_ => None,
133+
}?;
134+
135+
let stmt_index = location.statement_index;
136+
// Look behind for statement that assigns the local from a address of operator.
137+
// 6 is chosen as a heuristic determined by seeing the number of times
138+
// the optimization kicked in compiling rust std.
139+
let lower_index = stmt_index.saturating_sub(6);
140+
let statements_to_look_in = self.body.basic_blocks()[location.block].statements
141+
[lower_index..stmt_index]
142+
.iter()
143+
.rev();
144+
for stmt in statements_to_look_in {
145+
match &stmt.kind {
146+
// Exhaustive match on statements to detect conditions that warrant we bail out of the optimization.
147+
rustc_middle::mir::StatementKind::Assign(box (l, r))
148+
if l.local == local_being_derefed =>
149+
{
150+
match r {
151+
// Looking for immutable reference e.g _local_being_deref = &_1;
152+
Rvalue::Ref(
153+
_,
154+
// Only apply the optimization if it is an immutable borrow.
155+
BorrowKind::Shared,
156+
place_taken_address_of,
157+
) => {
158+
self.optimizations
159+
.unneeded_deref
160+
.insert(location, *place_taken_address_of);
161+
return Some(());
162+
}
163+
164+
// We found an assignment of `local_being_deref` that is not an immutable ref, e.g the following sequence
165+
// _2 = &_1;
166+
// _3 = &5
167+
// _2 = _3; <-- this means it is no longer valid to replace the last statement with `_5 = _1;`
168+
// _5 = (*_2);
169+
_ => return None,
170+
}
171+
}
172+
173+
// Inline asm can do anything, so bail out of the optimization.
174+
rustc_middle::mir::StatementKind::LlvmInlineAsm(_) => return None,
175+
176+
// Check that `local_being_deref` is not being used in a mutating way which can cause misoptimization.
177+
rustc_middle::mir::StatementKind::Assign(box (_, _))
178+
| rustc_middle::mir::StatementKind::Coverage(_)
179+
| rustc_middle::mir::StatementKind::Nop
180+
| rustc_middle::mir::StatementKind::FakeRead(_, _)
181+
| rustc_middle::mir::StatementKind::StorageLive(_)
182+
| rustc_middle::mir::StatementKind::StorageDead(_)
183+
| rustc_middle::mir::StatementKind::Retag(_, _)
184+
| rustc_middle::mir::StatementKind::AscribeUserType(_, _)
185+
| rustc_middle::mir::StatementKind::SetDiscriminant { .. } => {
186+
if MutatingUseVisitor::has_mutating_use_in_stmt(
187+
local_being_derefed,
188+
stmt,
189+
location,
190+
) {
191+
return None;
192+
}
193+
}
194+
}
195+
}
196+
}
197+
Some(())
198+
}
199+
90200
fn find_unneeded_equality_comparison(&mut self, rvalue: &Rvalue<'tcx>, location: Location) {
91201
// find Ne(_place, false) or Ne(false, _place)
92202
// or Eq(_place, true) or Eq(true, _place)
@@ -153,6 +263,8 @@ impl Visitor<'tcx> for OptimizationFinder<'b, 'tcx> {
153263
}
154264
}
155265

266+
let _ = self.find_deref_of_address(rvalue, location);
267+
156268
self.find_unneeded_equality_comparison(rvalue, location);
157269

158270
self.super_rvalue(rvalue, location)
@@ -164,4 +276,5 @@ struct OptimizationList<'tcx> {
164276
and_stars: FxHashSet<Location>,
165277
arrays_lengths: FxHashMap<Location, Constant<'tcx>>,
166278
unneeded_equality_comparison: FxHashMap<Location, Operand<'tcx>>,
279+
unneeded_deref: FxHashMap<Location, Place<'tcx>>,
167280
}

src/test/mir-opt/const_prop/ref_deref.main.ConstProp.diff

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
// + span: $DIR/ref_deref.rs:5:6: 5:10
2020
// + literal: Const { ty: &i32, val: Unevaluated(WithOptConstParam { did: DefId(0:3 ~ ref_deref[317d]::main[0]), const_param_did: None }, [], Some(promoted[0])) }
2121
_2 = _4; // scope 0 at $DIR/ref_deref.rs:5:6: 5:10
22-
- _1 = (*_2); // scope 0 at $DIR/ref_deref.rs:5:5: 5:10
22+
- _1 = (*_4); // scope 0 at $DIR/ref_deref.rs:5:5: 5:10
2323
+ _1 = const 4_i32; // scope 0 at $DIR/ref_deref.rs:5:5: 5:10
2424
StorageDead(_2); // scope 0 at $DIR/ref_deref.rs:5:10: 5:11
2525
StorageDead(_1); // scope 0 at $DIR/ref_deref.rs:5:10: 5:11

src/test/mir-opt/const_prop/ref_deref_project.main.ConstProp.diff

+1-1
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
// + span: $DIR/ref_deref_project.rs:5:6: 5:17
2020
// + literal: Const { ty: &(i32, i32), val: Unevaluated(WithOptConstParam { did: DefId(0:3 ~ ref_deref_project[317d]::main[0]), const_param_did: None }, [], Some(promoted[0])) }
2121
_2 = &((*_4).1: i32); // scope 0 at $DIR/ref_deref_project.rs:5:6: 5:17
22-
_1 = (*_2); // scope 0 at $DIR/ref_deref_project.rs:5:5: 5:17
22+
_1 = ((*_4).1: i32); // scope 0 at $DIR/ref_deref_project.rs:5:5: 5:17
2323
StorageDead(_2); // scope 0 at $DIR/ref_deref_project.rs:5:17: 5:18
2424
StorageDead(_1); // scope 0 at $DIR/ref_deref_project.rs:5:17: 5:18
2525
_0 = const (); // scope 0 at $DIR/ref_deref_project.rs:4:11: 6:2

src/test/mir-opt/inline/inline_closure_borrows_arg.foo.Inline.after.mir

+12-8
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,14 @@ fn foo(_1: T, _2: &i32) -> i32 {
99
let mut _5: (&i32, &i32); // in scope 0 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
1010
let mut _6: &i32; // in scope 0 at $DIR/inline-closure-borrows-arg.rs:16:7: 16:8
1111
let mut _7: &i32; // in scope 0 at $DIR/inline-closure-borrows-arg.rs:16:10: 16:11
12-
let mut _8: &i32; // in scope 0 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
1312
let mut _9: &i32; // in scope 0 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
13+
let mut _10: &i32; // in scope 0 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
1414
scope 1 {
1515
debug x => _3; // in scope 1 at $DIR/inline-closure-borrows-arg.rs:12:9: 12:10
1616
scope 2 {
17-
debug r => _8; // in scope 2 at $DIR/inline-closure-borrows-arg.rs:12:14: 12:15
18-
debug _s => _9; // in scope 2 at $DIR/inline-closure-borrows-arg.rs:12:23: 12:25
17+
debug r => _9; // in scope 2 at $DIR/inline-closure-borrows-arg.rs:12:14: 12:15
18+
debug _s => _10; // in scope 2 at $DIR/inline-closure-borrows-arg.rs:12:23: 12:25
19+
let _8: &i32; // in scope 2 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
1920
}
2021
}
2122
scope 3 {
@@ -33,13 +34,16 @@ fn foo(_1: T, _2: &i32) -> i32 {
3334
_7 = &(*_2); // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:10: 16:11
3435
(_5.0: &i32) = move _6; // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
3536
(_5.1: &i32) = move _7; // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
36-
StorageLive(_8); // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
37-
_8 = move (_5.0: &i32); // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
3837
StorageLive(_9); // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
39-
_9 = move (_5.1: &i32); // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
40-
_0 = (*_8); // scope 3 at $DIR/inline-closure-borrows-arg.rs:14:9: 14:18
38+
_9 = move (_5.0: &i32); // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
39+
StorageLive(_10); // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
40+
_10 = move (_5.1: &i32); // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
41+
StorageLive(_8); // scope 2 at $DIR/inline-closure-borrows-arg.rs:13:13: 13:21
42+
_8 = _9; // scope 2 at $DIR/inline-closure-borrows-arg.rs:13:24: 13:27
43+
_0 = (*_9); // scope 3 at $DIR/inline-closure-borrows-arg.rs:14:9: 14:18
44+
StorageDead(_8); // scope 2 at $DIR/inline-closure-borrows-arg.rs:15:5: 15:6
45+
StorageDead(_10); // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
4146
StorageDead(_9); // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
42-
StorageDead(_8); // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:5: 16:12
4347
StorageDead(_7); // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:11: 16:12
4448
StorageDead(_6); // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:11: 16:12
4549
StorageDead(_5); // scope 1 at $DIR/inline-closure-borrows-arg.rs:16:11: 16:12
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
- // MIR for `deep_opt` before InstCombine
2+
+ // MIR for `deep_opt` after InstCombine
3+
4+
fn deep_opt() -> (u64, u64, u64) {
5+
let mut _0: (u64, u64, u64); // return place in scope 0 at $DIR/inst_combine_deref.rs:11:18: 11:33
6+
let _1: u64; // in scope 0 at $DIR/inst_combine_deref.rs:12:9: 12:11
7+
let mut _10: u64; // in scope 0 at $DIR/inst_combine_deref.rs:21:6: 21:8
8+
let mut _11: u64; // in scope 0 at $DIR/inst_combine_deref.rs:21:10: 21:12
9+
let mut _12: u64; // in scope 0 at $DIR/inst_combine_deref.rs:21:14: 21:16
10+
scope 1 {
11+
debug x1 => _1; // in scope 1 at $DIR/inst_combine_deref.rs:12:9: 12:11
12+
let _2: u64; // in scope 1 at $DIR/inst_combine_deref.rs:13:9: 13:11
13+
scope 2 {
14+
debug x2 => _2; // in scope 2 at $DIR/inst_combine_deref.rs:13:9: 13:11
15+
let _3: u64; // in scope 2 at $DIR/inst_combine_deref.rs:14:9: 14:11
16+
scope 3 {
17+
debug x3 => _3; // in scope 3 at $DIR/inst_combine_deref.rs:14:9: 14:11
18+
let _4: &u64; // in scope 3 at $DIR/inst_combine_deref.rs:15:9: 15:11
19+
scope 4 {
20+
debug y1 => _4; // in scope 4 at $DIR/inst_combine_deref.rs:15:9: 15:11
21+
let _5: &u64; // in scope 4 at $DIR/inst_combine_deref.rs:16:9: 16:11
22+
scope 5 {
23+
debug y2 => _5; // in scope 5 at $DIR/inst_combine_deref.rs:16:9: 16:11
24+
let _6: &u64; // in scope 5 at $DIR/inst_combine_deref.rs:17:9: 17:11
25+
scope 6 {
26+
debug y3 => _6; // in scope 6 at $DIR/inst_combine_deref.rs:17:9: 17:11
27+
let _7: u64; // in scope 6 at $DIR/inst_combine_deref.rs:18:9: 18:11
28+
scope 7 {
29+
debug z1 => _7; // in scope 7 at $DIR/inst_combine_deref.rs:18:9: 18:11
30+
let _8: u64; // in scope 7 at $DIR/inst_combine_deref.rs:19:9: 19:11
31+
scope 8 {
32+
debug z2 => _8; // in scope 8 at $DIR/inst_combine_deref.rs:19:9: 19:11
33+
let _9: u64; // in scope 8 at $DIR/inst_combine_deref.rs:20:9: 20:11
34+
scope 9 {
35+
debug z3 => _9; // in scope 9 at $DIR/inst_combine_deref.rs:20:9: 20:11
36+
}
37+
}
38+
}
39+
}
40+
}
41+
}
42+
}
43+
}
44+
}
45+
46+
bb0: {
47+
StorageLive(_1); // scope 0 at $DIR/inst_combine_deref.rs:12:9: 12:11
48+
_1 = const 1_u64; // scope 0 at $DIR/inst_combine_deref.rs:12:14: 12:15
49+
StorageLive(_2); // scope 1 at $DIR/inst_combine_deref.rs:13:9: 13:11
50+
_2 = const 2_u64; // scope 1 at $DIR/inst_combine_deref.rs:13:14: 13:15
51+
StorageLive(_3); // scope 2 at $DIR/inst_combine_deref.rs:14:9: 14:11
52+
_3 = const 3_u64; // scope 2 at $DIR/inst_combine_deref.rs:14:14: 14:15
53+
StorageLive(_4); // scope 3 at $DIR/inst_combine_deref.rs:15:9: 15:11
54+
_4 = &_1; // scope 3 at $DIR/inst_combine_deref.rs:15:14: 15:17
55+
StorageLive(_5); // scope 4 at $DIR/inst_combine_deref.rs:16:9: 16:11
56+
_5 = &_2; // scope 4 at $DIR/inst_combine_deref.rs:16:14: 16:17
57+
StorageLive(_6); // scope 5 at $DIR/inst_combine_deref.rs:17:9: 17:11
58+
_6 = &_3; // scope 5 at $DIR/inst_combine_deref.rs:17:14: 17:17
59+
StorageLive(_7); // scope 6 at $DIR/inst_combine_deref.rs:18:9: 18:11
60+
- _7 = (*_4); // scope 6 at $DIR/inst_combine_deref.rs:18:14: 18:17
61+
+ _7 = _1; // scope 6 at $DIR/inst_combine_deref.rs:18:14: 18:17
62+
StorageLive(_8); // scope 7 at $DIR/inst_combine_deref.rs:19:9: 19:11
63+
- _8 = (*_5); // scope 7 at $DIR/inst_combine_deref.rs:19:14: 19:17
64+
+ _8 = _2; // scope 7 at $DIR/inst_combine_deref.rs:19:14: 19:17
65+
StorageLive(_9); // scope 8 at $DIR/inst_combine_deref.rs:20:9: 20:11
66+
- _9 = (*_6); // scope 8 at $DIR/inst_combine_deref.rs:20:14: 20:17
67+
+ _9 = _3; // scope 8 at $DIR/inst_combine_deref.rs:20:14: 20:17
68+
StorageLive(_10); // scope 9 at $DIR/inst_combine_deref.rs:21:6: 21:8
69+
_10 = _7; // scope 9 at $DIR/inst_combine_deref.rs:21:6: 21:8
70+
StorageLive(_11); // scope 9 at $DIR/inst_combine_deref.rs:21:10: 21:12
71+
_11 = _8; // scope 9 at $DIR/inst_combine_deref.rs:21:10: 21:12
72+
StorageLive(_12); // scope 9 at $DIR/inst_combine_deref.rs:21:14: 21:16
73+
_12 = _9; // scope 9 at $DIR/inst_combine_deref.rs:21:14: 21:16
74+
(_0.0: u64) = move _10; // scope 9 at $DIR/inst_combine_deref.rs:21:5: 21:17
75+
(_0.1: u64) = move _11; // scope 9 at $DIR/inst_combine_deref.rs:21:5: 21:17
76+
(_0.2: u64) = move _12; // scope 9 at $DIR/inst_combine_deref.rs:21:5: 21:17
77+
StorageDead(_12); // scope 9 at $DIR/inst_combine_deref.rs:21:16: 21:17
78+
StorageDead(_11); // scope 9 at $DIR/inst_combine_deref.rs:21:16: 21:17
79+
StorageDead(_10); // scope 9 at $DIR/inst_combine_deref.rs:21:16: 21:17
80+
StorageDead(_9); // scope 8 at $DIR/inst_combine_deref.rs:22:1: 22:2
81+
StorageDead(_8); // scope 7 at $DIR/inst_combine_deref.rs:22:1: 22:2
82+
StorageDead(_7); // scope 6 at $DIR/inst_combine_deref.rs:22:1: 22:2
83+
StorageDead(_6); // scope 5 at $DIR/inst_combine_deref.rs:22:1: 22:2
84+
StorageDead(_5); // scope 4 at $DIR/inst_combine_deref.rs:22:1: 22:2
85+
StorageDead(_4); // scope 3 at $DIR/inst_combine_deref.rs:22:1: 22:2
86+
StorageDead(_3); // scope 2 at $DIR/inst_combine_deref.rs:22:1: 22:2
87+
StorageDead(_2); // scope 1 at $DIR/inst_combine_deref.rs:22:1: 22:2
88+
StorageDead(_1); // scope 0 at $DIR/inst_combine_deref.rs:22:1: 22:2
89+
return; // scope 0 at $DIR/inst_combine_deref.rs:22:2: 22:2
90+
}
91+
}
92+

0 commit comments

Comments
 (0)