Skip to content

Commit

Permalink
Auto merge of #83177 - erikdesjardins:zstassign, r=oli-obk
Browse files Browse the repository at this point in the history
Remove assignments to ZST places instead of marking ZST return place as unused

partially reverts #83118

requested by `@tmiasko` in #83118 (comment)

r? `@oli-obk`
  • Loading branch information
bors committed Mar 23, 2021
2 parents 4eb0bc7 + 6960bc9 commit 79e5814
Show file tree
Hide file tree
Showing 86 changed files with 178 additions and 176 deletions.
2 changes: 2 additions & 0 deletions compiler/rustc_mir/src/transform/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ pub mod promote_consts;
pub mod remove_noop_landing_pads;
pub mod remove_storage_markers;
pub mod remove_unneeded_drops;
pub mod remove_zsts;
pub mod required_consts;
pub mod rustc_peek;
pub mod simplify;
Expand Down Expand Up @@ -494,6 +495,7 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
// The main optimizations that we do on MIR.
let optimizations: &[&dyn MirPass<'tcx>] = &[
&remove_storage_markers::RemoveStorageMarkers,
&remove_zsts::RemoveZsts,
&const_goto::ConstGoto,
&remove_unneeded_drops::RemoveUnneededDrops,
&match_branches::MatchBranchSimplification,
Expand Down
57 changes: 57 additions & 0 deletions compiler/rustc_mir/src/transform/remove_zsts.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
//! Removes assignments to ZST places.
use crate::transform::MirPass;
use rustc_middle::mir::{Body, StatementKind};
use rustc_middle::ty::{self, Ty, TyCtxt};

pub struct RemoveZsts;

impl<'tcx> MirPass<'tcx> for RemoveZsts {
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
if tcx.sess.mir_opt_level() < 3 {
return;
}
let param_env = tcx.param_env(body.source.def_id());
let (basic_blocks, local_decls) = body.basic_blocks_and_local_decls_mut();
for block in basic_blocks.iter_mut() {
for statement in block.statements.iter_mut() {
match statement.kind {
StatementKind::Assign(box (place, _)) => {
let place_ty = place.ty(local_decls, tcx).ty;
if !maybe_zst(place_ty) {
continue;
}
let layout = match tcx.layout_of(param_env.and(place_ty)) {
Ok(layout) => layout,
Err(_) => continue,
};
if !layout.is_zst() {
continue;
}
if tcx.consider_optimizing(|| {
format!(
"RemoveZsts - Place: {:?} SourceInfo: {:?}",
place, statement.source_info
)
}) {
statement.make_nop();
}
}
_ => {}
}
}
}
}
}

/// A cheap, approximate check to avoid unnecessary `layout_of` calls.
fn maybe_zst(ty: Ty<'_>) -> bool {
match ty.kind() {
// maybe ZST (could be more precise)
ty::Adt(..) | ty::Array(..) | ty::Closure(..) | ty::Tuple(..) | ty::Opaque(..) => true,
// definitely ZST
ty::FnDef(..) | ty::Never => true,
// unreachable or can't be ZST
_ => false,
}
}
79 changes: 27 additions & 52 deletions compiler/rustc_mir/src/transform/simplify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,10 @@ use crate::transform::MirPass;
use rustc_index::vec::{Idx, IndexVec};
use rustc_middle::mir::visit::{MutVisitor, MutatingUseContext, PlaceContext, Visitor};
use rustc_middle::mir::*;
use rustc_middle::ty::ParamEnv;
use rustc_middle::ty::TyCtxt;
use smallvec::SmallVec;
use std::{borrow::Cow, convert::TryInto};
use std::borrow::Cow;
use std::convert::TryInto;

pub struct SimplifyCfg {
label: String,
Expand Down Expand Up @@ -326,7 +326,7 @@ impl<'tcx> MirPass<'tcx> for SimplifyLocals {

pub fn simplify_locals<'tcx>(body: &mut Body<'tcx>, tcx: TyCtxt<'tcx>) {
// First, we're going to get a count of *actual* uses for every `Local`.
let mut used_locals = UsedLocals::new(body, tcx);
let mut used_locals = UsedLocals::new(body);

// Next, we're going to remove any `Local` with zero actual uses. When we remove those
// `Locals`, we're also going to subtract any uses of other `Locals` from the `used_locals`
Expand All @@ -336,8 +336,7 @@ pub fn simplify_locals<'tcx>(body: &mut Body<'tcx>, tcx: TyCtxt<'tcx>) {
remove_unused_definitions(&mut used_locals, body);

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

// Only bother running the `LocalUpdater` if we actually found locals to remove.
if map.iter().any(Option::is_none) {
Expand All @@ -350,61 +349,54 @@ pub fn simplify_locals<'tcx>(body: &mut Body<'tcx>, tcx: TyCtxt<'tcx>) {
}

/// Construct the mapping while swapping out unused stuff out from the `vec`.
fn make_local_map<'tcx, V>(
fn make_local_map<V>(
local_decls: &mut IndexVec<Local, V>,
used_locals: &UsedLocals<'tcx>,
arg_count: u32,
used_locals: &UsedLocals,
) -> IndexVec<Local, Option<Local>> {
let mut map: IndexVec<Local, Option<Local>> = IndexVec::from_elem(None, local_decls);
let mut map: IndexVec<Local, Option<Local>> = IndexVec::from_elem(None, &*local_decls);
let mut used = Local::new(0);

for alive_index in local_decls.indices() {
// When creating the local map treat the `RETURN_PLACE` and arguments as used.
if alive_index.as_u32() <= arg_count || used_locals.is_used(alive_index) {
map[alive_index] = Some(used);
if alive_index != used {
local_decls.swap(alive_index, used);
}
used.increment_by(1);
// `is_used` treats the `RETURN_PLACE` and arguments as used.
if !used_locals.is_used(alive_index) {
continue;
}

map[alive_index] = Some(used);
if alive_index != used {
local_decls.swap(alive_index, used);
}
used.increment_by(1);
}
local_decls.truncate(used.index());
map
}

/// Keeps track of used & unused locals.
struct UsedLocals<'tcx> {
struct UsedLocals {
increment: bool,
arg_count: u32,
use_count: IndexVec<Local, u32>,
is_static: bool,
local_decls: IndexVec<Local, LocalDecl<'tcx>>,
param_env: ParamEnv<'tcx>,
tcx: TyCtxt<'tcx>,
}

impl UsedLocals<'tcx> {
impl UsedLocals {
/// Determines which locals are used & unused in the given body.
fn new(body: &Body<'tcx>, tcx: TyCtxt<'tcx>) -> Self {
let def_id = body.source.def_id();
let is_static = tcx.is_static(def_id);
let param_env = tcx.param_env(def_id);
let local_decls = body.local_decls.clone();
fn new(body: &Body<'_>) -> Self {
let mut this = Self {
increment: true,
arg_count: body.arg_count.try_into().unwrap(),
use_count: IndexVec::from_elem(0, &body.local_decls),
is_static,
local_decls,
param_env,
tcx,
};
this.visit_body(body);
this
}

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

/// Updates the use counts to reflect the removal of given statement.
Expand Down Expand Up @@ -434,7 +426,7 @@ impl UsedLocals<'tcx> {
}
}

impl Visitor<'tcx> for UsedLocals<'tcx> {
impl Visitor<'_> for UsedLocals {
fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) {
match statement.kind {
StatementKind::LlvmInlineAsm(..)
Expand All @@ -461,21 +453,7 @@ impl Visitor<'tcx> for UsedLocals<'tcx> {
}
}

fn visit_local(&mut self, local: &Local, ctx: PlaceContext, _location: Location) {
debug!("local: {:?} is_static: {:?}, ctx: {:?}", local, self.is_static, ctx);
// Do not count _0 as a used in `return;` if it is a ZST.
let return_place = *local == RETURN_PLACE
&& matches!(ctx, PlaceContext::NonMutatingUse(visit::NonMutatingUseContext::Move));
if !self.is_static && return_place {
let ty = self.local_decls[*local].ty;
let param_env_and = self.param_env.and(ty);
if let Ok(layout) = self.tcx.layout_of(param_env_and) {
debug!("layout.is_zst: {:?}", layout.is_zst());
if layout.is_zst() {
return;
}
}
}
fn visit_local(&mut self, local: &Local, _ctx: PlaceContext, _location: Location) {
if self.increment {
self.use_count[*local] += 1;
} else {
Expand All @@ -486,10 +464,7 @@ impl Visitor<'tcx> for UsedLocals<'tcx> {
}

/// Removes unused definitions. Updates the used locals to reflect the changes made.
fn remove_unused_definitions<'a, 'tcx>(
used_locals: &'a mut UsedLocals<'tcx>,
body: &mut Body<'tcx>,
) {
fn remove_unused_definitions<'a, 'tcx>(used_locals: &'a mut UsedLocals, body: &mut Body<'tcx>) {
// The use counts are updated as we remove the statements. A local might become unused
// during the retain operation, leading to a temporary inconsistency (storage statements or
// definitions referencing the local might remain). For correctness it is crucial that this
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ fn main() -> () {
_1 = (*_2); // scope 0 at $DIR/const_allocation.rs:8:5: 8:8
StorageDead(_2); // scope 0 at $DIR/const_allocation.rs:8:8: 8:9
StorageDead(_1); // scope 0 at $DIR/const_allocation.rs:8:8: 8:9
_0 = const (); // scope 0 at $DIR/const_allocation.rs:7:11: 9:2
nop; // scope 0 at $DIR/const_allocation.rs:7:11: 9:2
return; // scope 0 at $DIR/const_allocation.rs:9:2: 9:2
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ fn main() -> () {
_1 = (*_2); // scope 0 at $DIR/const_allocation.rs:8:5: 8:8
StorageDead(_2); // scope 0 at $DIR/const_allocation.rs:8:8: 8:9
StorageDead(_1); // scope 0 at $DIR/const_allocation.rs:8:8: 8:9
_0 = const (); // scope 0 at $DIR/const_allocation.rs:7:11: 9:2
nop; // scope 0 at $DIR/const_allocation.rs:7:11: 9:2
return; // scope 0 at $DIR/const_allocation.rs:9:2: 9:2
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ fn main() -> () {
_1 = (*_2); // scope 0 at $DIR/const_allocation2.rs:5:5: 5:8
StorageDead(_2); // scope 0 at $DIR/const_allocation2.rs:5:8: 5:9
StorageDead(_1); // scope 0 at $DIR/const_allocation2.rs:5:8: 5:9
_0 = const (); // scope 0 at $DIR/const_allocation2.rs:4:11: 6:2
nop; // scope 0 at $DIR/const_allocation2.rs:4:11: 6:2
return; // scope 0 at $DIR/const_allocation2.rs:6:2: 6:2
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ fn main() -> () {
_1 = (*_2); // scope 0 at $DIR/const_allocation2.rs:5:5: 5:8
StorageDead(_2); // scope 0 at $DIR/const_allocation2.rs:5:8: 5:9
StorageDead(_1); // scope 0 at $DIR/const_allocation2.rs:5:8: 5:9
_0 = const (); // scope 0 at $DIR/const_allocation2.rs:4:11: 6:2
nop; // scope 0 at $DIR/const_allocation2.rs:4:11: 6:2
return; // scope 0 at $DIR/const_allocation2.rs:6:2: 6:2
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ fn main() -> () {
_1 = (*_2); // scope 0 at $DIR/const_allocation3.rs:5:5: 5:8
StorageDead(_2); // scope 0 at $DIR/const_allocation3.rs:5:8: 5:9
StorageDead(_1); // scope 0 at $DIR/const_allocation3.rs:5:8: 5:9
_0 = const (); // scope 0 at $DIR/const_allocation3.rs:4:11: 6:2
nop; // scope 0 at $DIR/const_allocation3.rs:4:11: 6:2
return; // scope 0 at $DIR/const_allocation3.rs:6:2: 6:2
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ fn main() -> () {
_1 = (*_2); // scope 0 at $DIR/const_allocation3.rs:5:5: 5:8
StorageDead(_2); // scope 0 at $DIR/const_allocation3.rs:5:8: 5:9
StorageDead(_1); // scope 0 at $DIR/const_allocation3.rs:5:8: 5:9
_0 = const (); // scope 0 at $DIR/const_allocation3.rs:4:11: 6:2
nop; // scope 0 at $DIR/const_allocation3.rs:4:11: 6:2
return; // scope 0 at $DIR/const_allocation3.rs:6:2: 6:2
}
}
Expand Down
1 change: 0 additions & 1 deletion src/test/mir-opt/const_debuginfo.main.ConstDebugInfo.diff
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@
_13 = const 64_u32; // scope 8 at $DIR/const_debuginfo.rs:21:13: 21:22
StorageDead(_15); // scope 8 at $DIR/const_debuginfo.rs:21:21: 21:22
StorageDead(_14); // scope 8 at $DIR/const_debuginfo.rs:21:21: 21:22
_0 = const (); // scope 0 at $DIR/const_debuginfo.rs:8:11: 22:2
StorageDead(_13); // scope 8 at $DIR/const_debuginfo.rs:22:1: 22:2
StorageDead(_12); // scope 7 at $DIR/const_debuginfo.rs:22:1: 22:2
StorageDead(_11); // scope 6 at $DIR/const_debuginfo.rs:22:1: 22:2
Expand Down
2 changes: 1 addition & 1 deletion src/test/mir-opt/const_prop/aggregate.main.ConstProp.diff
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
+ _1 = const 1_i32; // scope 0 at $DIR/aggregate.rs:5:13: 5:28
StorageDead(_2); // scope 0 at $DIR/aggregate.rs:5:27: 5:28
StorageDead(_3); // scope 0 at $DIR/aggregate.rs:5:28: 5:29
_0 = const (); // scope 0 at $DIR/aggregate.rs:4:11: 6:2
nop; // scope 0 at $DIR/aggregate.rs:4:11: 6:2
StorageDead(_1); // scope 0 at $DIR/aggregate.rs:6:1: 6:2
return; // scope 0 at $DIR/aggregate.rs:6:2: 6:2
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
+ _1 = const 2_u32; // scope 0 at $DIR/array_index.rs:5:18: 5:33
StorageDead(_3); // scope 0 at $DIR/array_index.rs:5:33: 5:34
StorageDead(_2); // scope 0 at $DIR/array_index.rs:5:33: 5:34
_0 = const (); // scope 0 at $DIR/array_index.rs:4:11: 6:2
nop; // scope 0 at $DIR/array_index.rs:4:11: 6:2
StorageDead(_1); // scope 0 at $DIR/array_index.rs:6:1: 6:2
return; // scope 0 at $DIR/array_index.rs:6:2: 6:2
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
+ _1 = const 2_u32; // scope 0 at $DIR/array_index.rs:5:18: 5:33
StorageDead(_3); // scope 0 at $DIR/array_index.rs:5:33: 5:34
StorageDead(_2); // scope 0 at $DIR/array_index.rs:5:33: 5:34
_0 = const (); // scope 0 at $DIR/array_index.rs:4:11: 6:2
nop; // scope 0 at $DIR/array_index.rs:4:11: 6:2
StorageDead(_1); // scope 0 at $DIR/array_index.rs:6:1: 6:2
return; // scope 0 at $DIR/array_index.rs:6:2: 6:2
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
- _2 = Div(const 1_i32, move _3); // scope 1 at $DIR/bad_op_div_by_zero.rs:5:14: 5:19
+ _2 = Div(const 1_i32, const 0_i32); // scope 1 at $DIR/bad_op_div_by_zero.rs:5:14: 5:19
StorageDead(_3); // scope 1 at $DIR/bad_op_div_by_zero.rs:5:18: 5:19
_0 = const (); // scope 0 at $DIR/bad_op_div_by_zero.rs:3:11: 6:2
nop; // scope 0 at $DIR/bad_op_div_by_zero.rs:3:11: 6:2
StorageDead(_2); // scope 1 at $DIR/bad_op_div_by_zero.rs:6:1: 6:2
StorageDead(_1); // scope 0 at $DIR/bad_op_div_by_zero.rs:6:1: 6:2
return; // scope 0 at $DIR/bad_op_div_by_zero.rs:6:2: 6:2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
- _2 = Rem(const 1_i32, move _3); // scope 1 at $DIR/bad_op_mod_by_zero.rs:5:14: 5:19
+ _2 = Rem(const 1_i32, const 0_i32); // scope 1 at $DIR/bad_op_mod_by_zero.rs:5:14: 5:19
StorageDead(_3); // scope 1 at $DIR/bad_op_mod_by_zero.rs:5:18: 5:19
_0 = const (); // scope 0 at $DIR/bad_op_mod_by_zero.rs:3:11: 6:2
nop; // scope 0 at $DIR/bad_op_mod_by_zero.rs:3:11: 6:2
StorageDead(_2); // scope 1 at $DIR/bad_op_mod_by_zero.rs:6:1: 6:2
StorageDead(_1); // scope 0 at $DIR/bad_op_mod_by_zero.rs:6:1: 6:2
return; // scope 0 at $DIR/bad_op_mod_by_zero.rs:6:2: 6:2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
bb1: {
_5 = (*_1)[_6]; // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25
StorageDead(_6); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:25: 7:26
_0 = const (); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:6:5: 8:6
nop; // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:6:5: 8:6
StorageDead(_5); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:8:5: 8:6
StorageDead(_1); // scope 0 at $DIR/bad_op_unsafe_oob_for_slices.rs:9:1: 9:2
return; // scope 0 at $DIR/bad_op_unsafe_oob_for_slices.rs:9:2: 9:2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
bb1: {
_5 = (*_1)[_6]; // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:18: 7:25
StorageDead(_6); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:7:25: 7:26
_0 = const (); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:6:5: 8:6
nop; // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:6:5: 8:6
StorageDead(_5); // scope 2 at $DIR/bad_op_unsafe_oob_for_slices.rs:8:5: 8:6
StorageDead(_1); // scope 0 at $DIR/bad_op_unsafe_oob_for_slices.rs:9:1: 9:2
return; // scope 0 at $DIR/bad_op_unsafe_oob_for_slices.rs:9:2: 9:2
Expand Down
2 changes: 1 addition & 1 deletion src/test/mir-opt/const_prop/boxes.main.ConstProp.diff
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

bb1: {
StorageDead(_3); // scope 0 at $DIR/boxes.rs:12:26: 12:27
_0 = const (); // scope 0 at $DIR/boxes.rs:11:11: 13:2
nop; // scope 0 at $DIR/boxes.rs:11:11: 13:2
StorageDead(_1); // scope 0 at $DIR/boxes.rs:13:1: 13:2
return; // scope 0 at $DIR/boxes.rs:13:2: 13:2
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/mir-opt/const_prop/cast.main.ConstProp.diff
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
StorageLive(_2); // scope 1 at $DIR/cast.rs:6:9: 6:10
- _2 = const 42_u32 as u8 (Misc); // scope 1 at $DIR/cast.rs:6:13: 6:24
+ _2 = const 42_u8; // scope 1 at $DIR/cast.rs:6:13: 6:24
_0 = const (); // scope 0 at $DIR/cast.rs:3:11: 7:2
nop; // scope 0 at $DIR/cast.rs:3:11: 7:2
StorageDead(_2); // scope 1 at $DIR/cast.rs:7:1: 7:2
StorageDead(_1); // scope 0 at $DIR/cast.rs:7:1: 7:2
return; // scope 0 at $DIR/cast.rs:7:2: 7:2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
bb1: {
- _1 = move (_2.0: u32); // scope 0 at $DIR/checked_add.rs:5:18: 5:23
+ _1 = const 2_u32; // scope 0 at $DIR/checked_add.rs:5:18: 5:23
_0 = const (); // scope 0 at $DIR/checked_add.rs:4:11: 6:2
nop; // scope 0 at $DIR/checked_add.rs:4:11: 6:2
StorageDead(_1); // scope 0 at $DIR/checked_add.rs:6:1: 6:2
return; // scope 0 at $DIR/checked_add.rs:6:2: 6:2
}
Expand Down
Loading

0 comments on commit 79e5814

Please sign in to comment.