Skip to content

Commit e00862b

Browse files
committed
A way to remove otherwise unused locals from MIR
Replaces the previous hack
1 parent 3e2e8b9 commit e00862b

File tree

11 files changed

+141
-67
lines changed

11 files changed

+141
-67
lines changed

src/librustc_data_structures/indexed_vec.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,21 @@ impl<I: Idx, T> IndexVec<I, T> {
164164
pub unsafe fn as_mut_ptr(&mut self) -> *mut T {
165165
self.raw.as_mut_ptr()
166166
}
167+
168+
#[inline]
169+
pub fn shrink_to_fit(&mut self) {
170+
self.raw.shrink_to_fit()
171+
}
172+
173+
#[inline]
174+
pub fn swap(&mut self, a: usize, b: usize) {
175+
self.raw.swap(a, b)
176+
}
177+
178+
#[inline]
179+
pub fn truncate(&mut self, a: usize) {
180+
self.raw.truncate(a)
181+
}
167182
}
168183

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

src/librustc_driver/driver.rs

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -910,16 +910,16 @@ pub fn phase_3_run_analysis_passes<'tcx, F, R>(sess: &'tcx Session,
910910
"MIR dump",
911911
|| mir::mir_map::build_mir_for_crate(tcx));
912912

913-
time(time_passes, "MIR passes", || {
913+
time(time_passes, "MIR validation", || {
914914
let mut passes = sess.mir_passes.borrow_mut();
915915
// Push all the built-in passes.
916916
passes.push_hook(box mir::transform::dump_mir::DumpMir);
917-
passes.push_pass(box mir::transform::simplify_cfg::SimplifyCfg::new("initial"));
917+
passes.push_pass(box mir::transform::simplify::SimplifyCfg::new("initial"));
918918
passes.push_pass(box mir::transform::qualify_consts::QualifyAndPromoteConstants);
919919
passes.push_pass(box mir::transform::type_check::TypeckMir);
920920
passes.push_pass(
921921
box mir::transform::simplify_branches::SimplifyBranches::new("initial"));
922-
passes.push_pass(box mir::transform::simplify_cfg::SimplifyCfg::new("qualify-consts"));
922+
passes.push_pass(box mir::transform::simplify::SimplifyCfg::new("qualify-consts"));
923923
// And run everything.
924924
passes.run_passes(tcx, &mut mir_map);
925925
});
@@ -983,25 +983,26 @@ pub fn phase_4_translate_to_llvm<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
983983

984984
// Run the passes that transform the MIR into a more suitable for translation
985985
// to LLVM code.
986-
time(time_passes, "Prepare MIR codegen passes", || {
986+
time(time_passes, "MIR optimisations", || {
987987
let mut passes = ::rustc::mir::transform::Passes::new();
988988
passes.push_hook(box mir::transform::dump_mir::DumpMir);
989989
passes.push_pass(box mir::transform::no_landing_pads::NoLandingPads);
990-
passes.push_pass(box mir::transform::simplify_cfg::SimplifyCfg::new("no-landing-pads"));
990+
passes.push_pass(box mir::transform::simplify::SimplifyCfg::new("no-landing-pads"));
991991

992992
passes.push_pass(box mir::transform::erase_regions::EraseRegions);
993993

994994
passes.push_pass(box mir::transform::deaggregator::Deaggregator);
995995
passes.push_pass(box mir::transform::const_propagate::ConstPropagate);
996-
passes.push_pass(box mir::transform::simplify_cfg::SimplifyCfg::new("ccs-propagate"));
996+
passes.push_pass(box mir::transform::simplify::SimplifyCfg::new("const-propagate"));
997997
passes.push_pass(box mir::transform::deadcode::DeadCode);
998998

999999
passes.push_pass(box mir::transform::add_call_guards::AddCallGuards);
10001000
passes.push_pass(box borrowck::ElaborateDrops);
10011001
passes.push_pass(box mir::transform::no_landing_pads::NoLandingPads);
1002-
passes.push_pass(box mir::transform::simplify_cfg::SimplifyCfg::new("elaborate-drops"));
1002+
passes.push_pass(box mir::transform::simplify::SimplifyCfg::new("elaborate-drops"));
10031003

10041004

1005+
passes.push_pass(box mir::transform::simplify::SimplifyLocals);
10051006
passes.push_pass(box mir::transform::add_call_guards::AddCallGuards);
10061007
passes.push_pass(box mir::transform::dump_mir::Marker("PreTrans"));
10071008

src/librustc_mir/transform/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
pub mod dataflow;
1212

1313
pub mod simplify_branches;
14-
pub mod simplify_cfg;
14+
pub mod simplify;
1515
pub mod erase_regions;
1616
pub mod no_landing_pads;
1717
pub mod type_check;

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

Lines changed: 114 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,23 @@
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 variables.
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 that is that typeck lets most blocks whose end is not reachable have an arbitrary
26+
//! return type, rather than having the usual () return type (as a note, typeck's notion of
27+
//! reachability is in fact slightly weaker than MIR CFG reachability - see #31617).
2228
//!
2329
//! A standard example of the situation is:
2430
//! ```rust
@@ -27,16 +33,16 @@
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::repr::*;
3944
use rustc::mir::transform::{MirPass, MirSource, Pass};
45+
use rustc::mir::visit::{MutVisitor, Visitor, LvalueContext};
4046
use rustc::mir::traversal;
4147
use std::fmt;
4248

@@ -247,3 +253,98 @@ fn remove_dead_blocks(mir: &mut Mir) {
247253
}
248254
}
249255
}
256+
257+
258+
pub struct SimplifyLocals;
259+
260+
impl Pass for SimplifyLocals {
261+
fn name(&self) -> ::std::borrow::Cow<'static, str> { "SimplifyLocals".into() }
262+
}
263+
264+
impl<'tcx> MirPass<'tcx> for SimplifyLocals {
265+
fn run_pass<'a>(&mut self, _: TyCtxt<'a, 'tcx, 'tcx>, _: MirSource, mir: &mut Mir<'tcx>) {
266+
let (live_vars, live_temps) = count_live_locals(mir);
267+
let var_map = make_local_map(&mut mir.var_decls, live_vars);
268+
let tmp_map = make_local_map(&mut mir.temp_decls, live_temps);
269+
// Update references to all vars and tmps now
270+
LocalUpdater { var_map: var_map, tmp_map: tmp_map }.visit_mir(mir);
271+
mir.var_decls.shrink_to_fit();
272+
mir.temp_decls.shrink_to_fit();
273+
}
274+
}
275+
276+
/// Construct the mapping while swapping out unused stuff out from the `vec`.
277+
fn make_local_map<'tcx, I: Idx, V>(vec: &mut IndexVec<I, V>, mask: BitVector) -> Vec<usize> {
278+
let mut map: Vec<usize> = ::std::iter::repeat(!0).take(vec.len()).collect();
279+
let mut used = 0;
280+
for alive_index in mask.iter() {
281+
map[alive_index] = used;
282+
if alive_index != used {
283+
vec.swap(alive_index, used);
284+
}
285+
used += 1;
286+
}
287+
vec.truncate(used);
288+
map
289+
}
290+
291+
fn count_live_locals<'tcx>(mir: &Mir<'tcx>) -> (BitVector, BitVector) {
292+
let mut marker = DeclMarker {
293+
live_vars: BitVector::new(mir.var_decls.len()),
294+
live_temps: BitVector::new(mir.temp_decls.len()),
295+
};
296+
marker.visit_mir(mir);
297+
let DeclMarker { live_vars, live_temps } = marker;
298+
(live_vars, live_temps)
299+
}
300+
301+
struct DeclMarker {
302+
pub live_vars: BitVector,
303+
pub live_temps: BitVector
304+
}
305+
306+
impl<'tcx> Visitor<'tcx> for DeclMarker {
307+
fn visit_lvalue(&mut self, lval: &Lvalue<'tcx>, ctx: LvalueContext) {
308+
if ctx == LvalueContext::StorageLive || ctx == LvalueContext::StorageDead {
309+
return; // ignore these altogether, they get removed along with their decls.
310+
}
311+
match *lval {
312+
Lvalue::Var(ref v) => self.live_vars.insert(v.index()),
313+
Lvalue::Temp(ref t) => self.live_temps.insert(t.index()),
314+
_ => false,
315+
};
316+
self.super_lvalue(lval, ctx);
317+
}
318+
}
319+
320+
struct LocalUpdater {
321+
var_map: Vec<usize>,
322+
tmp_map: Vec<usize>
323+
}
324+
325+
impl<'tcx> MutVisitor<'tcx> for LocalUpdater {
326+
fn visit_basic_block_data(&mut self, block: BasicBlock, data: &mut BasicBlockData<'tcx>) {
327+
// Remove unnecessary StorageLive and StorageDead annotations.
328+
data.statements.retain(|stmt| {
329+
match stmt.kind {
330+
StatementKind::StorageLive(ref lval) | StatementKind::StorageDead(ref lval) => {
331+
match *lval {
332+
Lvalue::Var(v) => self.var_map[v.index()] != !0,
333+
Lvalue::Temp(t) => self.tmp_map[t.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) {
343+
match *lval {
344+
Lvalue::Var(ref mut v) => *v = Var::new(self.var_map[v.index()]),
345+
Lvalue::Temp(ref mut t) => *t = Temp::new(self.tmp_map[t.index()]),
346+
_ => (),
347+
};
348+
self.super_lvalue(lval, ctx);
349+
}
350+
}

src/librustc_trans/mir/analyze.rs

Lines changed: 0 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -299,32 +299,3 @@ pub fn cleanup_kinds<'bcx,'tcx>(_bcx: Block<'bcx,'tcx>,
299299
debug!("cleanup_kinds: result={:?}", result);
300300
result
301301
}
302-
303-
pub fn count_live_locals<'tcx>(mir: &mir::Mir<'tcx>) -> (BitVector, BitVector) {
304-
let mut marker = DeclMarker {
305-
live_vars: BitVector::new(mir.var_decls.len()),
306-
live_temps: BitVector::new(mir.temp_decls.len()),
307-
};
308-
marker.visit_mir(mir);
309-
let DeclMarker { live_vars, live_temps } = marker;
310-
(live_vars, live_temps)
311-
}
312-
313-
struct DeclMarker {
314-
pub live_vars: BitVector,
315-
pub live_temps: BitVector
316-
}
317-
318-
impl<'tcx> Visitor<'tcx> for DeclMarker {
319-
fn visit_lvalue(&mut self, lval: &mir::Lvalue<'tcx>, ctx: LvalueContext) {
320-
if ctx == LvalueContext::StorageLive || ctx == LvalueContext::StorageDead {
321-
return; // ignore these altogether.
322-
}
323-
match *lval {
324-
mir::Lvalue::Var(ref v) => self.live_vars.insert(v.index()),
325-
mir::Lvalue::Temp(ref t) => self.live_temps.insert(t.index()),
326-
_ => false,
327-
};
328-
self.super_lvalue(lval, ctx);
329-
}
330-
}

src/librustc_trans/mir/block.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,6 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
202202
ty: tr_lvalue.ty.to_ty(bcx.tcx())
203203
}
204204
}
205-
LocalRef::Unused => bug!("reference to unused local"),
206205
};
207206
let llslot = match op.val {
208207
Immediate(_) | Pair(..) => {
@@ -857,7 +856,6 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
857856
LocalRef::Operand(Some(_)) => {
858857
bug!("lvalue local already assigned to");
859858
}
860-
LocalRef::Unused => bug!("reference to unused statement"),
861859
}
862860
} else {
863861
self.trans_lvalue(bcx, dest)

src/librustc_trans/mir/lvalue.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,6 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
9999
LocalRef::Operand(..) => {
100100
bug!("using operand local {:?} as lvalue", lvalue);
101101
}
102-
LocalRef::Unused => bug!("reference to unused local"),
103102
}
104103
}
105104

@@ -262,7 +261,6 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
262261
bug!("Lvalue local already set");
263262
}
264263
}
265-
LocalRef::Unused => bug!("reference to unused local"),
266264
}
267265
} else {
268266
let lvalue = self.trans_lvalue(bcx, lvalue);

src/librustc_trans/mir/mod.rs

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ impl<'blk, 'tcx> MirContext<'blk, 'tcx> {
115115
enum LocalRef<'tcx> {
116116
Lvalue(LvalueRef<'tcx>),
117117
Operand(Option<OperandRef<'tcx>>),
118-
Unused,
119118
}
120119

121120
impl<'tcx> LocalRef<'tcx> {
@@ -155,7 +154,6 @@ pub fn trans_mir<'blk, 'tcx: 'blk>(fcx: &'blk FunctionContext<'blk, 'tcx>) {
155154
(analyze::lvalue_locals(bcx, &mir),
156155
analyze::cleanup_kinds(bcx, &mir))
157156
});
158-
let (live_vars, live_temps) = analyze::count_live_locals(&mir);
159157

160158

161159
// Compute debuginfo scopes from MIR scopes.
@@ -165,9 +163,6 @@ pub fn trans_mir<'blk, 'tcx: 'blk>(fcx: &'blk FunctionContext<'blk, 'tcx>) {
165163
let locals = {
166164
let args = arg_local_refs(&bcx, &mir, &scopes, &lvalue_locals);
167165
let vars = mir.var_decls.iter_enumerated().map(|(var_idx, decl)| {
168-
if !live_vars.contains(var_idx.index()) {
169-
return LocalRef::Unused;
170-
}
171166
let ty = bcx.monomorphize(&decl.ty);
172167
let scope = scopes[decl.source_info.scope];
173168
let dbg = !scope.is_null() && bcx.sess().opts.debuginfo == FullDebugInfo;
@@ -186,9 +181,6 @@ pub fn trans_mir<'blk, 'tcx: 'blk>(fcx: &'blk FunctionContext<'blk, 'tcx>) {
186181
LocalRef::Lvalue(lvalue)
187182
});
188183
let temps = mir.temp_decls.iter_enumerated().map(|(temp_idx, decl)| {
189-
if !live_temps.contains(temp_idx.index()) {
190-
return LocalRef::Unused;
191-
}
192184
let ty = bcx.monomorphize(&decl.ty);
193185
let local = mir.local_index(&mir::Lvalue::Temp(temp_idx)).unwrap();
194186
if lvalue_locals.contains(local.index()) {

src/librustc_trans/mir/operand.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,6 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
186186
LocalRef::Lvalue(..) => {
187187
// use path below
188188
}
189-
LocalRef::Unused => bug!("reference to unused local"),
190189
}
191190
}
192191

src/librustc_trans/mir/statement.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ impl<'bcx, 'tcx> MirContext<'bcx, 'tcx> {
5454
self.trans_rvalue_operand(bcx, rvalue, debug_loc).0
5555
}
5656
}
57-
LocalRef::Unused => bug!("reference to unused local"),
5857
}
5958
} else {
6059
let tr_dest = self.trans_lvalue(&bcx, lvalue);

src/test/codegen/lifetime_start_end.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,14 @@
1818
#[rustc_mir] // FIXME #27840 MIR has different codegen.
1919
pub fn test() {
2020
let a = 0;
21-
&a; // keep variable in an alloca
21+
drop(&a); // keep variable in an alloca
2222

2323
// CHECK: [[S_a:%[0-9]+]] = bitcast i32* %a to i8*
2424
// CHECK: call void @llvm.lifetime.start(i{{[0-9 ]+}}, i8* [[S_a]])
2525

2626
{
2727
let b = &Some(a);
28-
&b; // keep variable in an alloca
28+
drop(&b); // keep variable in an alloca
2929

3030
// CHECK: [[S_b:%[0-9]+]] = bitcast %"2.std::option::Option<i32>"** %b to i8*
3131
// CHECK: call void @llvm.lifetime.start(i{{[0-9 ]+}}, i8* [[S_b]])
@@ -41,7 +41,7 @@ pub fn test() {
4141
}
4242

4343
let c = 1;
44-
&c; // keep variable in an alloca
44+
drop(&c); // keep variable in an alloca
4545

4646
// CHECK: [[S_c:%[0-9]+]] = bitcast i32* %c to i8*
4747
// CHECK: call void @llvm.lifetime.start(i{{[0-9 ]+}}, i8* [[S_c]])

0 commit comments

Comments
 (0)