Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit e730bff

Browse files
committed
Auto merge of rust-lang#136788 - FractalFir:prop_trivial_locals, r=<try>
[perf experiment] A MIR pass removing unneded temporary locals Experiment with early MIR optimization removing temporary locals # Motivation Currently, a lot of MIR contains unneded assigements to temporaries: ### Rust ```rust fn add_points(lhs:(f32,f32,f32),rhs:(f32,f32,f32))->(f32,f32,f32){ (lhs.0 + rhs.0, lhs.1 + rhs.1, lhs.2 + rhs.2) } ``` ### Orignal MIR ```mir bb0: { StorageLive(_3); StorageLive(_4); // _4 and _5 are not needed! _4 = copy (_1.0: f32); StorageLive(_5); _5 = copy (_2.0: f32); _3 = Add(move _4, move _5); StorageDead(_5); StorageDead(_4); StorageLive(_6); StorageLive(_7); // _7 and _8 are not needed! _7 = copy (_1.1: f32); StorageLive(_8); _8 = copy (_2.1: f32); _6 = Add(move _7, move _8); StorageDead(_8); StorageDead(_7); StorageLive(_9); StorageLive(_10); // _10 and _9 are not needed! _10 = copy (_1.2: f32); StorageLive(_11); _11 = copy (_2.2: f32); _9 = Add(move _10, move _11); StorageDead(_11); StorageDead(_10); _0 = (move _3, move _6, move _9); StorageDead(_9); StorageDead(_6); StorageDead(_3); return; } ``` This pass tries to remove as such many temporaries as possible. This leads to reduced MIR sizes, which should hopefully speed the next passes up. ```mir fn add_points(_1: (f32, f32, f32), _2: (f32, f32, f32)) -> (f32, f32, f32) { debug lhs => _1; debug rhs => _2; let mut _0: (f32, f32, f32); let mut _3: f32; let mut _4: f32; let mut _5: f32; bb0: { StorageLive(_3); _3 = Add(copy (_1.0: f32), copy (_2.0: f32)); StorageLive(_4); _4 = Add(copy (_1.1: f32), copy (_2.1: f32)); StorageLive(_5); _5 = Add(copy (_1.2: f32), copy (_2.2: f32)); _0 = (move _3, move _4, move _5); StorageDead(_5); StorageDead(_4); StorageDead(_3); return; } } ``` **This PR is not yet meant for merging!** The current version is still a bit from being done: it does not optimize locals used in calls, and some parts may need tweaking. Still, I belive it is at least worth timing at this point, which is why I am requesting a perf run.
2 parents 124cc92 + 3e56b39 commit e730bff

File tree

3 files changed

+171
-2
lines changed

3 files changed

+171
-2
lines changed

compiler/rustc_mir_transform/src/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ declare_passes! {
125125
mod check_undefined_transmutes : CheckUndefinedTransmutes;
126126
// This pass is public to allow external drivers to perform MIR cleanup
127127
pub mod cleanup_post_borrowck : CleanupPostBorrowck;
128-
128+
mod prop_trivial_locals : PropTrivialLocals;
129129
mod copy_prop : CopyProp;
130130
mod coroutine : StateTransform;
131131
mod coverage : InstrumentCoverage;
@@ -654,6 +654,7 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
654654
// Perform instsimplify before inline to eliminate some trivial calls (like clone
655655
// shims).
656656
&instsimplify::InstSimplify::BeforeInline,
657+
&prop_trivial_locals::PropTrivialLocals,
657658
// Perform inlining of `#[rustc_force_inline]`-annotated callees.
658659
&inline::ForceInline,
659660
// Perform inlining, which may add a lot of code.
Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
#![allow(unused_imports, unused_variables, dead_code)]
2+
//! This pass removes unneded temporary locals.
3+
//! Eg. This:
4+
//! ```
5+
//! _3 = copy _1;
6+
//! _4 = Add(_3, _2);
7+
//! ```
8+
//! Will get turned into this:
9+
//! ```
10+
//! _4 = Add(_1,_2)
11+
//! ```
12+
use rustc_index::IndexVec;
13+
use rustc_middle::mir::*;
14+
use rustc_middle::ty::TyCtxt;
15+
use tracing::{debug, trace};
16+
17+
pub(super) struct PropTrivialLocals;
18+
struct StatmentPos(BasicBlock, usize);
19+
impl StatmentPos {
20+
fn nop_out(self, blocks: &mut IndexVec<BasicBlock, BasicBlockData<'_>>) {
21+
blocks[self.0].statements[self.1].make_nop();
22+
}
23+
}
24+
fn propagate_operand<'tcx>(
25+
operand: &mut Operand<'tcx>,
26+
local: Local,
27+
local_replacement: &Operand<'tcx>,
28+
) -> bool {
29+
let place = match operand {
30+
Operand::Copy(place) | Operand::Move(place) => place,
31+
_ => return false,
32+
};
33+
if place.local != local {
34+
return false;
35+
}
36+
if place.projection.is_empty() {
37+
*operand = local_replacement.clone();
38+
return true;
39+
}
40+
return false;
41+
}
42+
impl<'tcx> crate::MirPass<'tcx> for PropTrivialLocals {
43+
fn is_enabled(&self, sess: &rustc_session::Session) -> bool {
44+
sess.mir_opt_level() > 1
45+
}
46+
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
47+
trace!("Running PropTrivialLocals on {:?}", body.source);
48+
loop {
49+
let mut dead_candidates = false;
50+
for (bid, block) in body.basic_blocks.as_mut().iter_enumerated_mut() {
51+
let mut iter = StatementPairIterator::new(bid, &mut block.statements);
52+
while let Some(StatementPair((a_idx, a), (b_idx, b))) = iter.next() {
53+
let (StatementKind::Assign(tmp_a), StatementKind::Assign(tmp_b)) =
54+
(&a.kind, &mut b.kind)
55+
else {
56+
continue;
57+
};
58+
let Some(loc_a) = tmp_a.0.as_local() else {
59+
continue;
60+
};
61+
let Rvalue::Use(ref src_a) = tmp_a.1 else {
62+
continue;
63+
};
64+
match &mut tmp_b.1 {
65+
Rvalue::BinaryOp(_, args) => {
66+
dead_candidates |= propagate_operand(&mut args.0, loc_a, src_a)
67+
|| propagate_operand(&mut args.1, loc_a, src_a);
68+
}
69+
// I could add a `| Rvalue::Use(arg)` here, but I have not encountered this pattern in compiler-generated MIR *ever*,
70+
// so it is likely not worth even checking for. Likevise, `Rvalue::Ref | Rvalue::RawPtr` also seems to never benfit from this opt.
71+
Rvalue::UnaryOp(_, arg)
72+
| Rvalue::Cast(_, arg, _)
73+
| Rvalue::Repeat(arg, _) => {
74+
dead_candidates |= propagate_operand(arg, loc_a, src_a);
75+
}
76+
Rvalue::Aggregate(_, args) => {
77+
dead_candidates |=
78+
args.iter_mut().any(|arg| propagate_operand(arg, loc_a, src_a))
79+
}
80+
_ => continue,
81+
}
82+
}
83+
}
84+
if dead_candidates {
85+
crate::simplify::remove_unused_definitions(body);
86+
} else {
87+
break;
88+
}
89+
}
90+
}
91+
92+
fn is_required(&self) -> bool {
93+
false
94+
}
95+
}
96+
struct DeadLocalCandidates {
97+
dead: Local,
98+
}
99+
struct StatementPair<'a, 'tcx>(
100+
(StatmentPos, &'a mut Statement<'tcx>),
101+
(StatmentPos, &'a mut Statement<'tcx>),
102+
);
103+
struct StatementPairIterator<'a, 'tcx> {
104+
curr_block: BasicBlock,
105+
curr_idx: usize,
106+
statements: &'a mut [Statement<'tcx>],
107+
}
108+
impl<'a, 'tcx> StatementPairIterator<'a, 'tcx> {
109+
fn new(curr_block: BasicBlock, statements: &'a mut [Statement<'tcx>]) -> Self {
110+
Self { curr_block, statements, curr_idx: 0 }
111+
}
112+
fn get_at<'s>(&'s mut self, idx_a: usize, idx_b: usize) -> StatementPair<'s, 'tcx> {
113+
// Some bounds checks
114+
assert!(idx_a < idx_b);
115+
assert!(idx_b < self.statements.len());
116+
let (part_a, reminder) = self.statements.split_at_mut(idx_a + 1);
117+
let (part_b, reminder) = reminder.split_at_mut((idx_b - part_a.len()) + 1);
118+
let a = &mut part_a[part_a.len() - 1];
119+
let b = &mut part_b[part_b.len() - 1];
120+
StatementPair(
121+
(StatmentPos(self.curr_block, idx_a), a),
122+
(StatmentPos(self.curr_block, idx_b), b),
123+
)
124+
}
125+
fn is_statement_irrelevant(statement: &Statement<'_>) -> bool {
126+
match statement.kind {
127+
StatementKind::Nop
128+
| StatementKind::StorageLive(..)
129+
| StatementKind::PlaceMention(..)
130+
| StatementKind::Coverage(..)
131+
| StatementKind::ConstEvalCounter => true,
132+
StatementKind::Assign(..)
133+
| StatementKind::FakeRead(..)
134+
| StatementKind::SetDiscriminant { .. }
135+
| StatementKind::Deinit(..)
136+
| StatementKind::StorageDead(..)
137+
| StatementKind::Retag(..)
138+
| StatementKind::AscribeUserType(..)
139+
| StatementKind::Intrinsic(..)
140+
| StatementKind::BackwardIncompatibleDropHint { .. } => false,
141+
}
142+
}
143+
fn next<'s>(&'s mut self) -> Option<StatementPair<'s, 'tcx>> {
144+
// Skip irrelevant statements
145+
if self.curr_idx >= self.statements.len() {
146+
return None;
147+
}
148+
while Self::is_statement_irrelevant(&self.statements[self.curr_idx]) {
149+
self.curr_idx += 1;
150+
if self.curr_idx >= self.statements.len() {
151+
return None;
152+
}
153+
}
154+
let curr = self.curr_idx;
155+
self.curr_idx += 1;
156+
let mut next_idx = self.curr_idx;
157+
if next_idx >= self.statements.len() {
158+
return None;
159+
}
160+
while Self::is_statement_irrelevant(&self.statements[next_idx]) {
161+
next_idx += 1;
162+
if next_idx >= self.statements.len() {
163+
return None;
164+
}
165+
}
166+
Some(self.get_at(curr, next_idx))
167+
}
168+
}

compiler/rustc_mir_transform/src/validate.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -956,7 +956,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypeChecker<'a, 'tcx> {
956956
.tcx
957957
.normalize_erasing_regions(self.typing_env, dest.ty(self.tcx, args));
958958
if !self.mir_assign_valid_types(src.ty(self.body, self.tcx), dest_ty) {
959-
self.fail(location, "adt field has the wrong type");
959+
self.fail(location, &format!("adt field has the wrong type. src:{:?} dest_ty:{dest_ty:?} src:{src:?}",src.ty(self.body, self.tcx)));
960960
}
961961
}
962962
}

0 commit comments

Comments
 (0)