Skip to content

Commit ac1cebb

Browse files
committed
Move ZST check inside UsedLocals
1 parent ee62461 commit ac1cebb

File tree

2 files changed

+54
-50
lines changed

2 files changed

+54
-50
lines changed

compiler/rustc_mir/src/transform/simplify.rs

Lines changed: 53 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,10 @@ use crate::transform::MirPass;
3131
use rustc_index::vec::{Idx, IndexVec};
3232
use rustc_middle::mir::visit::{MutVisitor, MutatingUseContext, PlaceContext, Visitor};
3333
use rustc_middle::mir::*;
34+
use rustc_middle::ty::ParamEnv;
3435
use rustc_middle::ty::TyCtxt;
3536
use smallvec::SmallVec;
36-
use std::borrow::Cow;
37-
use std::convert::TryInto;
37+
use std::{borrow::Cow, convert::TryInto};
3838

3939
pub struct SimplifyCfg {
4040
label: String,
@@ -322,17 +322,18 @@ impl<'tcx> MirPass<'tcx> for SimplifyLocals {
322322
trace!("running SimplifyLocals on {:?}", body.source);
323323

324324
// First, we're going to get a count of *actual* uses for every `Local`.
325-
let mut used_locals = UsedLocals::new(body);
325+
let mut used_locals = UsedLocals::new(body, tcx);
326326

327327
// Next, we're going to remove any `Local` with zero actual uses. When we remove those
328328
// `Locals`, we're also going to subtract any uses of other `Locals` from the `used_locals`
329329
// count. For example, if we removed `_2 = discriminant(_1)`, then we'll subtract one from
330330
// `use_counts[_1]`. That in turn might make `_1` unused, so we loop until we hit a
331331
// fixedpoint where there are no more unused locals.
332-
remove_unused_definitions(&mut used_locals, body, tcx);
332+
remove_unused_definitions(&mut used_locals, body);
333333

334334
// Finally, we'll actually do the work of shrinking `body.local_decls` and remapping the `Local`s.
335-
let map = make_local_map(&mut body.local_decls, &used_locals);
335+
let arg_count = body.arg_count.try_into().unwrap();
336+
let map = make_local_map(&mut body.local_decls, &used_locals, arg_count);
336337

337338
// Only bother running the `LocalUpdater` if we actually found locals to remove.
338339
if map.iter().any(Option::is_none) {
@@ -346,54 +347,61 @@ impl<'tcx> MirPass<'tcx> for SimplifyLocals {
346347
}
347348

348349
/// Construct the mapping while swapping out unused stuff out from the `vec`.
349-
fn make_local_map<V>(
350+
fn make_local_map<'tcx, V>(
350351
local_decls: &mut IndexVec<Local, V>,
351-
used_locals: &UsedLocals,
352+
used_locals: &UsedLocals<'tcx>,
353+
arg_count: u32,
352354
) -> IndexVec<Local, Option<Local>> {
353-
let mut map: IndexVec<Local, Option<Local>> = IndexVec::from_elem(None, &*local_decls);
355+
let mut map: IndexVec<Local, Option<Local>> = IndexVec::from_elem(None, local_decls);
354356
let mut used = Local::new(0);
355357

356358
for alive_index in local_decls.indices() {
357-
// `is_used` treats the `RETURN_PLACE` and arguments as used.
358-
if !used_locals.is_used(alive_index) {
359-
continue;
360-
}
361-
362-
map[alive_index] = Some(used);
363-
if alive_index != used {
364-
local_decls.swap(alive_index, used);
359+
// When creating the local map treat the `RETURN_PLACE` and arguments as used.
360+
if alive_index.as_u32() <= arg_count || used_locals.is_used(alive_index) {
361+
map[alive_index] = Some(used);
362+
if alive_index != used {
363+
local_decls.swap(alive_index, used);
364+
}
365+
used.increment_by(1);
365366
}
366-
used.increment_by(1);
367367
}
368368
local_decls.truncate(used.index());
369369
map
370370
}
371371

372372
/// Keeps track of used & unused locals.
373-
struct UsedLocals {
373+
struct UsedLocals<'tcx> {
374374
increment: bool,
375-
arg_count: u32,
376375
use_count: IndexVec<Local, u32>,
376+
is_static: bool,
377+
local_decls: IndexVec<Local, LocalDecl<'tcx>>,
378+
param_env: ParamEnv<'tcx>,
379+
tcx: TyCtxt<'tcx>,
377380
}
378381

379-
impl UsedLocals {
382+
impl UsedLocals<'tcx> {
380383
/// Determines which locals are used & unused in the given body.
381-
fn new(body: &Body<'_>) -> Self {
384+
fn new(body: &Body<'tcx>, tcx: TyCtxt<'tcx>) -> Self {
385+
let def_id = body.source.def_id();
386+
let is_static = tcx.is_static(def_id);
387+
let param_env = tcx.param_env(def_id);
388+
let local_decls = body.local_decls.clone();
382389
let mut this = Self {
383390
increment: true,
384-
arg_count: body.arg_count.try_into().unwrap(),
385391
use_count: IndexVec::from_elem(0, &body.local_decls),
392+
is_static,
393+
local_decls,
394+
param_env,
395+
tcx,
386396
};
387397
this.visit_body(body);
388398
this
389399
}
390400

391401
/// Checks if local is used.
392-
///
393-
/// Return place and arguments are always considered used.
394402
fn is_used(&self, local: Local) -> bool {
395403
trace!("is_used({:?}): use_count: {:?}", local, self.use_count[local]);
396-
local.as_u32() <= self.arg_count || self.use_count[local] != 0
404+
self.use_count[local] != 0
397405
}
398406

399407
/// Updates the use counts to reflect the removal of given statement.
@@ -422,7 +430,7 @@ impl UsedLocals {
422430
}
423431
}
424432

425-
impl Visitor<'_> for UsedLocals {
433+
impl Visitor<'tcx> for UsedLocals<'tcx> {
426434
fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) {
427435
match statement.kind {
428436
StatementKind::LlvmInlineAsm(..)
@@ -448,7 +456,23 @@ impl Visitor<'_> for UsedLocals {
448456
}
449457
}
450458

451-
fn visit_local(&mut self, local: &Local, _ctx: PlaceContext, _location: Location) {
459+
fn visit_local(&mut self, local: &Local, ctx: PlaceContext, _location: Location) {
460+
debug!("local: {:?} is_static: {:?}, ctx: {:?}", local, self.is_static, ctx);
461+
// Do not count a local as used in `_local = <rhs>` if RHS is a ZST.
462+
let store = matches!(ctx, PlaceContext::MutatingUse(MutatingUseContext::Store));
463+
// Do not count _0 as a used in `return;` if it is a ZST.
464+
let return_place = *local == RETURN_PLACE
465+
&& matches!(ctx, PlaceContext::NonMutatingUse(visit::NonMutatingUseContext::Move));
466+
if !self.is_static && (store || return_place) {
467+
let ty = self.local_decls[*local].ty;
468+
let param_env_and = self.param_env.and(ty);
469+
if let Ok(layout) = self.tcx.layout_of(param_env_and) {
470+
debug!("layout.is_zst: {:?}", layout.is_zst());
471+
if layout.is_zst() {
472+
return;
473+
}
474+
}
475+
}
452476
if self.increment {
453477
self.use_count[*local] += 1;
454478
} else {
@@ -460,21 +484,14 @@ impl Visitor<'_> for UsedLocals {
460484

461485
/// Removes unused definitions. Updates the used locals to reflect the changes made.
462486
fn remove_unused_definitions<'a, 'tcx>(
463-
used_locals: &'a mut UsedLocals,
487+
used_locals: &'a mut UsedLocals<'tcx>,
464488
body: &mut Body<'tcx>,
465-
tcx: TyCtxt<'tcx>,
466489
) {
467490
// The use counts are updated as we remove the statements. A local might become unused
468491
// during the retain operation, leading to a temporary inconsistency (storage statements or
469492
// definitions referencing the local might remain). For correctness it is crucial that this
470493
// computation reaches a fixed point.
471494

472-
let def_id = body.source.def_id();
473-
let is_static = tcx.is_static(def_id);
474-
let param_env = tcx.param_env(def_id);
475-
476-
let local_decls = body.local_decls.clone();
477-
478495
let mut modified = true;
479496
while modified {
480497
modified = false;
@@ -486,21 +503,7 @@ fn remove_unused_definitions<'a, 'tcx>(
486503
StatementKind::StorageLive(local) | StatementKind::StorageDead(local) => {
487504
used_locals.is_used(*local)
488505
}
489-
StatementKind::Assign(box (place, _)) => {
490-
let used = used_locals.is_used(place.local);
491-
let mut is_zst = false;
492-
493-
// ZST locals can be removed
494-
if used && !is_static {
495-
let ty = local_decls[place.local].ty;
496-
let param_env_and = param_env.and(ty);
497-
if let Ok(layout) = tcx.layout_of(param_env_and) {
498-
is_zst = layout.is_zst();
499-
}
500-
}
501-
502-
used && !is_zst
503-
}
506+
StatementKind::Assign(box (place, _)) => used_locals.is_used(place.local),
504507

505508
StatementKind::SetDiscriminant { ref place, .. } => {
506509
used_locals.is_used(place.local)

src/test/mir-opt/inline/issue_76997_inline_scopes_parenting.main.Inline.after.mir

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ fn main() -> () {
2828
StorageLive(_5); // scope 1 at $DIR/issue-76997-inline-scopes-parenting.rs:6:5: 6:10
2929
_5 = move (_3.0: ()); // scope 1 at $DIR/issue-76997-inline-scopes-parenting.rs:6:5: 6:10
3030
StorageLive(_6); // scope 2 at $DIR/issue-76997-inline-scopes-parenting.rs:6:5: 6:10
31+
_6 = const (); // scope 2 at $DIR/issue-76997-inline-scopes-parenting.rs:6:5: 6:10
3132
StorageDead(_6); // scope 2 at $DIR/issue-76997-inline-scopes-parenting.rs:6:5: 6:10
3233
StorageDead(_5); // scope 1 at $DIR/issue-76997-inline-scopes-parenting.rs:6:5: 6:10
3334
StorageDead(_4); // scope 1 at $DIR/issue-76997-inline-scopes-parenting.rs:6:9: 6:10

0 commit comments

Comments
 (0)