Skip to content

Remove MIR assignments to ZST types #80493

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 3 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion compiler/rustc_codegen_ssa/src/mir/analyze.rs
Original file line number Diff line number Diff line change
@@ -281,7 +281,18 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx>
Some(assignment_location) => {
assignment_location.dominates(location, &self.dominators)
}
None => false,
None => {
debug!("No first assignment found for {:?}", local);
// We have not seen any assignment to the local yet,
// but before marking not_ssa, check if it is a ZST,
// in which case we don't need to initialize the local.
let ty = self.fx.mir.local_decls[local].ty;
let ty = self.fx.monomorphize(ty);

let is_zst = self.fx.cx.layout_of(ty).is_zst();
debug!("is_zst: {}", is_zst);
is_zst
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if anything breaks if we do in fact just go ahead and unconditionally mark any variable without assignments as "ssa".

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried replacing is_zst with true which at least didn't break test/{mir-opt,codegen}. Should i push that instead of the current zst handling?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets not do that in the current PR, especially because apparently this was added as part of a ICE fix, but without tests. I will experiment with it separately.

}
};
if !ssa_read {
self.not_ssa(local);
81 changes: 54 additions & 27 deletions compiler/rustc_mir/src/transform/simplify.rs
Original file line number Diff line number Diff line change
@@ -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;
use std::convert::TryInto;
use std::{borrow::Cow, convert::TryInto};

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

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

// 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`
@@ -332,7 +332,8 @@ impl<'tcx> MirPass<'tcx> for SimplifyLocals {
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 map = make_local_map(&mut body.local_decls, &used_locals);
let arg_count = body.arg_count.try_into().unwrap();
let map = make_local_map(&mut body.local_decls, &used_locals, arg_count);

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

/// Construct the mapping while swapping out unused stuff out from the `vec`.
fn make_local_map<V>(
fn make_local_map<'tcx, V>(
local_decls: &mut IndexVec<Local, V>,
used_locals: &UsedLocals,
used_locals: &UsedLocals<'tcx>,
arg_count: u32,
) -> 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() {
// `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);
// 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);
}
used.increment_by(1);
}
local_decls.truncate(used.index());
map
}

/// Keeps track of used & unused locals.
struct UsedLocals {
struct UsedLocals<'tcx> {
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 {
impl UsedLocals<'tcx> {
/// Determines which locals are used & unused in the given body.
fn new(body: &Body<'_>) -> Self {
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();
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]);
local.as_u32() <= self.arg_count || self.use_count[local] != 0
self.use_count[local] != 0
}

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

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

fn visit_local(&mut self, local: &Local, _ctx: PlaceContext, _location: Location) {
fn visit_local(&mut self, local: &Local, ctx: PlaceContext, _location: Location) {
debug!("local: {:?} is_static: {:?}, ctx: {:?}", local, self.is_static, ctx);
// Do not count a local as used in `_local = <rhs>` if RHS is a ZST.
let store = matches!(ctx, PlaceContext::MutatingUse(MutatingUseContext::Store));
// 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 && (store || 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;
}
}
}
if self.increment {
self.use_count[*local] += 1;
} else {
@@ -459,7 +483,10 @@ impl Visitor<'_> for UsedLocals {
}

/// Removes unused definitions. Updates the used locals to reflect the changes made.
fn remove_unused_definitions<'a, 'tcx>(used_locals: &'a mut UsedLocals, body: &mut Body<'tcx>) {
fn remove_unused_definitions<'a, 'tcx>(
used_locals: &'a mut UsedLocals<'tcx>,
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
Original file line number Diff line number Diff line change
@@ -4,7 +4,6 @@ fn hello() -> () {
let mut _0: (); // return place in scope 0 at $DIR/control-flow-simplification.rs:11:14: 11:14

bb0: {
_0 = const (); // scope 0 at $DIR/control-flow-simplification.rs:14:6: 14:6
return; // scope 0 at $DIR/control-flow-simplification.rs:15:2: 15:2
}
}
Original file line number Diff line number Diff line change
@@ -22,7 +22,6 @@ fn main() -> () {
_2 = const 3_i32; // scope 1 at $DIR/optimizes_into_variable.rs:13:13: 13:34
StorageLive(_3); // scope 2 at $DIR/optimizes_into_variable.rs:14:9: 14:10
_3 = const 42_u32; // scope 2 at $DIR/optimizes_into_variable.rs:14:13: 14:38
_0 = const (); // scope 0 at $DIR/optimizes_into_variable.rs:11:11: 15:2
StorageDead(_3); // scope 2 at $DIR/optimizes_into_variable.rs:15:1: 15:2
StorageDead(_2); // scope 1 at $DIR/optimizes_into_variable.rs:15:1: 15:2
StorageDead(_1); // scope 0 at $DIR/optimizes_into_variable.rs:15:1: 15:2
Original file line number Diff line number Diff line change
@@ -22,7 +22,6 @@ fn main() -> () {
_2 = const 3_i32; // scope 1 at $DIR/optimizes_into_variable.rs:13:13: 13:34
StorageLive(_3); // scope 2 at $DIR/optimizes_into_variable.rs:14:9: 14:10
_3 = const 42_u32; // scope 2 at $DIR/optimizes_into_variable.rs:14:13: 14:38
_0 = const (); // scope 0 at $DIR/optimizes_into_variable.rs:11:11: 15:2
StorageDead(_3); // scope 2 at $DIR/optimizes_into_variable.rs:15:1: 15:2
StorageDead(_2); // scope 1 at $DIR/optimizes_into_variable.rs:15:1: 15:2
StorageDead(_1); // scope 0 at $DIR/optimizes_into_variable.rs:15:1: 15:2
Original file line number Diff line number Diff line change
@@ -56,7 +56,6 @@
StorageLive(_6); // scope 3 at $DIR/cycle.rs:14:10: 14:11
- _6 = _1; // scope 3 at $DIR/cycle.rs:14:10: 14:11
+ _6 = _4; // scope 3 at $DIR/cycle.rs:14:10: 14:11
_5 = const (); // scope 4 at $DIR/cycle.rs:14:5: 14:12
StorageDead(_6); // scope 3 at $DIR/cycle.rs:14:11: 14:12
StorageDead(_5); // scope 3 at $DIR/cycle.rs:14:12: 14:13
_0 = const (); // scope 0 at $DIR/cycle.rs:8:11: 15:2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some assignments are not removed compared to earlier versions of this PR. Why are those not optimized now?

Original file line number Diff line number Diff line change
@@ -31,7 +31,6 @@
StorageLive(_3); // scope 1 at $DIR/union.rs:15:5: 15:27
StorageLive(_4); // scope 1 at $DIR/union.rs:15:10: 15:26
_4 = (_1.0: u32); // scope 2 at $DIR/union.rs:15:19: 15:24
_3 = const (); // scope 3 at $DIR/union.rs:15:5: 15:27
StorageDead(_4); // scope 1 at $DIR/union.rs:15:26: 15:27
StorageDead(_3); // scope 1 at $DIR/union.rs:15:27: 15:28
_0 = const (); // scope 0 at $DIR/union.rs:8:11: 16:2
Original file line number Diff line number Diff line change
@@ -16,7 +16,6 @@
- }
-
- bb1: {
+ _1 = const (); // scope 1 at $DIR/inline-compatibility.rs:24:5: 24:18
StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:24:18: 24:19
_0 = const (); // scope 0 at $DIR/inline-compatibility.rs:23:37: 25:2
return; // scope 0 at $DIR/inline-compatibility.rs:25:2: 25:2
Original file line number Diff line number Diff line change
@@ -16,7 +16,6 @@
- }
-
- bb1: {
+ _1 = const (); // scope 1 at $DIR/inline-compatibility.rs:13:5: 13:21
StorageDead(_1); // scope 0 at $DIR/inline-compatibility.rs:13:21: 13:22
_0 = const (); // scope 0 at $DIR/inline-compatibility.rs:12:40: 14:2
return; // scope 0 at $DIR/inline-compatibility.rs:14:2: 14:2
1 change: 0 additions & 1 deletion src/test/mir-opt/inline/inline_cycle.two.Inline.diff
Original file line number Diff line number Diff line change
@@ -37,7 +37,6 @@
+ StorageDead(_5); // scope 1 at $DIR/inline-cycle.rs:49:5: 49:12
+ StorageDead(_4); // scope 1 at $DIR/inline-cycle.rs:49:5: 49:12
+ StorageDead(_3); // scope 1 at $DIR/inline-cycle.rs:49:5: 49:12
+ _1 = const (); // scope 1 at $DIR/inline-cycle.rs:49:5: 49:12
+ StorageDead(_2); // scope 0 at $DIR/inline-cycle.rs:49:5: 49:12
StorageDead(_1); // scope 0 at $DIR/inline-cycle.rs:49:12: 49:13
_0 = const (); // scope 0 at $DIR/inline-cycle.rs:48:10: 50:2
Original file line number Diff line number Diff line change
@@ -48,7 +48,6 @@ fn main() -> () {

bb4: {
StorageDead(_5); // scope 1 at $DIR/inline-options.rs:10:5: 10:21
_2 = const (); // scope 1 at $DIR/inline-options.rs:10:5: 10:21
StorageDead(_2); // scope 0 at $DIR/inline-options.rs:10:21: 10:22
_0 = const (); // scope 0 at $DIR/inline-options.rs:8:11: 11:2
return; // scope 0 at $DIR/inline-options.rs:11:2: 11:2
Original file line number Diff line number Diff line change
@@ -29,7 +29,6 @@ fn main() -> () {
_5 = move (_3.0: ()); // scope 1 at $DIR/issue-76997-inline-scopes-parenting.rs:6:5: 6:10
StorageLive(_6); // scope 2 at $DIR/issue-76997-inline-scopes-parenting.rs:6:5: 6:10
_6 = const (); // scope 2 at $DIR/issue-76997-inline-scopes-parenting.rs:6:5: 6:10
_0 = const (); // scope 3 at $DIR/issue-76997-inline-scopes-parenting.rs:6:5: 6:10
StorageDead(_6); // scope 2 at $DIR/issue-76997-inline-scopes-parenting.rs:6:5: 6:10
StorageDead(_5); // scope 1 at $DIR/issue-76997-inline-scopes-parenting.rs:6:5: 6:10
StorageDead(_4); // scope 1 at $DIR/issue-76997-inline-scopes-parenting.rs:6:9: 6:10
1 change: 0 additions & 1 deletion src/test/mir-opt/issue_73223.main.PreCodegen.32bit.diff
Original file line number Diff line number Diff line change
@@ -138,7 +138,6 @@
StorageDead(_9); // scope 4 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
StorageDead(_8); // scope 3 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
StorageDead(_7); // scope 3 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_0 = const (); // scope 0 at $DIR/issue-73223.rs:1:11: 9:2
StorageDead(_1); // scope 0 at $DIR/issue-73223.rs:9:1: 9:2
return; // scope 0 at $DIR/issue-73223.rs:9:2: 9:2
}
1 change: 0 additions & 1 deletion src/test/mir-opt/issue_73223.main.PreCodegen.64bit.diff
Original file line number Diff line number Diff line change
@@ -138,7 +138,6 @@
StorageDead(_9); // scope 4 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
StorageDead(_8); // scope 3 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
StorageDead(_7); // scope 3 at $SRC_DIR/core/src/macros/mod.rs:LL:COL
_0 = const (); // scope 0 at $DIR/issue-73223.rs:1:11: 9:2
StorageDead(_1); // scope 0 at $DIR/issue-73223.rs:9:1: 9:2
return; // scope 0 at $DIR/issue-73223.rs:9:2: 9:2
}
Original file line number Diff line number Diff line change
@@ -25,7 +25,6 @@ fn f_u64() -> () {
bb1: {
StorageDead(_3); // scope 1 at $DIR/lower_intrinsics.rs:35:5: 35:21
StorageDead(_2); // scope 1 at $DIR/lower_intrinsics.rs:35:5: 35:21
_0 = const (); // scope 0 at $DIR/lower_intrinsics.rs:34:16: 36:2
return; // scope 0 at $DIR/lower_intrinsics.rs:36:2: 36:2
}
}
Original file line number Diff line number Diff line change
@@ -22,7 +22,6 @@ fn f_unit() -> () {
bb1: {
StorageDead(_2); // scope 1 at $DIR/lower_intrinsics.rs:29:5: 29:19
StorageDead(_1); // scope 0 at $DIR/lower_intrinsics.rs:29:18: 29:19
_0 = const (); // scope 0 at $DIR/lower_intrinsics.rs:28:17: 30:2
return; // scope 0 at $DIR/lower_intrinsics.rs:30:2: 30:2
}
}
Original file line number Diff line number Diff line change
@@ -13,12 +13,10 @@
}

bb1: {
_0 = const (); // scope 0 at $DIR/multiple_return_terminators.rs:5:10: 7:6
goto -> bb3; // scope 0 at $DIR/multiple_return_terminators.rs:5:5: 9:6
}

bb2: {
_0 = const (); // scope 0 at $DIR/multiple_return_terminators.rs:7:12: 9:6
goto -> bb3; // scope 0 at $DIR/multiple_return_terminators.rs:5:5: 9:6
}

Original file line number Diff line number Diff line change
@@ -14,7 +14,6 @@
StorageLive(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:21:5: 21:12
StorageLive(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:21:10: 21:11
_3 = move _1; // scope 0 at $DIR/remove_unneeded_drops.rs:21:10: 21:11
_2 = const (); // scope 1 at $DIR/remove_unneeded_drops.rs:21:5: 21:12
drop(_3) -> [return: bb2, unwind: bb1]; // scope 1 at $DIR/remove_unneeded_drops.rs:21:5: 21:12
}

Original file line number Diff line number Diff line change
@@ -14,7 +14,6 @@
StorageLive(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:9:5: 9:12
StorageLive(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:9:10: 9:11
_3 = move _1; // scope 0 at $DIR/remove_unneeded_drops.rs:9:10: 9:11
_2 = const (); // scope 1 at $DIR/remove_unneeded_drops.rs:9:5: 9:12
drop(_3) -> [return: bb2, unwind: bb1]; // scope 1 at $DIR/remove_unneeded_drops.rs:9:5: 9:12
}

Original file line number Diff line number Diff line change
@@ -14,7 +14,6 @@
StorageLive(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:4:5: 4:12
StorageLive(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:4:10: 4:11
_3 = _1; // scope 0 at $DIR/remove_unneeded_drops.rs:4:10: 4:11
_2 = const (); // scope 1 at $DIR/remove_unneeded_drops.rs:4:5: 4:12
- drop(_3) -> bb1; // scope 1 at $DIR/remove_unneeded_drops.rs:4:5: 4:12
- }
-
Original file line number Diff line number Diff line change
@@ -14,7 +14,6 @@
StorageLive(_2); // scope 0 at $DIR/remove_unneeded_drops.rs:14:5: 14:12
StorageLive(_3); // scope 0 at $DIR/remove_unneeded_drops.rs:14:10: 14:11
_3 = _1; // scope 0 at $DIR/remove_unneeded_drops.rs:14:10: 14:11
_2 = const (); // scope 1 at $DIR/remove_unneeded_drops.rs:14:5: 14:12
- drop(_3) -> bb1; // scope 1 at $DIR/remove_unneeded_drops.rs:14:5: 14:12
- }
-
2 changes: 1 addition & 1 deletion src/test/mir-opt/simplify_locals.c.SimplifyLocals.diff
Original file line number Diff line number Diff line change
@@ -20,7 +20,7 @@
- _3 = &_1; // scope 1 at $DIR/simplify-locals.rs:16:20: 16:26
- _2 = move _3 as &[u8] (Pointer(Unsize)); // scope 1 at $DIR/simplify-locals.rs:16:20: 16:26
- StorageDead(_2); // scope 1 at $DIR/simplify-locals.rs:16:26: 16:27
_0 = const (); // scope 0 at $DIR/simplify-locals.rs:13:8: 17:2
- _0 = const (); // scope 0 at $DIR/simplify-locals.rs:13:8: 17:2
StorageDead(_1); // scope 0 at $DIR/simplify-locals.rs:17:1: 17:2
return; // scope 0 at $DIR/simplify-locals.rs:17:2: 17:2
}
2 changes: 1 addition & 1 deletion src/test/mir-opt/simplify_locals.d1.SimplifyLocals.diff
Original file line number Diff line number Diff line change
@@ -11,7 +11,7 @@
- StorageLive(_1); // scope 0 at $DIR/simplify-locals.rs:22:13: 22:17
- discriminant(_1) = 0; // scope 0 at $DIR/simplify-locals.rs:22:13: 22:17
- StorageDead(_1); // scope 0 at $DIR/simplify-locals.rs:22:17: 22:18
_0 = const (); // scope 0 at $DIR/simplify-locals.rs:20:9: 23:2
- _0 = const (); // scope 0 at $DIR/simplify-locals.rs:20:9: 23:2
return; // scope 0 at $DIR/simplify-locals.rs:23:2: 23:2
}
}
2 changes: 1 addition & 1 deletion src/test/mir-opt/simplify_locals.d2.SimplifyLocals.diff
Original file line number Diff line number Diff line change
@@ -31,7 +31,7 @@
- // + literal: Const { ty: E, val: Value(Scalar(0x01)) }
- StorageDead(_1); // scope 0 at $DIR/simplify-locals.rs:28:25: 28:26
- StorageDead(_2); // scope 0 at $DIR/simplify-locals.rs:28:26: 28:27
_0 = const (); // scope 0 at $DIR/simplify-locals.rs:26:9: 29:2
- _0 = const (); // scope 0 at $DIR/simplify-locals.rs:26:9: 29:2
return; // scope 0 at $DIR/simplify-locals.rs:29:2: 29:2
}
}
Loading