Skip to content

Commit 1b87855

Browse files
authored
Auto merge of #36306 - nagisa:mir-local-cleanup, r=eddyb
A way to remove otherwise unused locals from MIR There is a certain amount of desire for a pass which cleans up the provably unused variables (no assignments or reads). There has been an implementation of such pass by @scottcarr, and another (two!) implementations by me in my own dataflow efforts. PR like #35916 proves that this pass is useful even on its own, which is why I cherry-picked it out from my dataflow effort. @nikomatsakis previously expressed concerns over this pass not seeming to be very cheap to run and therefore unsuitable for regular cleanup duties. Turns out, regular cleanup of local declarations is not at all necessary, at least now, because majority of passes simply do not (or should not) care about them. That’s why it is viable to only run this pass once (perhaps a few more times in the future?) per function, right before translation. r? @eddyb or @nikomatsakis
2 parents ac919fc + 4752367 commit 1b87855

File tree

6 files changed

+137
-35
lines changed

6 files changed

+137
-35
lines changed

src/librustc/mir/visit.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -157,14 +157,14 @@ macro_rules! make_mir_visitor {
157157

158158
fn visit_projection(&mut self,
159159
lvalue: & $($mutability)* LvalueProjection<'tcx>,
160-
context: LvalueContext,
160+
context: LvalueContext<'tcx>,
161161
location: Location) {
162162
self.super_projection(lvalue, context, location);
163163
}
164164

165165
fn visit_projection_elem(&mut self,
166166
lvalue: & $($mutability)* LvalueElem<'tcx>,
167-
context: LvalueContext,
167+
context: LvalueContext<'tcx>,
168168
location: Location) {
169169
self.super_projection_elem(lvalue, context, location);
170170
}
@@ -579,7 +579,7 @@ macro_rules! make_mir_visitor {
579579

580580
fn super_projection(&mut self,
581581
proj: & $($mutability)* LvalueProjection<'tcx>,
582-
context: LvalueContext,
582+
context: LvalueContext<'tcx>,
583583
location: Location) {
584584
let Projection {
585585
ref $($mutability)* base,
@@ -596,7 +596,7 @@ macro_rules! make_mir_visitor {
596596

597597
fn super_projection_elem(&mut self,
598598
proj: & $($mutability)* LvalueElem<'tcx>,
599-
_context: LvalueContext,
599+
_context: LvalueContext<'tcx>,
600600
location: Location) {
601601
match *proj {
602602
ProjectionElem::Deref => {
@@ -739,7 +739,7 @@ macro_rules! make_mir_visitor {
739739
make_mir_visitor!(Visitor,);
740740
make_mir_visitor!(MutVisitor,mut);
741741

742-
#[derive(Copy, Clone, Debug)]
742+
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
743743
pub enum LvalueContext<'tcx> {
744744
// Appears as LHS of an assignment
745745
Store,

src/librustc_data_structures/indexed_vec.rs

+15
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,21 @@ impl<I: Idx, T> IndexVec<I, T> {
149149
pub fn last(&self) -> Option<I> {
150150
self.len().checked_sub(1).map(I::new)
151151
}
152+
153+
#[inline]
154+
pub fn shrink_to_fit(&mut self) {
155+
self.raw.shrink_to_fit()
156+
}
157+
158+
#[inline]
159+
pub fn swap(&mut self, a: usize, b: usize) {
160+
self.raw.swap(a, b)
161+
}
162+
163+
#[inline]
164+
pub fn truncate(&mut self, a: usize) {
165+
self.raw.truncate(a)
166+
}
152167
}
153168

154169
impl<I: Idx, T> Index<I> for IndexVec<I, T> {

src/librustc_driver/driver.rs

+12-9
Original file line numberDiff line numberDiff line change
@@ -917,17 +917,19 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
917917
"MIR dump",
918918
|| mir::mir_map::build_mir_for_crate(tcx));
919919

920-
time(time_passes, "MIR passes", || {
920+
time(time_passes, "MIR cleanup and validation", || {
921921
let mut passes = sess.mir_passes.borrow_mut();
922-
// Push all the built-in passes.
922+
// Push all the built-in validation passes.
923+
// NB: if you’re adding an *optimisation* it ought to go to another set of passes
924+
// in stage 4 below.
923925
passes.push_hook(box mir::transform::dump_mir::DumpMir);
924-
passes.push_pass(box mir::transform::simplify_cfg::SimplifyCfg::new("initial"));
926+
passes.push_pass(box mir::transform::simplify::SimplifyCfg::new("initial"));
925927
passes.push_pass(
926928
box mir::transform::qualify_consts::QualifyAndPromoteConstants::default());
927929
passes.push_pass(box mir::transform::type_check::TypeckMir);
928930
passes.push_pass(
929931
box mir::transform::simplify_branches::SimplifyBranches::new("initial"));
930-
passes.push_pass(box mir::transform::simplify_cfg::SimplifyCfg::new("qualify-consts"));
932+
passes.push_pass(box mir::transform::simplify::SimplifyCfg::new("qualify-consts"));
931933
// And run everything.
932934
passes.run_passes(tcx);
933935
});
@@ -989,27 +991,28 @@ pub fn phase_4_translate_to_llvm<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
989991
"resolving dependency formats",
990992
|| dependency_format::calculate(&tcx.sess));
991993

992-
// Run the passes that transform the MIR into a more suitable for translation
993-
// to LLVM code.
994-
time(time_passes, "Prepare MIR codegen passes", || {
994+
// Run the passes that transform the MIR into a more suitable form for translation to LLVM
995+
// code.
996+
time(time_passes, "MIR optimisations", || {
995997
let mut passes = ::rustc::mir::transform::Passes::new();
996998
passes.push_hook(box mir::transform::dump_mir::DumpMir);
997999
passes.push_pass(box mir::transform::no_landing_pads::NoLandingPads);
998-
passes.push_pass(box mir::transform::simplify_cfg::SimplifyCfg::new("no-landing-pads"));
1000+
passes.push_pass(box mir::transform::simplify::SimplifyCfg::new("no-landing-pads"));
9991001

10001002
// From here on out, regions are gone.
10011003
passes.push_pass(box mir::transform::erase_regions::EraseRegions);
10021004

10031005
passes.push_pass(box mir::transform::add_call_guards::AddCallGuards);
10041006
passes.push_pass(box borrowck::ElaborateDrops);
10051007
passes.push_pass(box mir::transform::no_landing_pads::NoLandingPads);
1006-
passes.push_pass(box mir::transform::simplify_cfg::SimplifyCfg::new("elaborate-drops"));
1008+
passes.push_pass(box mir::transform::simplify::SimplifyCfg::new("elaborate-drops"));
10071009

10081010
// No lifetime analysis based on borrowing can be done from here on out.
10091011
passes.push_pass(box mir::transform::instcombine::InstCombine::new());
10101012
passes.push_pass(box mir::transform::deaggregator::Deaggregator);
10111013
passes.push_pass(box mir::transform::copy_prop::CopyPropagation);
10121014

1015+
passes.push_pass(box mir::transform::simplify::SimplifyLocals);
10131016
passes.push_pass(box mir::transform::add_call_guards::AddCallGuards);
10141017
passes.push_pass(box mir::transform::dump_mir::Marker("PreTrans"));
10151018

src/librustc_mir/transform/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
// except according to those terms.
1010

1111
pub mod simplify_branches;
12-
pub mod simplify_cfg;
12+
pub mod simplify;
1313
pub mod erase_regions;
1414
pub mod no_landing_pads;
1515
pub mod type_check;

src/librustc_mir/transform/simplify_cfg.rs src/librustc_mir/transform/simplify.rs

+104-14
Original file line numberDiff line numberDiff line change
@@ -8,35 +8,41 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
//! A pass that removes various redundancies in the CFG. It should be
12-
//! called after every significant CFG modification to tidy things
13-
//! up.
11+
//! A number of passes which remove various redundancies in the CFG.
1412
//!
15-
//! This pass must also be run before any analysis passes because it removes
16-
//! dead blocks, and some of these can be ill-typed.
13+
//! The `SimplifyCfg` pass gets rid of unnecessary blocks in the CFG, whereas the `SimplifyLocals`
14+
//! gets rid of all the unnecessary local variable declarations.
1715
//!
18-
//! The cause of that is that typeck lets most blocks whose end is not
19-
//! reachable have an arbitrary return type, rather than having the
20-
//! usual () return type (as a note, typeck's notion of reachability
21-
//! is in fact slightly weaker than MIR CFG reachability - see #31617).
16+
//! The `SimplifyLocals` pass is kinda expensive and therefore not very suitable to be run often.
17+
//! Most of the passes should not care or be impacted in meaningful ways due to extra locals
18+
//! either, so running the pass once, right before translation, should suffice.
19+
//!
20+
//! On the other side of the spectrum, the `SimplifyCfg` pass is considerably cheap to run, thus
21+
//! one should run it after every pass which may modify CFG in significant ways. This pass must
22+
//! also be run before any analysis passes because it removes dead blocks, and some of these can be
23+
//! ill-typed.
24+
//!
25+
//! The cause of this typing issue is typeck allowing most blocks whose end is not reachable have
26+
//! an arbitrary return type, rather than having the usual () return type (as a note, typeck's
27+
//! notion of reachability is in fact slightly weaker than MIR CFG reachability - see #31617). A
28+
//! standard example of the situation is:
2229
//!
23-
//! A standard example of the situation is:
2430
//! ```rust
2531
//! fn example() {
2632
//! let _a: char = { return; };
2733
//! }
2834
//! ```
2935
//!
30-
//! Here the block (`{ return; }`) has the return type `char`,
31-
//! rather than `()`, but the MIR we naively generate still contains
32-
//! the `_a = ()` write in the unreachable block "after" the return.
33-
36+
//! Here the block (`{ return; }`) has the return type `char`, rather than `()`, but the MIR we
37+
//! naively generate still contains the `_a = ()` write in the unreachable block "after" the
38+
//! return.
3439
3540
use rustc_data_structures::bitvec::BitVector;
3641
use rustc_data_structures::indexed_vec::{Idx, IndexVec};
3742
use rustc::ty::TyCtxt;
3843
use rustc::mir::*;
3944
use rustc::mir::transform::{MirPass, MirSource, Pass};
45+
use rustc::mir::visit::{MutVisitor, Visitor, LvalueContext};
4046
use std::fmt;
4147

4248
pub struct SimplifyCfg<'a> { label: &'a str }
@@ -257,3 +263,87 @@ fn remove_dead_blocks(mir: &mut Mir) {
257263
}
258264
}
259265
}
266+
267+
268+
pub struct SimplifyLocals;
269+
270+
impl Pass for SimplifyLocals {
271+
fn name(&self) -> ::std::borrow::Cow<'static, str> { "SimplifyLocals".into() }
272+
}
273+
274+
impl<'tcx> MirPass<'tcx> for SimplifyLocals {
275+
fn run_pass<'a>(&mut self, _: TyCtxt<'a, 'tcx, 'tcx>, _: MirSource, mir: &mut Mir<'tcx>) {
276+
let mut marker = DeclMarker { locals: BitVector::new(mir.local_decls.len()) };
277+
marker.visit_mir(mir);
278+
// Return pointer and arguments are always live
279+
marker.locals.insert(0);
280+
for idx in mir.args_iter() {
281+
marker.locals.insert(idx.index());
282+
}
283+
let map = make_local_map(&mut mir.local_decls, marker.locals);
284+
// Update references to all vars and tmps now
285+
LocalUpdater { map: map }.visit_mir(mir);
286+
mir.local_decls.shrink_to_fit();
287+
}
288+
}
289+
290+
/// Construct the mapping while swapping out unused stuff out from the `vec`.
291+
fn make_local_map<'tcx, I: Idx, V>(vec: &mut IndexVec<I, V>, mask: BitVector) -> Vec<usize> {
292+
let mut map: Vec<usize> = ::std::iter::repeat(!0).take(vec.len()).collect();
293+
let mut used = 0;
294+
for alive_index in mask.iter() {
295+
map[alive_index] = used;
296+
if alive_index != used {
297+
vec.swap(alive_index, used);
298+
}
299+
used += 1;
300+
}
301+
vec.truncate(used);
302+
map
303+
}
304+
305+
struct DeclMarker {
306+
pub locals: BitVector,
307+
}
308+
309+
impl<'tcx> Visitor<'tcx> for DeclMarker {
310+
fn visit_lvalue(&mut self, lval: &Lvalue<'tcx>, ctx: LvalueContext<'tcx>, loc: Location) {
311+
if ctx == LvalueContext::StorageLive || ctx == LvalueContext::StorageDead {
312+
// ignore these altogether, they get removed along with their otherwise unused decls.
313+
return;
314+
}
315+
if let Lvalue::Local(ref v) = *lval {
316+
self.locals.insert(v.index());
317+
}
318+
self.super_lvalue(lval, ctx, loc);
319+
}
320+
}
321+
322+
struct LocalUpdater {
323+
map: Vec<usize>,
324+
}
325+
326+
impl<'tcx> MutVisitor<'tcx> for LocalUpdater {
327+
fn visit_basic_block_data(&mut self, block: BasicBlock, data: &mut BasicBlockData<'tcx>) {
328+
// Remove unnecessary StorageLive and StorageDead annotations.
329+
data.statements.retain(|stmt| {
330+
match stmt.kind {
331+
StatementKind::StorageLive(ref lval) | StatementKind::StorageDead(ref lval) => {
332+
match *lval {
333+
Lvalue::Local(l) => self.map[l.index()] != !0,
334+
_ => true
335+
}
336+
}
337+
_ => true
338+
}
339+
});
340+
self.super_basic_block_data(block, data);
341+
}
342+
fn visit_lvalue(&mut self, lval: &mut Lvalue<'tcx>, ctx: LvalueContext<'tcx>, loc: Location) {
343+
match *lval {
344+
Lvalue::Local(ref mut l) => *l = Local::new(self.map[l.index()]),
345+
_ => (),
346+
};
347+
self.super_lvalue(lval, ctx, loc);
348+
}
349+
}

src/librustc_trans/mir/analyze.rs

-6
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,6 @@ pub fn lvalue_locals<'bcx, 'tcx>(bcx: Block<'bcx,'tcx>,
4242
common::type_is_fat_ptr(bcx.tcx(), ty));
4343
} else if common::type_is_imm_pair(bcx.ccx(), ty) {
4444
// We allow pairs and uses of any of their 2 fields.
45-
} else if !analyzer.seen_assigned.contains(index) {
46-
// No assignment has been seen, which means that
47-
// either the local has been marked as lvalue
48-
// already, or there is no possible initialization
49-
// for the local, making any reads invalid.
50-
// This is useful in weeding out dead temps.
5145
} else {
5246
// These sorts of types require an alloca. Note that
5347
// type_is_immediate() may *still* be true, particularly

0 commit comments

Comments
 (0)