diff --git a/compiler/rustc_borrowck/src/def_use.rs b/compiler/rustc_borrowck/src/def_use.rs index eec994f88b96..a5c0d77429de 100644 --- a/compiler/rustc_borrowck/src/def_use.rs +++ b/compiler/rustc_borrowck/src/def_use.rs @@ -72,5 +72,9 @@ pub fn categorize(context: PlaceContext) -> Option { // 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") + } } } diff --git a/compiler/rustc_codegen_ssa/src/mir/analyze.rs b/compiler/rustc_codegen_ssa/src/mir/analyze.rs index d768d4920c5b..efb424af3ed9 100644 --- a/compiler/rustc_codegen_ssa/src/mir/analyze.rs +++ b/compiler/rustc_codegen_ssa/src/mir/analyze.rs @@ -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 diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index 49a789072f95..45b1ad6df822 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -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. diff --git a/compiler/rustc_mir_dataflow/src/impls/init_locals.rs b/compiler/rustc_mir_dataflow/src/impls/init_locals.rs index b355871d64f6..584ab9718ed6 100644 --- a/compiler/rustc_mir_dataflow/src/impls/init_locals.rs +++ b/compiler/rustc_mir_dataflow/src/impls/init_locals.rs @@ -77,6 +77,10 @@ impl Visitor<'_> for TransferFunction<'_, T> where T: GenKill, { + // 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), diff --git a/compiler/rustc_mir_dataflow/src/impls/liveness.rs b/compiler/rustc_mir_dataflow/src/impls/liveness.rs index 602ccec76a68..5a788c153a47 100644 --- a/compiler/rustc_mir_dataflow/src/impls/liveness.rs +++ b/compiler/rustc_mir_dataflow/src/impls/liveness.rs @@ -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`), 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 diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs index aa47630a26ac..13b49256d488 100644 --- a/compiler/rustc_mir_transform/src/const_prop.rs +++ b/compiler/rustc_mir_transform/src/const_prop.rs @@ -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 diff --git a/compiler/rustc_mir_transform/src/const_prop_lint.rs b/compiler/rustc_mir_transform/src/const_prop_lint.rs index 50400cdeac97..d6331a88c5b7 100644 --- a/compiler/rustc_mir_transform/src/const_prop_lint.rs +++ b/compiler/rustc_mir_transform/src/const_prop_lint.rs @@ -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] { diff --git a/src/test/mir-opt/dest-prop/union.main.DestinationPropagation.diff b/src/test/mir-opt/dest-prop/union.main.DestinationPropagation.diff index fcf5ef911606..9330e68b1aa8 100644 --- a/src/test/mir-opt/dest-prop/union.main.DestinationPropagation.diff +++ b/src/test/mir-opt/dest-prop/union.main.DestinationPropagation.diff @@ -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()) } @@ -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 } } diff --git a/src/test/mir-opt/early_otherwise_branch_68867.try_sum.EarlyOtherwiseBranch.before-SimplifyConstCondition-final.after.diff b/src/test/mir-opt/early_otherwise_branch_68867.try_sum.EarlyOtherwiseBranch.before-SimplifyConstCondition-final.after.diff index 8396d071ded1..592388e69a91 100644 --- a/src/test/mir-opt/early_otherwise_branch_68867.try_sum.EarlyOtherwiseBranch.before-SimplifyConstCondition-final.after.diff +++ b/src/test/mir-opt/early_otherwise_branch_68867.try_sum.EarlyOtherwiseBranch.before-SimplifyConstCondition-final.after.diff @@ -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 }