From 3c43b61b870add2daddbd8e480477e5a8aa409c2 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Wed, 26 Apr 2023 18:30:12 +0000 Subject: [PATCH] Do not consider borrowed Freeze locals as SSA. --- compiler/rustc_mir_transform/src/copy_prop.rs | 3 +- .../src/normalize_array_len.rs | 5 +- compiler/rustc_mir_transform/src/ref_prop.rs | 6 +-- compiler/rustc_mir_transform/src/ssa.rs | 49 ++++++++++++------- .../copy-prop/borrowed_local.f.CopyProp.diff | 3 +- ...ence_propagation.ReferencePropagation.diff | 3 +- ...gation_const_ptr.ReferencePropagation.diff | 3 +- ...filter.variant_a-{closure#0}.CopyProp.diff | 16 +++--- ..._a-{closure#0}.DestinationPropagation.diff | 16 ++++++ ...nt_a-{closure#0}.ReferencePropagation.diff | 8 +++ 10 files changed, 69 insertions(+), 43 deletions(-) diff --git a/compiler/rustc_mir_transform/src/copy_prop.rs b/compiler/rustc_mir_transform/src/copy_prop.rs index 2fa5c0bca15bf..c565d6f13b17f 100644 --- a/compiler/rustc_mir_transform/src/copy_prop.rs +++ b/compiler/rustc_mir_transform/src/copy_prop.rs @@ -33,9 +33,8 @@ impl<'tcx> MirPass<'tcx> for CopyProp { } fn propagate_ssa<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { - let param_env = tcx.param_env_reveal_all_normalized(body.source.def_id()); let borrowed_locals = borrowed_locals(body); - let ssa = SsaLocals::new(tcx, param_env, body, &borrowed_locals); + let ssa = SsaLocals::new(body); let fully_moved = fully_moved_locals(&ssa, body); debug!(?fully_moved); diff --git a/compiler/rustc_mir_transform/src/normalize_array_len.rs b/compiler/rustc_mir_transform/src/normalize_array_len.rs index 109a2c0aec6f2..3d61d33ce3536 100644 --- a/compiler/rustc_mir_transform/src/normalize_array_len.rs +++ b/compiler/rustc_mir_transform/src/normalize_array_len.rs @@ -7,7 +7,6 @@ use rustc_index::IndexVec; use rustc_middle::mir::visit::*; use rustc_middle::mir::*; use rustc_middle::ty::{self, TyCtxt}; -use rustc_mir_dataflow::impls::borrowed_locals; pub struct NormalizeArrayLen; @@ -24,9 +23,7 @@ impl<'tcx> MirPass<'tcx> for NormalizeArrayLen { } fn normalize_array_len_calls<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { - let param_env = tcx.param_env_reveal_all_normalized(body.source.def_id()); - let borrowed_locals = borrowed_locals(body); - let ssa = SsaLocals::new(tcx, param_env, body, &borrowed_locals); + let ssa = SsaLocals::new(body); let slice_lengths = compute_slice_length(tcx, &ssa, body); debug!(?slice_lengths); diff --git a/compiler/rustc_mir_transform/src/ref_prop.rs b/compiler/rustc_mir_transform/src/ref_prop.rs index dfdf4caf88111..a2e7651007337 100644 --- a/compiler/rustc_mir_transform/src/ref_prop.rs +++ b/compiler/rustc_mir_transform/src/ref_prop.rs @@ -4,7 +4,7 @@ use rustc_index::IndexVec; use rustc_middle::mir::visit::*; use rustc_middle::mir::*; use rustc_middle::ty::TyCtxt; -use rustc_mir_dataflow::impls::{borrowed_locals, MaybeStorageDead}; +use rustc_mir_dataflow::impls::MaybeStorageDead; use rustc_mir_dataflow::storage::always_storage_live_locals; use rustc_mir_dataflow::Analysis; @@ -82,9 +82,7 @@ impl<'tcx> MirPass<'tcx> for ReferencePropagation { } fn propagate_ssa<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { - let param_env = tcx.param_env_reveal_all_normalized(body.source.def_id()); - let borrowed_locals = borrowed_locals(body); - let ssa = SsaLocals::new(tcx, param_env, body, &borrowed_locals); + let ssa = SsaLocals::new(body); let mut replacer = compute_replacement(tcx, body, &ssa); debug!(?replacer.targets, ?replacer.allowed_replacements, ?replacer.storage_to_remove); diff --git a/compiler/rustc_mir_transform/src/ssa.rs b/compiler/rustc_mir_transform/src/ssa.rs index 270bb540b80d9..d7fc6e2f6c302 100644 --- a/compiler/rustc_mir_transform/src/ssa.rs +++ b/compiler/rustc_mir_transform/src/ssa.rs @@ -1,3 +1,16 @@ +//! We denote as "SSA" the set of locals that verify the following properties: +//! 1/ They are only assigned-to once, either as a function parameter, or in an assign statement; +//! 2/ This single assignment dominates all uses; +//! +//! As a consequence of rule 2, we consider that borrowed locals are not SSA, even if they are +//! `Freeze`, as we do not track that the assignment dominates all uses of the borrow. +//! +//! We say a local has a stable address if its address has SSA-like properties: +//! 1/ It has a single `StorageLive` statement, or none at all (always-live); +//! 2/ All its uses dominate this `StorageLive` statement. +//! +//! We do not discard borrowed locals from this analysis, as we cannot take their address' address. + use either::Either; use rustc_data_structures::graph::dominators::Dominators; use rustc_index::bit_set::BitSet; @@ -5,7 +18,6 @@ use rustc_index::{IndexSlice, IndexVec}; use rustc_middle::middle::resolve_bound_vars::Set1; use rustc_middle::mir::visit::*; use rustc_middle::mir::*; -use rustc_middle::ty::{ParamEnv, TyCtxt}; use rustc_mir_dataflow::storage::always_storage_live_locals; #[derive(Debug)] @@ -62,12 +74,7 @@ impl SmallDominators { } impl SsaLocals { - pub fn new<'tcx>( - tcx: TyCtxt<'tcx>, - param_env: ParamEnv<'tcx>, - body: &Body<'tcx>, - borrowed_locals: &BitSet, - ) -> SsaLocals { + pub fn new<'tcx>(body: &Body<'tcx>) -> SsaLocals { let assignment_order = Vec::with_capacity(body.local_decls.len()); let assignments = IndexVec::from_elem(Set1::Empty, &body.local_decls); @@ -80,13 +87,8 @@ impl SsaLocals { let mut visitor = SsaVisitor { assignments, assignment_order, dominators, direct_uses, storage_live }; - for (local, decl) in body.local_decls.iter_enumerated() { - if matches!(body.local_kind(local), LocalKind::Arg) { - visitor.assignments[local] = Set1::One(LocationExtended::Arg); - } - if borrowed_locals.contains(local) && !decl.ty.is_freeze(tcx, param_env) { - visitor.assignments[local] = Set1::Many; - } + for local in body.args_iter() { + visitor.assignments[local] = Set1::One(LocationExtended::Arg); } for local in always_storage_live_locals(body).iter() { @@ -237,6 +239,8 @@ struct SsaVisitor { impl<'tcx> Visitor<'tcx> for SsaVisitor { fn visit_local(&mut self, local: Local, ctxt: PlaceContext, loc: Location) { match ctxt { + PlaceContext::MutatingUse(MutatingUseContext::Projection) + | PlaceContext::NonMutatingUse(NonMutatingUseContext::Projection) => bug!(), PlaceContext::MutatingUse(MutatingUseContext::Store) => { self.assignments[local].insert(LocationExtended::Plain(loc)); if let Set1::One(_) = self.assignments[local] { @@ -246,13 +250,18 @@ impl<'tcx> Visitor<'tcx> for SsaVisitor { self.dominators.check_dominates(&mut self.storage_live[local], loc); } // Anything can happen with raw pointers, so remove them. - PlaceContext::NonMutatingUse(NonMutatingUseContext::AddressOf) + // We do not verify that all uses of the borrow dominate the assignment to `local`, + // so we have to remove them too. + PlaceContext::NonMutatingUse( + NonMutatingUseContext::SharedBorrow + | NonMutatingUseContext::ShallowBorrow + | NonMutatingUseContext::UniqueBorrow + | NonMutatingUseContext::AddressOf, + ) | PlaceContext::MutatingUse(_) => { self.assignments[local] = Set1::Many; self.dominators.check_dominates(&mut self.storage_live[local], loc); } - // Immutable borrows are taken into account in `SsaLocals::new` by - // removing non-freeze locals. PlaceContext::NonMutatingUse(_) => { self.dominators.check_dominates(&mut self.assignments[local], loc); self.dominators.check_dominates(&mut self.storage_live[local], loc); @@ -270,15 +279,17 @@ impl<'tcx> Visitor<'tcx> for SsaVisitor { // Do not do anything for storage statements and debuginfo. if ctxt.is_use() { // Only change the context if it is a real use, not a "use" in debuginfo. - let new_ctxt = PlaceContext::NonMutatingUse(NonMutatingUseContext::Projection); + let new_ctxt = PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy); self.visit_projection(place.as_ref(), new_ctxt, loc); self.dominators.check_dominates(&mut self.assignments[place.local], loc); self.dominators.check_dominates(&mut self.storage_live[place.local], loc); } return; + } else { + self.visit_projection(place.as_ref(), ctxt, loc); + self.visit_local(place.local, ctxt, loc); } - self.super_place(place, ctxt, loc); } } diff --git a/tests/mir-opt/copy-prop/borrowed_local.f.CopyProp.diff b/tests/mir-opt/copy-prop/borrowed_local.f.CopyProp.diff index 2a0bff57db9cf..51707e71661c5 100644 --- a/tests/mir-opt/copy-prop/borrowed_local.f.CopyProp.diff +++ b/tests/mir-opt/copy-prop/borrowed_local.f.CopyProp.diff @@ -20,8 +20,7 @@ } bb1: { -- _0 = opaque::(_3) -> bb2; // scope 0 at $DIR/borrowed_local.rs:+12:13: +12:38 -+ _0 = opaque::(_1) -> bb2; // scope 0 at $DIR/borrowed_local.rs:+12:13: +12:38 + _0 = opaque::(_3) -> bb2; // scope 0 at $DIR/borrowed_local.rs:+12:13: +12:38 // mir::Constant // + span: $DIR/borrowed_local.rs:28:28: 28:34 // + literal: Const { ty: fn(u8) -> bool {opaque::}, val: Value() } diff --git a/tests/mir-opt/reference_prop.reference_propagation.ReferencePropagation.diff b/tests/mir-opt/reference_prop.reference_propagation.ReferencePropagation.diff index 12aea890e6382..9a9d5d652346a 100644 --- a/tests/mir-opt/reference_prop.reference_propagation.ReferencePropagation.diff +++ b/tests/mir-opt/reference_prop.reference_propagation.ReferencePropagation.diff @@ -178,9 +178,8 @@ StorageLive(_17); // scope 9 at $DIR/reference_prop.rs:+22:13: +22:14 _17 = &_16; // scope 9 at $DIR/reference_prop.rs:+22:17: +22:19 StorageLive(_18); // scope 10 at $DIR/reference_prop.rs:+23:13: +23:14 -- _18 = (*_16); // scope 10 at $DIR/reference_prop.rs:+23:17: +23:19 + _18 = (*_16); // scope 10 at $DIR/reference_prop.rs:+23:17: +23:19 - _14 = const (); // scope 0 at $DIR/reference_prop.rs:+19:5: +24:6 -+ _18 = _15; // scope 10 at $DIR/reference_prop.rs:+23:17: +23:19 StorageDead(_18); // scope 10 at $DIR/reference_prop.rs:+24:5: +24:6 StorageDead(_17); // scope 9 at $DIR/reference_prop.rs:+24:5: +24:6 StorageDead(_16); // scope 8 at $DIR/reference_prop.rs:+24:5: +24:6 diff --git a/tests/mir-opt/reference_prop.reference_propagation_const_ptr.ReferencePropagation.diff b/tests/mir-opt/reference_prop.reference_propagation_const_ptr.ReferencePropagation.diff index 2e6097af2c98a..8edc8104f8273 100644 --- a/tests/mir-opt/reference_prop.reference_propagation_const_ptr.ReferencePropagation.diff +++ b/tests/mir-opt/reference_prop.reference_propagation_const_ptr.ReferencePropagation.diff @@ -207,9 +207,8 @@ StorageLive(_16); // scope 12 at $DIR/reference_prop.rs:+22:13: +22:14 _16 = &_15; // scope 12 at $DIR/reference_prop.rs:+22:17: +22:19 StorageLive(_17); // scope 13 at $DIR/reference_prop.rs:+23:13: +23:14 -- _17 = (*_15); // scope 13 at $DIR/reference_prop.rs:+23:17: +23:19 + _17 = (*_15); // scope 13 at $DIR/reference_prop.rs:+23:17: +23:19 - _13 = const (); // scope 10 at $DIR/reference_prop.rs:+19:5: +24:6 -+ _17 = _14; // scope 13 at $DIR/reference_prop.rs:+23:17: +23:19 StorageDead(_17); // scope 13 at $DIR/reference_prop.rs:+24:5: +24:6 StorageDead(_16); // scope 12 at $DIR/reference_prop.rs:+24:5: +24:6 StorageDead(_15); // scope 11 at $DIR/reference_prop.rs:+24:5: +24:6 diff --git a/tests/mir-opt/slice_filter.variant_a-{closure#0}.CopyProp.diff b/tests/mir-opt/slice_filter.variant_a-{closure#0}.CopyProp.diff index 3bb0358ffe3e6..60e5056c7a926 100644 --- a/tests/mir-opt/slice_filter.variant_a-{closure#0}.CopyProp.diff +++ b/tests/mir-opt/slice_filter.variant_a-{closure#0}.CopyProp.diff @@ -101,16 +101,16 @@ } bb0: { -- StorageLive(_3); // scope 0 at $DIR/slice_filter.rs:+0:27: +0:28 + StorageLive(_3); // scope 0 at $DIR/slice_filter.rs:+0:27: +0:28 _25 = deref_copy (*_2); // scope 0 at $DIR/slice_filter.rs:+0:27: +0:28 _3 = &((*_25).0: usize); // scope 0 at $DIR/slice_filter.rs:+0:27: +0:28 -- StorageLive(_4); // scope 0 at $DIR/slice_filter.rs:+0:30: +0:31 + StorageLive(_4); // scope 0 at $DIR/slice_filter.rs:+0:30: +0:31 _26 = deref_copy (*_2); // scope 0 at $DIR/slice_filter.rs:+0:30: +0:31 _4 = &((*_26).1: usize); // scope 0 at $DIR/slice_filter.rs:+0:30: +0:31 -- StorageLive(_5); // scope 0 at $DIR/slice_filter.rs:+0:33: +0:34 + StorageLive(_5); // scope 0 at $DIR/slice_filter.rs:+0:33: +0:34 _27 = deref_copy (*_2); // scope 0 at $DIR/slice_filter.rs:+0:33: +0:34 _5 = &((*_27).2: usize); // scope 0 at $DIR/slice_filter.rs:+0:33: +0:34 -- StorageLive(_6); // scope 0 at $DIR/slice_filter.rs:+0:36: +0:37 + StorageLive(_6); // scope 0 at $DIR/slice_filter.rs:+0:36: +0:37 _28 = deref_copy (*_2); // scope 0 at $DIR/slice_filter.rs:+0:36: +0:37 _6 = &((*_28).3: usize); // scope 0 at $DIR/slice_filter.rs:+0:36: +0:37 StorageLive(_7); // scope 1 at $DIR/slice_filter.rs:+0:40: +0:56 @@ -184,10 +184,10 @@ bb3: { StorageDead(_16); // scope 1 at $DIR/slice_filter.rs:+0:75: +0:76 StorageDead(_7); // scope 1 at $DIR/slice_filter.rs:+0:75: +0:76 -- StorageDead(_6); // scope 0 at $DIR/slice_filter.rs:+0:75: +0:76 -- StorageDead(_5); // scope 0 at $DIR/slice_filter.rs:+0:75: +0:76 -- StorageDead(_4); // scope 0 at $DIR/slice_filter.rs:+0:75: +0:76 -- StorageDead(_3); // scope 0 at $DIR/slice_filter.rs:+0:75: +0:76 + StorageDead(_6); // scope 0 at $DIR/slice_filter.rs:+0:75: +0:76 + StorageDead(_5); // scope 0 at $DIR/slice_filter.rs:+0:75: +0:76 + StorageDead(_4); // scope 0 at $DIR/slice_filter.rs:+0:75: +0:76 + StorageDead(_3); // scope 0 at $DIR/slice_filter.rs:+0:75: +0:76 return; // scope 0 at $DIR/slice_filter.rs:+0:76: +0:76 } diff --git a/tests/mir-opt/slice_filter.variant_a-{closure#0}.DestinationPropagation.diff b/tests/mir-opt/slice_filter.variant_a-{closure#0}.DestinationPropagation.diff index a81553733a7db..7ad1ccf28a607 100644 --- a/tests/mir-opt/slice_filter.variant_a-{closure#0}.DestinationPropagation.diff +++ b/tests/mir-opt/slice_filter.variant_a-{closure#0}.DestinationPropagation.diff @@ -89,15 +89,23 @@ } bb0: { +- StorageLive(_3); // scope 0 at $DIR/slice_filter.rs:+0:27: +0:28 ++ nop; // scope 0 at $DIR/slice_filter.rs:+0:27: +0:28 _25 = deref_copy (*_2); // scope 0 at $DIR/slice_filter.rs:+0:27: +0:28 - _3 = &((*_25).0: usize); // scope 0 at $DIR/slice_filter.rs:+0:27: +0:28 +- StorageLive(_4); // scope 0 at $DIR/slice_filter.rs:+0:30: +0:31 + _20 = &((*_25).0: usize); // scope 0 at $DIR/slice_filter.rs:+0:27: +0:28 ++ nop; // scope 0 at $DIR/slice_filter.rs:+0:30: +0:31 _26 = deref_copy (*_2); // scope 0 at $DIR/slice_filter.rs:+0:30: +0:31 - _4 = &((*_26).1: usize); // scope 0 at $DIR/slice_filter.rs:+0:30: +0:31 +- StorageLive(_5); // scope 0 at $DIR/slice_filter.rs:+0:33: +0:34 + _15 = &((*_26).1: usize); // scope 0 at $DIR/slice_filter.rs:+0:30: +0:31 ++ nop; // scope 0 at $DIR/slice_filter.rs:+0:33: +0:34 _27 = deref_copy (*_2); // scope 0 at $DIR/slice_filter.rs:+0:33: +0:34 - _5 = &((*_27).2: usize); // scope 0 at $DIR/slice_filter.rs:+0:33: +0:34 +- StorageLive(_6); // scope 0 at $DIR/slice_filter.rs:+0:36: +0:37 + _11 = &((*_27).2: usize); // scope 0 at $DIR/slice_filter.rs:+0:33: +0:34 ++ nop; // scope 0 at $DIR/slice_filter.rs:+0:36: +0:37 _28 = deref_copy (*_2); // scope 0 at $DIR/slice_filter.rs:+0:36: +0:37 - _6 = &((*_28).3: usize); // scope 0 at $DIR/slice_filter.rs:+0:36: +0:37 - StorageLive(_7); // scope 1 at $DIR/slice_filter.rs:+0:40: +0:56 @@ -162,8 +170,16 @@ bb3: { - StorageDead(_16); // scope 1 at $DIR/slice_filter.rs:+0:75: +0:76 - StorageDead(_7); // scope 1 at $DIR/slice_filter.rs:+0:75: +0:76 +- StorageDead(_6); // scope 0 at $DIR/slice_filter.rs:+0:75: +0:76 +- StorageDead(_5); // scope 0 at $DIR/slice_filter.rs:+0:75: +0:76 +- StorageDead(_4); // scope 0 at $DIR/slice_filter.rs:+0:75: +0:76 +- StorageDead(_3); // scope 0 at $DIR/slice_filter.rs:+0:75: +0:76 + nop; // scope 1 at $DIR/slice_filter.rs:+0:75: +0:76 + nop; // scope 1 at $DIR/slice_filter.rs:+0:75: +0:76 ++ nop; // scope 0 at $DIR/slice_filter.rs:+0:75: +0:76 ++ nop; // scope 0 at $DIR/slice_filter.rs:+0:75: +0:76 ++ nop; // scope 0 at $DIR/slice_filter.rs:+0:75: +0:76 ++ nop; // scope 0 at $DIR/slice_filter.rs:+0:75: +0:76 return; // scope 0 at $DIR/slice_filter.rs:+0:76: +0:76 } diff --git a/tests/mir-opt/slice_filter.variant_a-{closure#0}.ReferencePropagation.diff b/tests/mir-opt/slice_filter.variant_a-{closure#0}.ReferencePropagation.diff index 18ea730595492..f6350b3812a2a 100644 --- a/tests/mir-opt/slice_filter.variant_a-{closure#0}.ReferencePropagation.diff +++ b/tests/mir-opt/slice_filter.variant_a-{closure#0}.ReferencePropagation.diff @@ -93,12 +93,16 @@ } bb0: { + StorageLive(_3); // scope 0 at $DIR/slice_filter.rs:+0:27: +0:28 _25 = deref_copy (*_2); // scope 0 at $DIR/slice_filter.rs:+0:27: +0:28 _3 = &((*_25).0: usize); // scope 0 at $DIR/slice_filter.rs:+0:27: +0:28 + StorageLive(_4); // scope 0 at $DIR/slice_filter.rs:+0:30: +0:31 _26 = deref_copy (*_2); // scope 0 at $DIR/slice_filter.rs:+0:30: +0:31 _4 = &((*_26).1: usize); // scope 0 at $DIR/slice_filter.rs:+0:30: +0:31 + StorageLive(_5); // scope 0 at $DIR/slice_filter.rs:+0:33: +0:34 _27 = deref_copy (*_2); // scope 0 at $DIR/slice_filter.rs:+0:33: +0:34 _5 = &((*_27).2: usize); // scope 0 at $DIR/slice_filter.rs:+0:33: +0:34 + StorageLive(_6); // scope 0 at $DIR/slice_filter.rs:+0:36: +0:37 _28 = deref_copy (*_2); // scope 0 at $DIR/slice_filter.rs:+0:36: +0:37 _6 = &((*_28).3: usize); // scope 0 at $DIR/slice_filter.rs:+0:36: +0:37 StorageLive(_7); // scope 1 at $DIR/slice_filter.rs:+0:40: +0:56 @@ -160,6 +164,10 @@ bb3: { StorageDead(_16); // scope 1 at $DIR/slice_filter.rs:+0:75: +0:76 StorageDead(_7); // scope 1 at $DIR/slice_filter.rs:+0:75: +0:76 + StorageDead(_6); // scope 0 at $DIR/slice_filter.rs:+0:75: +0:76 + StorageDead(_5); // scope 0 at $DIR/slice_filter.rs:+0:75: +0:76 + StorageDead(_4); // scope 0 at $DIR/slice_filter.rs:+0:75: +0:76 + StorageDead(_3); // scope 0 at $DIR/slice_filter.rs:+0:75: +0:76 return; // scope 0 at $DIR/slice_filter.rs:+0:76: +0:76 }