From 77dac91d4f6f7200032f9219a92aad240b000a63 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Tue, 25 Apr 2023 20:09:41 +0000 Subject: [PATCH 1/3] Add test. --- ...ace_mention.main.DeadStoreElimination.diff | 25 +++++++++++++++++++ .../dead-store-elimination/place_mention.rs | 9 +++++++ 2 files changed, 34 insertions(+) create mode 100644 tests/mir-opt/dead-store-elimination/place_mention.main.DeadStoreElimination.diff create mode 100644 tests/mir-opt/dead-store-elimination/place_mention.rs diff --git a/tests/mir-opt/dead-store-elimination/place_mention.main.DeadStoreElimination.diff b/tests/mir-opt/dead-store-elimination/place_mention.main.DeadStoreElimination.diff new file mode 100644 index 0000000000000..73aadf3b231cb --- /dev/null +++ b/tests/mir-opt/dead-store-elimination/place_mention.main.DeadStoreElimination.diff @@ -0,0 +1,25 @@ +- // MIR for `main` before DeadStoreElimination ++ // MIR for `main` after DeadStoreElimination + + fn main() -> () { + let mut _0: (); // return place in scope 0 at $DIR/place_mention.rs:+0:11: +0:11 + let mut _1: (&str, &str); // in scope 0 at $DIR/place_mention.rs:+3:18: +3:36 + scope 1 { + } + + bb0: { + StorageLive(_1); // scope 0 at $DIR/place_mention.rs:+3:18: +3:36 +- _1 = (const "Hello", const "World"); // scope 0 at $DIR/place_mention.rs:+3:18: +3:36 +- // mir::Constant +- // + span: $DIR/place_mention.rs:8:19: 8:26 +- // + literal: Const { ty: &str, val: Value(Slice(..)) } +- // mir::Constant +- // + span: $DIR/place_mention.rs:8:28: 8:35 +- // + literal: Const { ty: &str, val: Value(Slice(..)) } + PlaceMention(_1); // scope 0 at $DIR/place_mention.rs:+3:18: +3:36 + StorageDead(_1); // scope 0 at $DIR/place_mention.rs:+3:36: +3:37 + _0 = const (); // scope 0 at $DIR/place_mention.rs:+0:11: +4:2 + return; // scope 0 at $DIR/place_mention.rs:+4:2: +4:2 + } + } + diff --git a/tests/mir-opt/dead-store-elimination/place_mention.rs b/tests/mir-opt/dead-store-elimination/place_mention.rs new file mode 100644 index 0000000000000..59dc74454a402 --- /dev/null +++ b/tests/mir-opt/dead-store-elimination/place_mention.rs @@ -0,0 +1,9 @@ +// unit-test: DeadStoreElimination +// compile-flags: -Zmir-keep-place-mention + +// EMIT_MIR place_mention.main.DeadStoreElimination.diff +fn main() { + // Verify that we account for the `PlaceMention` statement as a use of the tuple, + // and don't remove it as a dead store. + let (_, _) = ("Hello", "World"); +} From 9325a254f0455e375846a2e4aa6bcdf848527d2a Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Tue, 25 Apr 2023 20:09:54 +0000 Subject: [PATCH 2/3] Make PlaceMention a non-mutating use. --- compiler/rustc_borrowck/src/def_use.rs | 2 +- compiler/rustc_borrowck/src/type_check/mod.rs | 8 +++----- compiler/rustc_codegen_ssa/src/mir/analyze.rs | 4 +++- compiler/rustc_middle/src/mir/visit.rs | 6 +++--- compiler/rustc_mir_dataflow/src/impls/liveness.rs | 1 + compiler/rustc_mir_transform/src/const_prop.rs | 1 + .../place_mention.main.DeadStoreElimination.diff | 14 +++++++------- 7 files changed, 19 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_borrowck/src/def_use.rs b/compiler/rustc_borrowck/src/def_use.rs index 6259722b6940f..74e6ce37e971a 100644 --- a/compiler/rustc_borrowck/src/def_use.rs +++ b/compiler/rustc_borrowck/src/def_use.rs @@ -54,7 +54,7 @@ pub fn categorize(context: PlaceContext) -> Option { // `PlaceMention` and `AscribeUserType` both evaluate the place, which must not // contain dangling references. - PlaceContext::NonUse(NonUseContext::PlaceMention) | + PlaceContext::NonMutatingUse(NonMutatingUseContext::PlaceMention) | PlaceContext::NonUse(NonUseContext::AscribeUserTy) | PlaceContext::MutatingUse(MutatingUseContext::AddressOf) | diff --git a/compiler/rustc_borrowck/src/type_check/mod.rs b/compiler/rustc_borrowck/src/type_check/mod.rs index d5e50a61b032c..729bde4d25cab 100644 --- a/compiler/rustc_borrowck/src/type_check/mod.rs +++ b/compiler/rustc_borrowck/src/type_check/mod.rs @@ -771,12 +771,10 @@ impl<'a, 'b, 'tcx> TypeVerifier<'a, 'b, 'tcx> { match context { PlaceContext::MutatingUse(_) => ty::Invariant, - PlaceContext::NonUse(StorageDead | StorageLive | PlaceMention | VarDebugInfo) => { - ty::Invariant - } + PlaceContext::NonUse(StorageDead | StorageLive | VarDebugInfo) => ty::Invariant, PlaceContext::NonMutatingUse( - Inspect | Copy | Move | SharedBorrow | ShallowBorrow | UniqueBorrow | AddressOf - | Projection, + Inspect | Copy | Move | PlaceMention | SharedBorrow | ShallowBorrow | UniqueBorrow + | AddressOf | Projection, ) => ty::Covariant, PlaceContext::NonUse(AscribeUserTy) => ty::Covariant, } diff --git a/compiler/rustc_codegen_ssa/src/mir/analyze.rs b/compiler/rustc_codegen_ssa/src/mir/analyze.rs index 0334c7ff13264..569599faa362b 100644 --- a/compiler/rustc_codegen_ssa/src/mir/analyze.rs +++ b/compiler/rustc_codegen_ssa/src/mir/analyze.rs @@ -203,7 +203,9 @@ impl<'mir, 'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> Visitor<'tcx> self.assign(local, DefLocation::Body(location)); } - PlaceContext::NonUse(_) | PlaceContext::MutatingUse(MutatingUseContext::Retag) => {} + PlaceContext::NonUse(_) + | PlaceContext::NonMutatingUse(NonMutatingUseContext::PlaceMention) + | PlaceContext::MutatingUse(MutatingUseContext::Retag) => {} PlaceContext::NonMutatingUse( NonMutatingUseContext::Copy | NonMutatingUseContext::Move, diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index 0a9fcd898b93e..eb5bcbb67ebbf 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -410,7 +410,7 @@ macro_rules! make_mir_visitor { StatementKind::PlaceMention(place) => { self.visit_place( place, - PlaceContext::NonUse(NonUseContext::PlaceMention), + PlaceContext::NonMutatingUse(NonMutatingUseContext::PlaceMention), location ); } @@ -1251,6 +1251,8 @@ pub enum NonMutatingUseContext { UniqueBorrow, /// AddressOf for *const pointer. AddressOf, + /// PlaceMention statement. + PlaceMention, /// Used as base for another place, e.g., `x` in `x.y`. Will not mutate the place. /// For example, the projection `x.y` is not marked as a mutation in these cases: /// ```ignore (illustrative) @@ -1301,8 +1303,6 @@ pub enum NonUseContext { AscribeUserTy, /// The data of a user variable, for debug info. VarDebugInfo, - /// PlaceMention statement. - PlaceMention, } #[derive(Copy, Clone, Debug, PartialEq, Eq)] diff --git a/compiler/rustc_mir_dataflow/src/impls/liveness.rs b/compiler/rustc_mir_dataflow/src/impls/liveness.rs index bc67aa476f1af..aeca0073304ea 100644 --- a/compiler/rustc_mir_dataflow/src/impls/liveness.rs +++ b/compiler/rustc_mir_dataflow/src/impls/liveness.rs @@ -197,6 +197,7 @@ impl DefUse { | NonMutatingUseContext::Copy | NonMutatingUseContext::Inspect | NonMutatingUseContext::Move + | NonMutatingUseContext::PlaceMention | NonMutatingUseContext::ShallowBorrow | NonMutatingUseContext::SharedBorrow | NonMutatingUseContext::UniqueBorrow, diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs index 880745b8f0eb7..5522591d1742d 100644 --- a/compiler/rustc_mir_transform/src/const_prop.rs +++ b/compiler/rustc_mir_transform/src/const_prop.rs @@ -774,6 +774,7 @@ impl Visitor<'_> for CanConstProp { | NonMutatingUse(NonMutatingUseContext::Move) | NonMutatingUse(NonMutatingUseContext::Inspect) | NonMutatingUse(NonMutatingUseContext::Projection) + | NonMutatingUse(NonMutatingUseContext::PlaceMention) | NonUse(_) => {} // These could be propagated with a smarter analysis or just some careful thinking about diff --git a/tests/mir-opt/dead-store-elimination/place_mention.main.DeadStoreElimination.diff b/tests/mir-opt/dead-store-elimination/place_mention.main.DeadStoreElimination.diff index 73aadf3b231cb..761c074ed9450 100644 --- a/tests/mir-opt/dead-store-elimination/place_mention.main.DeadStoreElimination.diff +++ b/tests/mir-opt/dead-store-elimination/place_mention.main.DeadStoreElimination.diff @@ -9,13 +9,13 @@ bb0: { StorageLive(_1); // scope 0 at $DIR/place_mention.rs:+3:18: +3:36 -- _1 = (const "Hello", const "World"); // scope 0 at $DIR/place_mention.rs:+3:18: +3:36 -- // mir::Constant -- // + span: $DIR/place_mention.rs:8:19: 8:26 -- // + literal: Const { ty: &str, val: Value(Slice(..)) } -- // mir::Constant -- // + span: $DIR/place_mention.rs:8:28: 8:35 -- // + literal: Const { ty: &str, val: Value(Slice(..)) } + _1 = (const "Hello", const "World"); // scope 0 at $DIR/place_mention.rs:+3:18: +3:36 + // mir::Constant + // + span: $DIR/place_mention.rs:8:19: 8:26 + // + literal: Const { ty: &str, val: Value(Slice(..)) } + // mir::Constant + // + span: $DIR/place_mention.rs:8:28: 8:35 + // + literal: Const { ty: &str, val: Value(Slice(..)) } PlaceMention(_1); // scope 0 at $DIR/place_mention.rs:+3:18: +3:36 StorageDead(_1); // scope 0 at $DIR/place_mention.rs:+3:36: +3:37 _0 = const (); // scope 0 at $DIR/place_mention.rs:+0:11: +4:2 From 4ec76df4a9edc6c7a3a69a82bd3eab3881b70ffa Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sat, 29 Apr 2023 16:16:41 +0000 Subject: [PATCH 3/3] Expand comment on NonMutatingUseContext. --- compiler/rustc_middle/src/mir/visit.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/compiler/rustc_middle/src/mir/visit.rs b/compiler/rustc_middle/src/mir/visit.rs index eb5bcbb67ebbf..8e8ca7874f586 100644 --- a/compiler/rustc_middle/src/mir/visit.rs +++ b/compiler/rustc_middle/src/mir/visit.rs @@ -1252,6 +1252,9 @@ pub enum NonMutatingUseContext { /// AddressOf for *const pointer. AddressOf, /// PlaceMention statement. + /// + /// This statement is executed as a check that the `Place` is live without reading from it, + /// so it must be considered as a non-mutating use. PlaceMention, /// Used as base for another place, e.g., `x` in `x.y`. Will not mutate the place. /// For example, the projection `x.y` is not marked as a mutation in these cases: