Skip to content

Commit 79e5814

Browse files
borsbors
committed
Auto merge of rust-lang#83177 - erikdesjardins:zstassign, r=oli-obk
Remove assignments to ZST places instead of marking ZST return place as unused partially reverts rust-lang#83118 requested by `@tmiasko` in rust-lang#83118 (comment) r? `@oli-obk`
2 parents 4eb0bc7 + 6960bc9 commit 79e5814

File tree

86 files changed

+178
-176
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

86 files changed

+178
-176
lines changed

‎compiler/rustc_mir/src/transform/mod.rs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ pub mod promote_consts;
4444
pub mod remove_noop_landing_pads;
4545
pub mod remove_storage_markers;
4646
pub mod remove_unneeded_drops;
47+
pub mod remove_zsts;
4748
pub mod required_consts;
4849
pub mod rustc_peek;
4950
pub mod simplify;
@@ -494,6 +495,7 @@ fn run_optimization_passes<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
494495
// The main optimizations that we do on MIR.
495496
let optimizations: &[&dyn MirPass<'tcx>] = &[
496497
&remove_storage_markers::RemoveStorageMarkers,
498+
&remove_zsts::RemoveZsts,
497499
&const_goto::ConstGoto,
498500
&remove_unneeded_drops::RemoveUnneededDrops,
499501
&match_branches::MatchBranchSimplification,

‎compiler/rustc_mir/src/transform/remove_zsts.rs‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
//! Removes assignments to ZST places.
2+
3+
use crate::transform::MirPass;
4+
use rustc_middle::mir::{Body, StatementKind};
5+
use rustc_middle::ty::{self, Ty, TyCtxt};
6+
7+
pub struct RemoveZsts;
8+
9+
impl<'tcx> MirPass<'tcx> for RemoveZsts {
10+
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
11+
if tcx.sess.mir_opt_level() < 3 {
12+
return;
13+
}
14+
let param_env = tcx.param_env(body.source.def_id());
15+
let (basic_blocks, local_decls) = body.basic_blocks_and_local_decls_mut();
16+
for block in basic_blocks.iter_mut() {
17+
for statement in block.statements.iter_mut() {
18+
match statement.kind {
19+
StatementKind::Assign(box (place, _)) => {
20+
let place_ty = place.ty(local_decls, tcx).ty;
21+
if !maybe_zst(place_ty) {
22+
continue;
23+
}
24+
let layout = match tcx.layout_of(param_env.and(place_ty)) {
25+
Ok(layout) => layout,
26+
Err(_) => continue,
27+
};
28+
if !layout.is_zst() {
29+
continue;
30+
}
31+
if tcx.consider_optimizing(|| {
32+
format!(
33+
"RemoveZsts - Place: {:?} SourceInfo: {:?}",
34+
place, statement.source_info
35+
)
36+
}) {
37+
statement.make_nop();
38+
}
39+
}
40+
_ => {}
41+
}
42+
}
43+
}
44+
}
45+
}
46+
47+
/// A cheap, approximate check to avoid unnecessary `layout_of` calls.
48+
fn maybe_zst(ty: Ty<'_>) -> bool {
49+
match ty.kind() {
50+
// maybe ZST (could be more precise)
51+
ty::Adt(..) | ty::Array(..) | ty::Closure(..) | ty::Tuple(..) | ty::Opaque(..) => true,
52+
// definitely ZST
53+
ty::FnDef(..) | ty::Never => true,
54+
// unreachable or can't be ZST
55+
_ => false,
56+
}
57+
}

‎compiler/rustc_mir/src/transform/simplify.rs‎

Lines changed: 27 additions & 52 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;
3534
use rustc_middle::ty::TyCtxt;
3635
use smallvec::SmallVec;
37-
use std::{borrow::Cow, convert::TryInto};
36+
use std::borrow::Cow;
37+
use std::convert::TryInto;
3838

3939
pub struct SimplifyCfg {
4040
label: String,
@@ -326,7 +326,7 @@ impl<'tcx> MirPass<'tcx> for SimplifyLocals {
326326

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

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

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

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

352351
/// Construct the mapping while swapping out unused stuff out from the `vec`.
353-
fn make_local_map<'tcx, V>(
352+
fn make_local_map<V>(
354353
local_decls: &mut IndexVec<Local, V>,
355-
used_locals: &UsedLocals<'tcx>,
356-
arg_count: u32,
354+
used_locals: &UsedLocals,
357355
) -> IndexVec<Local, Option<Local>> {
358-
let mut map: IndexVec<Local, Option<Local>> = IndexVec::from_elem(None, local_decls);
356+
let mut map: IndexVec<Local, Option<Local>> = IndexVec::from_elem(None, &*local_decls);
359357
let mut used = Local::new(0);
360358

361359
for alive_index in local_decls.indices() {
362-
// When creating the local map treat the `RETURN_PLACE` and arguments as used.
363-
if alive_index.as_u32() <= arg_count || used_locals.is_used(alive_index) {
364-
map[alive_index] = Some(used);
365-
if alive_index != used {
366-
local_decls.swap(alive_index, used);
367-
}
368-
used.increment_by(1);
360+
// `is_used` treats the `RETURN_PLACE` and arguments as used.
361+
if !used_locals.is_used(alive_index) {
362+
continue;
369363
}
364+
365+
map[alive_index] = Some(used);
366+
if alive_index != used {
367+
local_decls.swap(alive_index, used);
368+
}
369+
used.increment_by(1);
370370
}
371371
local_decls.truncate(used.index());
372372
map
373373
}
374374

375375
/// Keeps track of used & unused locals.
376-
struct UsedLocals<'tcx> {
376+
struct UsedLocals {
377377
increment: bool,
378+
arg_count: u32,
378379
use_count: IndexVec<Local, u32>,
379-
is_static: bool,
380-
local_decls: IndexVec<Local, LocalDecl<'tcx>>,
381-
param_env: ParamEnv<'tcx>,
382-
tcx: TyCtxt<'tcx>,
383380
}
384381

385-
impl UsedLocals<'tcx> {
382+
impl UsedLocals {
386383
/// Determines which locals are used & unused in the given body.
387-
fn new(body: &Body<'tcx>, tcx: TyCtxt<'tcx>) -> Self {
388-
let def_id = body.source.def_id();
389-
let is_static = tcx.is_static(def_id);
390-
let param_env = tcx.param_env(def_id);
391-
let local_decls = body.local_decls.clone();
384+
fn new(body: &Body<'_>) -> Self {
392385
let mut this = Self {
393386
increment: true,
387+
arg_count: body.arg_count.try_into().unwrap(),
394388
use_count: IndexVec::from_elem(0, &body.local_decls),
395-
is_static,
396-
local_decls,
397-
param_env,
398-
tcx,
399389
};
400390
this.visit_body(body);
401391
this
402392
}
403393

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

410402
/// Updates the use counts to reflect the removal of given statement.
@@ -434,7 +426,7 @@ impl UsedLocals<'tcx> {
434426
}
435427
}
436428

437-
impl Visitor<'tcx> for UsedLocals<'tcx> {
429+
impl Visitor<'_> for UsedLocals {
438430
fn visit_statement(&mut self, statement: &Statement<'tcx>, location: Location) {
439431
match statement.kind {
440432
StatementKind::LlvmInlineAsm(..)
@@ -461,21 +453,7 @@ impl Visitor<'tcx> for UsedLocals<'tcx> {
461453
}
462454
}
463455

464-
fn visit_local(&mut self, local: &Local, ctx: PlaceContext, _location: Location) {
465-
debug!("local: {:?} is_static: {:?}, ctx: {:?}", local, self.is_static, ctx);
466-
// Do not count _0 as a used in `return;` if it is a ZST.
467-
let return_place = *local == RETURN_PLACE
468-
&& matches!(ctx, PlaceContext::NonMutatingUse(visit::NonMutatingUseContext::Move));
469-
if !self.is_static && return_place {
470-
let ty = self.local_decls[*local].ty;
471-
let param_env_and = self.param_env.and(ty);
472-
if let Ok(layout) = self.tcx.layout_of(param_env_and) {
473-
debug!("layout.is_zst: {:?}", layout.is_zst());
474-
if layout.is_zst() {
475-
return;
476-
}
477-
}
478-
}
456+
fn visit_local(&mut self, local: &Local, _ctx: PlaceContext, _location: Location) {
479457
if self.increment {
480458
self.use_count[*local] += 1;
481459
} else {
@@ -486,10 +464,7 @@ impl Visitor<'tcx> for UsedLocals<'tcx> {
486464
}
487465

488466
/// Removes unused definitions. Updates the used locals to reflect the changes made.
489-
fn remove_unused_definitions<'a, 'tcx>(
490-
used_locals: &'a mut UsedLocals<'tcx>,
491-
body: &mut Body<'tcx>,
492-
) {
467+
fn remove_unused_definitions<'a, 'tcx>(used_locals: &'a mut UsedLocals, body: &mut Body<'tcx>) {
493468
// The use counts are updated as we remove the statements. A local might become unused
494469
// during the retain operation, leading to a temporary inconsistency (storage statements or
495470
// definitions referencing the local might remain). For correctness it is crucial that this

‎src/test/mir-opt/const_allocation.main.ConstProp.after.32bit.mir‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ fn main() -> () {
1818
_1 = (*_2); // scope 0 at $DIR/const_allocation.rs:8:5: 8:8
1919
StorageDead(_2); // scope 0 at $DIR/const_allocation.rs:8:8: 8:9
2020
StorageDead(_1); // scope 0 at $DIR/const_allocation.rs:8:8: 8:9
21-
_0 = const (); // scope 0 at $DIR/const_allocation.rs:7:11: 9:2
21+
nop; // scope 0 at $DIR/const_allocation.rs:7:11: 9:2
2222
return; // scope 0 at $DIR/const_allocation.rs:9:2: 9:2
2323
}
2424
}

‎src/test/mir-opt/const_allocation.main.ConstProp.after.64bit.mir‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ fn main() -> () {
1818
_1 = (*_2); // scope 0 at $DIR/const_allocation.rs:8:5: 8:8
1919
StorageDead(_2); // scope 0 at $DIR/const_allocation.rs:8:8: 8:9
2020
StorageDead(_1); // scope 0 at $DIR/const_allocation.rs:8:8: 8:9
21-
_0 = const (); // scope 0 at $DIR/const_allocation.rs:7:11: 9:2
21+
nop; // scope 0 at $DIR/const_allocation.rs:7:11: 9:2
2222
return; // scope 0 at $DIR/const_allocation.rs:9:2: 9:2
2323
}
2424
}

‎src/test/mir-opt/const_allocation2.main.ConstProp.after.32bit.mir‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ fn main() -> () {
1818
_1 = (*_2); // scope 0 at $DIR/const_allocation2.rs:5:5: 5:8
1919
StorageDead(_2); // scope 0 at $DIR/const_allocation2.rs:5:8: 5:9
2020
StorageDead(_1); // scope 0 at $DIR/const_allocation2.rs:5:8: 5:9
21-
_0 = const (); // scope 0 at $DIR/const_allocation2.rs:4:11: 6:2
21+
nop; // scope 0 at $DIR/const_allocation2.rs:4:11: 6:2
2222
return; // scope 0 at $DIR/const_allocation2.rs:6:2: 6:2
2323
}
2424
}

‎src/test/mir-opt/const_allocation2.main.ConstProp.after.64bit.mir‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ fn main() -> () {
1818
_1 = (*_2); // scope 0 at $DIR/const_allocation2.rs:5:5: 5:8
1919
StorageDead(_2); // scope 0 at $DIR/const_allocation2.rs:5:8: 5:9
2020
StorageDead(_1); // scope 0 at $DIR/const_allocation2.rs:5:8: 5:9
21-
_0 = const (); // scope 0 at $DIR/const_allocation2.rs:4:11: 6:2
21+
nop; // scope 0 at $DIR/const_allocation2.rs:4:11: 6:2
2222
return; // scope 0 at $DIR/const_allocation2.rs:6:2: 6:2
2323
}
2424
}

‎src/test/mir-opt/const_allocation3.main.ConstProp.after.32bit.mir‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ fn main() -> () {
1818
_1 = (*_2); // scope 0 at $DIR/const_allocation3.rs:5:5: 5:8
1919
StorageDead(_2); // scope 0 at $DIR/const_allocation3.rs:5:8: 5:9
2020
StorageDead(_1); // scope 0 at $DIR/const_allocation3.rs:5:8: 5:9
21-
_0 = const (); // scope 0 at $DIR/const_allocation3.rs:4:11: 6:2
21+
nop; // scope 0 at $DIR/const_allocation3.rs:4:11: 6:2
2222
return; // scope 0 at $DIR/const_allocation3.rs:6:2: 6:2
2323
}
2424
}

‎src/test/mir-opt/const_allocation3.main.ConstProp.after.64bit.mir‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ fn main() -> () {
1818
_1 = (*_2); // scope 0 at $DIR/const_allocation3.rs:5:5: 5:8
1919
StorageDead(_2); // scope 0 at $DIR/const_allocation3.rs:5:8: 5:9
2020
StorageDead(_1); // scope 0 at $DIR/const_allocation3.rs:5:8: 5:9
21-
_0 = const (); // scope 0 at $DIR/const_allocation3.rs:4:11: 6:2
21+
nop; // scope 0 at $DIR/const_allocation3.rs:4:11: 6:2
2222
return; // scope 0 at $DIR/const_allocation3.rs:6:2: 6:2
2323
}
2424
}

‎src/test/mir-opt/const_debuginfo.main.ConstDebugInfo.diff‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,6 @@
9999
_13 = const 64_u32; // scope 8 at $DIR/const_debuginfo.rs:21:13: 21:22
100100
StorageDead(_15); // scope 8 at $DIR/const_debuginfo.rs:21:21: 21:22
101101
StorageDead(_14); // scope 8 at $DIR/const_debuginfo.rs:21:21: 21:22
102-
_0 = const (); // scope 0 at $DIR/const_debuginfo.rs:8:11: 22:2
103102
StorageDead(_13); // scope 8 at $DIR/const_debuginfo.rs:22:1: 22:2
104103
StorageDead(_12); // scope 7 at $DIR/const_debuginfo.rs:22:1: 22:2
105104
StorageDead(_11); // scope 6 at $DIR/const_debuginfo.rs:22:1: 22:2

0 commit comments

Comments
 (0)