Add new MutatatingUseContexts for deinit and SetDiscriminant

This commit is contained in:
Jakob Degen 2022-04-11 06:04:53 -04:00
parent f7ca97a209
commit 48b01a0d0e
9 changed files with 57 additions and 41 deletions

View file

@ -72,5 +72,9 @@ pub fn categorize(context: PlaceContext) -> Option<DefUse> {
// Debug info is neither def nor use.
PlaceContext::NonUse(NonUseContext::VarDebugInfo) => None,
PlaceContext::MutatingUse(MutatingUseContext::Deinit | MutatingUseContext::SetDiscriminant) => {
bug!("These statements are not allowed in this MIR phase")
}
}
}

View file

@ -211,6 +211,8 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx>
PlaceContext::MutatingUse(
MutatingUseContext::Store
| MutatingUseContext::Deinit
| MutatingUseContext::SetDiscriminant
| MutatingUseContext::AsmOutput
| MutatingUseContext::Borrow
| MutatingUseContext::AddressOf

View file

@ -395,14 +395,14 @@ macro_rules! make_mir_visitor {
StatementKind::SetDiscriminant { place, .. } => {
self.visit_place(
place,
PlaceContext::MutatingUse(MutatingUseContext::Store),
PlaceContext::MutatingUse(MutatingUseContext::SetDiscriminant),
location
);
}
StatementKind::Deinit(place) => {
self.visit_place(
place,
PlaceContext::MutatingUse(MutatingUseContext::Store),
PlaceContext::MutatingUse(MutatingUseContext::Deinit),
location
)
}
@ -1181,6 +1181,10 @@ pub enum NonMutatingUseContext {
pub enum MutatingUseContext {
/// Appears as LHS of an assignment.
Store,
/// Appears on `SetDiscriminant`
SetDiscriminant,
/// Appears on `Deinit`
Deinit,
/// Output operand of an inline assembly block.
AsmOutput,
/// Destination of a call.

View file

@ -77,6 +77,10 @@ impl<T> Visitor<'_> for TransferFunction<'_, T>
where
T: GenKill<Local>,
{
// FIXME: Using `visit_local` here is a bug. For example, on `move _5.field` we mark `_5` as
// deinitialized, although clearly it is only partially deinitialized. This analysis is not
// actually used anywhere at the moment, so this is not critical, but this does need to be fixed
// before it starts being used again.
fn visit_local(&mut self, &local: &Local, context: PlaceContext, _: Location) {
use rustc_middle::mir::visit::{MutatingUseContext, NonMutatingUseContext, NonUseContext};
match context {
@ -87,6 +91,9 @@ where
| MutatingUseContext::Yield,
) => {}
// If it's deinitialized, it's no longer init
PlaceContext::MutatingUse(MutatingUseContext::Deinit) => self.trans.kill(local),
// Otherwise, when a place is mutated, we must consider it possibly initialized.
PlaceContext::MutatingUse(_) => self.trans.gen(local),

View file

@ -18,30 +18,6 @@ use crate::{AnalysisDomain, Backward, CallReturnPlaces, GenKill, GenKillAnalysis
/// such an assignment is currently marked as a "use" of `x` in an attempt to be maximally
/// conservative.
///
/// ## Enums and `SetDiscriminant`
///
/// Assigning a literal value to an `enum` (e.g. `Option<i32>`), does not result in a simple
/// assignment of the form `_1 = /*...*/` in the MIR. For example, the following assignment to `x`:
///
/// ```
/// x = Some(4);
/// ```
///
/// compiles to this MIR
///
/// ```
/// ((_1 as Some).0: i32) = const 4_i32;
/// discriminant(_1) = 1;
/// ```
///
/// However, `MaybeLiveLocals` **does** mark `x` (`_1`) as "killed" after a statement like this.
/// That's because it treats the `SetDiscriminant` operation as a definition of `x`, even though
/// the writes that actually initialized the locals happened earlier.
///
/// This makes `MaybeLiveLocals` unsuitable for certain classes of optimization normally associated
/// with a live variables analysis, notably dead-store elimination. It's a dirty hack, but it works
/// okay for the generator state transform (currently the main consumer of this analysis).
///
/// [`MaybeBorrowedLocals`]: super::MaybeBorrowedLocals
/// [flow-test]: https://github.com/rust-lang/rust/blob/a08c47310c7d49cbdc5d7afb38408ba519967ecd/src/test/ui/mir-dataflow/liveness-ptr.rs
/// [liveness]: https://en.wikipedia.org/wiki/Live_variable_analysis
@ -161,7 +137,13 @@ impl DefUse {
match context {
PlaceContext::NonUse(_) => None,
PlaceContext::MutatingUse(MutatingUseContext::Store) => Some(DefUse::Def),
PlaceContext::MutatingUse(MutatingUseContext::Store | MutatingUseContext::Deinit) => {
Some(DefUse::Def)
}
// Setting the discriminant is not a use because it does no reading, but it is also not
// a def because it does not overwrite the whole place
PlaceContext::MutatingUse(MutatingUseContext::SetDiscriminant) => None,
// `MutatingUseContext::Call` and `MutatingUseContext::Yield` indicate that this is the
// destination place for a `Call` return or `Yield` resume respectively. Since this is

View file

@ -897,8 +897,10 @@ impl Visitor<'_> for CanConstProp {
// mutations of the same local via `Store`
| MutatingUse(MutatingUseContext::Call)
| MutatingUse(MutatingUseContext::AsmOutput)
| MutatingUse(MutatingUseContext::Deinit)
// Actual store that can possibly even propagate a value
| MutatingUse(MutatingUseContext::Store) => {
| MutatingUse(MutatingUseContext::Store)
| MutatingUse(MutatingUseContext::SetDiscriminant) => {
if !self.found_assignment.insert(local) {
match &mut self.can_const_prop[local] {
// If the local can only get propagated in its own block, then we don't have

View file

@ -778,7 +778,9 @@ impl Visitor<'_> for CanConstProp {
// mutations of the same local via `Store`
| MutatingUse(MutatingUseContext::Call)
| MutatingUse(MutatingUseContext::AsmOutput)
| MutatingUse(MutatingUseContext::Deinit)
// Actual store that can possibly even propagate a value
| MutatingUse(MutatingUseContext::SetDiscriminant)
| MutatingUse(MutatingUseContext::Store) => {
if !self.found_assignment.insert(local) {
match &mut self.can_const_prop[local] {

View file

@ -17,9 +17,12 @@
}
bb0: {
StorageLive(_1); // scope 0 at $DIR/union.rs:13:9: 13:11
StorageLive(_2); // scope 0 at $DIR/union.rs:13:23: 13:28
_2 = val() -> bb1; // scope 0 at $DIR/union.rs:13:23: 13:28
- StorageLive(_1); // scope 0 at $DIR/union.rs:13:9: 13:11
- StorageLive(_2); // scope 0 at $DIR/union.rs:13:23: 13:28
- _2 = val() -> bb1; // scope 0 at $DIR/union.rs:13:23: 13:28
+ nop; // scope 0 at $DIR/union.rs:13:9: 13:11
+ nop; // scope 0 at $DIR/union.rs:13:23: 13:28
+ (_1.0: u32) = val() -> bb1; // scope 0 at $DIR/union.rs:13:23: 13:28
// mir::Constant
// + span: $DIR/union.rs:13:23: 13:26
// + literal: Const { ty: fn() -> u32 {val}, val: Value(Scalar(<ZST>)) }
@ -27,14 +30,17 @@
bb1: {
Deinit(_1); // scope 0 at $DIR/union.rs:13:14: 13:30
(_1.0: u32) = move _2; // scope 0 at $DIR/union.rs:13:14: 13:30
StorageDead(_2); // scope 0 at $DIR/union.rs:13:29: 13:30
- (_1.0: u32) = move _2; // scope 0 at $DIR/union.rs:13:14: 13:30
- StorageDead(_2); // scope 0 at $DIR/union.rs:13:29: 13:30
+ nop; // scope 0 at $DIR/union.rs:13:14: 13:30
+ nop; // scope 0 at $DIR/union.rs:13:29: 13:30
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
StorageDead(_4); // scope 1 at $DIR/union.rs:15:26: 15:27
StorageDead(_3); // scope 1 at $DIR/union.rs:15:27: 15:28
StorageDead(_1); // scope 0 at $DIR/union.rs:16:1: 16:2
- StorageDead(_1); // scope 0 at $DIR/union.rs:16:1: 16:2
+ nop; // scope 0 at $DIR/union.rs:16:1: 16:2
return; // scope 0 at $DIR/union.rs:16:2: 16:2
}
}

View file

@ -65,17 +65,22 @@
bb0: {
- StorageLive(_3); // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:8: 27:6
- StorageLive(_4); // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:14: 21:24
- StorageLive(_5); // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:15: 21:16
- _5 = _1; // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:15: 21:16
+ nop; // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:8: 27:6
StorageLive(_4); // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:14: 21:24
StorageLive(_5); // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:15: 21:16
_5 = _1; // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:15: 21:16
+ nop; // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:14: 21:24
+ nop; // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:15: 21:16
+ (_4.0: &ViewportPercentageLength) = _1; // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:15: 21:16
StorageLive(_6); // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:18: 21:23
_6 = _2; // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:18: 21:23
Deinit(_4); // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:14: 21:24
(_4.0: &ViewportPercentageLength) = move _5; // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:14: 21:24
- (_4.0: &ViewportPercentageLength) = move _5; // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:14: 21:24
+ nop; // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:14: 21:24
(_4.1: &ViewportPercentageLength) = move _6; // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:14: 21:24
StorageDead(_6); // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:23: 21:24
StorageDead(_5); // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:23: 21:24
- StorageDead(_5); // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:23: 21:24
+ nop; // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:23: 21:24
_11 = discriminant((*(_4.0: &ViewportPercentageLength))); // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:14: 21:24
- switchInt(move _11) -> [0_isize: bb1, 1_isize: bb3, 2_isize: bb4, 3_isize: bb5, otherwise: bb11]; // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:8: 21:24
+ StorageLive(_34); // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:8: 21:24
@ -100,8 +105,9 @@
discriminant(_0) = 1; // scope 0 at $DIR/early_otherwise_branch_68867.rs:26:21: 26:28
StorageDead(_33); // scope 0 at $DIR/early_otherwise_branch_68867.rs:26:27: 26:28
- StorageDead(_3); // scope 0 at $DIR/early_otherwise_branch_68867.rs:27:6: 27:7
- StorageDead(_4); // scope 0 at $DIR/early_otherwise_branch_68867.rs:28:1: 28:2
+ nop; // scope 0 at $DIR/early_otherwise_branch_68867.rs:27:6: 27:7
StorageDead(_4); // scope 0 at $DIR/early_otherwise_branch_68867.rs:28:1: 28:2
+ nop; // scope 0 at $DIR/early_otherwise_branch_68867.rs:28:1: 28:2
return; // scope 0 at $DIR/early_otherwise_branch_68867.rs:28:2: 28:2
}
@ -293,8 +299,9 @@
+ nop; // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:5: 27:7
discriminant(_0) = 0; // scope 0 at $DIR/early_otherwise_branch_68867.rs:21:5: 27:7
- StorageDead(_3); // scope 0 at $DIR/early_otherwise_branch_68867.rs:27:6: 27:7
- StorageDead(_4); // scope 0 at $DIR/early_otherwise_branch_68867.rs:28:1: 28:2
+ nop; // scope 0 at $DIR/early_otherwise_branch_68867.rs:27:6: 27:7
StorageDead(_4); // scope 0 at $DIR/early_otherwise_branch_68867.rs:28:1: 28:2
+ nop; // scope 0 at $DIR/early_otherwise_branch_68867.rs:28:1: 28:2
return; // scope 0 at $DIR/early_otherwise_branch_68867.rs:28:2: 28:2
}