diff options
12 files changed, 111 insertions, 121 deletions
diff --git a/compiler/rustc_mir_transform/src/copy_prop.rs b/compiler/rustc_mir_transform/src/copy_prop.rs index 0119b95cced..6094209952f 100644 --- a/compiler/rustc_mir_transform/src/copy_prop.rs +++ b/compiler/rustc_mir_transform/src/copy_prop.rs @@ -3,7 +3,6 @@ use rustc_index::IndexSlice; use rustc_middle::mir::visit::*; use rustc_middle::mir::*; use rustc_middle::ty::TyCtxt; -use rustc_mir_dataflow::impls::borrowed_locals; use crate::ssa::SsaLocals; @@ -32,8 +31,8 @@ impl<'tcx> MirPass<'tcx> for CopyProp { } fn propagate_ssa<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { - let borrowed_locals = borrowed_locals(body); - let ssa = SsaLocals::new(body); + let param_env = tcx.param_env_reveal_all_normalized(body.source.def_id()); + let ssa = SsaLocals::new(tcx, body, param_env); let fully_moved = fully_moved_locals(&ssa, body); debug!(?fully_moved); @@ -51,7 +50,7 @@ fn propagate_ssa<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { tcx, copy_classes: ssa.copy_classes(), fully_moved, - borrowed_locals, + borrowed_locals: ssa.borrowed_locals(), storage_to_remove, } .visit_body_preserves_cfg(body); @@ -101,7 +100,7 @@ struct Replacer<'a, 'tcx> { tcx: TyCtxt<'tcx>, fully_moved: BitSet<Local>, storage_to_remove: BitSet<Local>, - borrowed_locals: BitSet<Local>, + borrowed_locals: &'a BitSet<Local>, copy_classes: &'a IndexSlice<Local, Local>, } @@ -112,6 +111,9 @@ impl<'tcx> MutVisitor<'tcx> for Replacer<'_, 'tcx> { fn visit_local(&mut self, local: &mut Local, ctxt: PlaceContext, _: Location) { let new_local = self.copy_classes[*local]; + if self.borrowed_locals.contains(*local) || self.borrowed_locals.contains(new_local) { + return; + } match ctxt { // Do not modify the local in storage statements. PlaceContext::NonUse(NonUseContext::StorageLive | NonUseContext::StorageDead) => {} @@ -122,32 +124,14 @@ impl<'tcx> MutVisitor<'tcx> for Replacer<'_, 'tcx> { } } - fn visit_place(&mut self, place: &mut Place<'tcx>, ctxt: PlaceContext, loc: Location) { + fn visit_place(&mut self, place: &mut Place<'tcx>, _: PlaceContext, loc: Location) { if let Some(new_projection) = self.process_projection(place.projection, loc) { place.projection = self.tcx().mk_place_elems(&new_projection); } - let observes_address = match ctxt { - PlaceContext::NonMutatingUse( - NonMutatingUseContext::SharedBorrow - | NonMutatingUseContext::FakeBorrow - | NonMutatingUseContext::AddressOf, - ) => true, - // For debuginfo, merging locals is ok. - PlaceContext::NonUse(NonUseContext::VarDebugInfo) => { - self.borrowed_locals.contains(place.local) - } - _ => false, - }; - if observes_address && !place.is_indirect() { - // We observe the address of `place.local`. Do not replace it. - } else { - self.visit_local( - &mut place.local, - PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy), - loc, - ) - } + // Any non-mutating use context is ok. + let ctxt = PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy); + self.visit_local(&mut place.local, ctxt, loc) } fn visit_operand(&mut self, operand: &mut Operand<'tcx>, loc: Location) { diff --git a/compiler/rustc_mir_transform/src/gvn.rs b/compiler/rustc_mir_transform/src/gvn.rs index 8e8d78226c3..04251978865 100644 --- a/compiler/rustc_mir_transform/src/gvn.rs +++ b/compiler/rustc_mir_transform/src/gvn.rs @@ -121,7 +121,7 @@ impl<'tcx> MirPass<'tcx> for GVN { 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 ssa = SsaLocals::new(body); + let ssa = SsaLocals::new(tcx, body, param_env); // Clone dominators as we need them while mutating the body. let dominators = body.basic_blocks.dominators().clone(); diff --git a/compiler/rustc_mir_transform/src/normalize_array_len.rs b/compiler/rustc_mir_transform/src/normalize_array_len.rs index 128634bd7f2..c26a5461633 100644 --- a/compiler/rustc_mir_transform/src/normalize_array_len.rs +++ b/compiler/rustc_mir_transform/src/normalize_array_len.rs @@ -22,7 +22,8 @@ impl<'tcx> MirPass<'tcx> for NormalizeArrayLen { } fn normalize_array_len_calls<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) { - let ssa = SsaLocals::new(body); + let param_env = tcx.param_env_reveal_all_normalized(body.source.def_id()); + let ssa = SsaLocals::new(tcx, body, param_env); 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 d5642be5513..044ae32c1d4 100644 --- a/compiler/rustc_mir_transform/src/ref_prop.rs +++ b/compiler/rustc_mir_transform/src/ref_prop.rs @@ -82,7 +82,8 @@ impl<'tcx> MirPass<'tcx> for ReferencePropagation { } fn propagate_ssa<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) -> bool { - let ssa = SsaLocals::new(body); + let param_env = tcx.param_env_reveal_all_normalized(body.source.def_id()); + let ssa = SsaLocals::new(tcx, body, param_env); let mut replacer = compute_replacement(tcx, body, &ssa); debug!(?replacer.targets); diff --git a/compiler/rustc_mir_transform/src/ssa.rs b/compiler/rustc_mir_transform/src/ssa.rs index fddc62e6652..8b339cd4eff 100644 --- a/compiler/rustc_mir_transform/src/ssa.rs +++ b/compiler/rustc_mir_transform/src/ssa.rs @@ -2,8 +2,9 @@ //! 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. +//! As we do not track indirect assignments, a local that has its address taken (either by +//! AddressOf or by borrowing) is considered non-SSA. However, it is UB to modify through an +//! immutable borrow of a `Freeze` local. Those can still be considered to be SSA. use rustc_data_structures::graph::dominators::Dominators; use rustc_index::bit_set::BitSet; @@ -11,6 +12,7 @@ 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}; pub struct SsaLocals { /// Assignments to each local. This defines whether the local is SSA. @@ -24,6 +26,8 @@ pub struct SsaLocals { /// Number of "direct" uses of each local, ie. uses that are not dereferences. /// We ignore non-uses (Storage statements, debuginfo). direct_uses: IndexVec<Local, u32>, + /// Set of SSA locals that are immutably borrowed. + borrowed_locals: BitSet<Local>, } pub enum AssignedValue<'a, 'tcx> { @@ -33,15 +37,22 @@ pub enum AssignedValue<'a, 'tcx> { } impl SsaLocals { - pub fn new<'tcx>(body: &Body<'tcx>) -> SsaLocals { + pub fn new<'tcx>(tcx: TyCtxt<'tcx>, body: &Body<'tcx>, param_env: ParamEnv<'tcx>) -> SsaLocals { let assignment_order = Vec::with_capacity(body.local_decls.len()); let assignments = IndexVec::from_elem(Set1::Empty, &body.local_decls); let dominators = body.basic_blocks.dominators(); let direct_uses = IndexVec::from_elem(0, &body.local_decls); - let mut visitor = - SsaVisitor { body, assignments, assignment_order, dominators, direct_uses }; + let borrowed_locals = BitSet::new_empty(body.local_decls.len()); + let mut visitor = SsaVisitor { + body, + assignments, + assignment_order, + dominators, + direct_uses, + borrowed_locals, + }; for local in body.args_iter() { visitor.assignments[local] = Set1::One(DefLocation::Argument); @@ -58,6 +69,16 @@ impl SsaLocals { visitor.visit_var_debug_info(var_debug_info); } + // The immutability of shared borrows only works on `Freeze` locals. If the visitor found + // borrows, we need to check the types. For raw pointers and mutable borrows, the locals + // have already been marked as non-SSA. + debug!(?visitor.borrowed_locals); + for local in visitor.borrowed_locals.iter() { + if !body.local_decls[local].ty.is_freeze(tcx, param_env) { + visitor.assignments[local] = Set1::Many; + } + } + debug!(?visitor.assignments); debug!(?visitor.direct_uses); @@ -70,6 +91,7 @@ impl SsaLocals { assignments: visitor.assignments, assignment_order: visitor.assignment_order, direct_uses: visitor.direct_uses, + borrowed_locals: visitor.borrowed_locals, // This is filled by `compute_copy_classes`. copy_classes: IndexVec::default(), }; @@ -174,6 +196,11 @@ impl SsaLocals { &self.copy_classes } + /// Set of SSA locals that are immutably borrowed. + pub fn borrowed_locals(&self) -> &BitSet<Local> { + &self.borrowed_locals + } + /// Make a property uniform on a copy equivalence class by removing elements. pub fn meet_copy_equivalence(&self, property: &mut BitSet<Local>) { // Consolidate to have a local iff all its copies are. @@ -208,6 +235,8 @@ struct SsaVisitor<'tcx, 'a> { assignments: IndexVec<Local, Set1<DefLocation>>, assignment_order: Vec<Local>, direct_uses: IndexVec<Local, u32>, + // Track locals that are immutably borrowed, so we can check their type is `Freeze` later. + borrowed_locals: BitSet<Local>, } impl SsaVisitor<'_, '_> { @@ -232,16 +261,18 @@ impl<'tcx> Visitor<'tcx> for SsaVisitor<'tcx, '_> { PlaceContext::MutatingUse(MutatingUseContext::Projection) | PlaceContext::NonMutatingUse(NonMutatingUseContext::Projection) => bug!(), // Anything can happen with raw pointers, so remove them. - // 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::FakeBorrow - | NonMutatingUseContext::AddressOf, - ) + PlaceContext::NonMutatingUse(NonMutatingUseContext::AddressOf) | PlaceContext::MutatingUse(_) => { self.assignments[local] = Set1::Many; } + // Immutable borrows are ok, but we need to delay a check that the type is `Freeze`. + PlaceContext::NonMutatingUse( + NonMutatingUseContext::SharedBorrow | NonMutatingUseContext::FakeBorrow, + ) => { + self.borrowed_locals.insert(local); + self.check_dominates(local, loc); + self.direct_uses[local] += 1; + } PlaceContext::NonMutatingUse(_) => { self.check_dominates(local, loc); self.direct_uses[local] += 1; diff --git a/tests/mir-opt/gvn.borrowed.GVN.panic-abort.diff b/tests/mir-opt/gvn.borrowed.GVN.panic-abort.diff index 2825e0954c9..df3291bc1b5 100644 --- a/tests/mir-opt/gvn.borrowed.GVN.panic-abort.diff +++ b/tests/mir-opt/gvn.borrowed.GVN.panic-abort.diff @@ -13,7 +13,8 @@ } bb1: { - _0 = opaque::<u32>(_2) -> [return: bb2, unwind unreachable]; +- _0 = opaque::<u32>(_2) -> [return: bb2, unwind unreachable]; ++ _0 = opaque::<u32>(_1) -> [return: bb2, unwind unreachable]; } bb2: { diff --git a/tests/mir-opt/gvn.borrowed.GVN.panic-unwind.diff b/tests/mir-opt/gvn.borrowed.GVN.panic-unwind.diff index e03f8caa228..8f670de61ee 100644 --- a/tests/mir-opt/gvn.borrowed.GVN.panic-unwind.diff +++ b/tests/mir-opt/gvn.borrowed.GVN.panic-unwind.diff @@ -13,7 +13,8 @@ } bb1: { - _0 = opaque::<u32>(_2) -> [return: bb2, unwind continue]; +- _0 = opaque::<u32>(_2) -> [return: bb2, unwind continue]; ++ _0 = opaque::<u32>(_1) -> [return: bb2, unwind continue]; } bb2: { diff --git a/tests/mir-opt/gvn.rs b/tests/mir-opt/gvn.rs index 0f7cb9c0566..f168d4fec89 100644 --- a/tests/mir-opt/gvn.rs +++ b/tests/mir-opt/gvn.rs @@ -728,7 +728,7 @@ fn borrowed(x: u32) { // CHECK-NEXT: _3 = &_1; // CHECK-NEXT: _0 = opaque::<&u32>(_3) // CHECK: bb1: { - // CHECK-NEXT: _0 = opaque::<u32>(_2) + // CHECK-NEXT: _0 = opaque::<u32>(_1) // CHECK: bb2: { // CHECK-NEXT: _0 = opaque::<u32>((*_3)) mir!( diff --git a/tests/mir-opt/pre-codegen/slice_filter.variant_a-{closure#0}.PreCodegen.after.mir b/tests/mir-opt/pre-codegen/slice_filter.variant_a-{closure#0}.PreCodegen.after.mir index 65cac0a81ef..155a811f342 100644 --- a/tests/mir-opt/pre-codegen/slice_filter.variant_a-{closure#0}.PreCodegen.after.mir +++ b/tests/mir-opt/pre-codegen/slice_filter.variant_a-{closure#0}.PreCodegen.after.mir @@ -18,10 +18,10 @@ fn variant_a::{closure#0}(_1: &mut {closure@$DIR/slice_filter.rs:7:25: 7:39}, _2 let mut _24: &&usize; let _25: &usize; let mut _26: &&usize; - let mut _31: bool; + let mut _29: bool; + let mut _30: &&usize; + let _31: &usize; let mut _32: &&usize; - let _33: &usize; - let mut _34: &&usize; scope 1 { debug a => _4; debug b => _5; @@ -59,83 +59,69 @@ fn variant_a::{closure#0}(_1: &mut {closure@$DIR/slice_filter.rs:7:25: 7:39}, _2 scope 7 (inlined std::cmp::impls::<impl PartialOrd for usize>::le) { debug self => _27; debug other => _28; - let mut _29: usize; - let mut _30: usize; } } scope 8 (inlined std::cmp::impls::<impl PartialOrd for &usize>::le) { - debug self => _32; - debug other => _34; - let mut _35: &usize; - let mut _36: &usize; + debug self => _30; + debug other => _32; + let mut _33: &usize; + let mut _34: &usize; scope 9 (inlined std::cmp::impls::<impl PartialOrd for usize>::le) { - debug self => _35; - debug other => _36; - let mut _37: usize; - let mut _38: usize; + debug self => _33; + debug other => _34; + let mut _35: usize; + let mut _36: usize; } } } bb0: { - StorageLive(_4); _3 = (*_2); _4 = &((*_3).0: usize); - StorageLive(_5); _5 = &((*_3).1: usize); - StorageLive(_6); _6 = &((*_3).2: usize); - StorageLive(_7); _7 = &((*_3).3: usize); StorageLive(_15); StorageLive(_8); _8 = &_4; StorageLive(_10); - StorageLive(_9); - _9 = _6; + _9 = &((*_3).2: usize); _10 = &_9; StorageLive(_11); StorageLive(_12); _11 = _4; _12 = _9; - StorageLive(_13); - _13 = (*_11); - StorageLive(_14); - _14 = (*_12); - _15 = Le(move _13, move _14); - StorageDead(_14); - StorageDead(_13); + _13 = ((*_3).0: usize); + _14 = ((*_3).2: usize); + _15 = Le(_13, _14); StorageDead(_12); StorageDead(_11); switchInt(move _15) -> [0: bb1, otherwise: bb2]; } bb1: { - StorageDead(_9); StorageDead(_10); StorageDead(_8); goto -> bb4; } bb2: { - StorageDead(_9); StorageDead(_10); StorageDead(_8); StorageLive(_23); StorageLive(_16); _16 = &_7; StorageLive(_18); - StorageLive(_17); - _17 = _5; + _17 = &((*_3).1: usize); _18 = &_17; StorageLive(_19); StorageLive(_20); _19 = _7; _20 = _17; StorageLive(_21); - _21 = (*_19); + _21 = ((*_3).3: usize); StorageLive(_22); - _22 = (*_20); + _22 = ((*_3).1: usize); _23 = Le(move _21, move _22); StorageDead(_22); StorageDead(_21); @@ -145,38 +131,29 @@ fn variant_a::{closure#0}(_1: &mut {closure@$DIR/slice_filter.rs:7:25: 7:39}, _2 } bb3: { - StorageDead(_17); StorageDead(_18); StorageDead(_16); goto -> bb4; } bb4: { - StorageLive(_31); + StorageLive(_29); StorageLive(_24); _24 = &_6; StorageLive(_26); - StorageLive(_25); - _25 = _4; + _25 = &((*_3).0: usize); _26 = &_25; StorageLive(_27); StorageLive(_28); _27 = _6; _28 = _25; - StorageLive(_29); - _29 = (*_27); - StorageLive(_30); - _30 = (*_28); - _31 = Le(move _29, move _30); - StorageDead(_30); - StorageDead(_29); + _29 = Le(_14, _13); StorageDead(_28); StorageDead(_27); - switchInt(move _31) -> [0: bb5, otherwise: bb6]; + switchInt(move _29) -> [0: bb5, otherwise: bb6]; } bb5: { - StorageDead(_25); StorageDead(_26); StorageDead(_24); _0 = const false; @@ -184,41 +161,37 @@ fn variant_a::{closure#0}(_1: &mut {closure@$DIR/slice_filter.rs:7:25: 7:39}, _2 } bb6: { - StorageDead(_25); StorageDead(_26); StorageDead(_24); + StorageLive(_30); + _30 = &_5; StorageLive(_32); - _32 = &_5; - StorageLive(_34); + _31 = &((*_3).3: usize); + _32 = &_31; StorageLive(_33); - _33 = _7; - _34 = &_33; + StorageLive(_34); + _33 = _5; + _34 = _31; StorageLive(_35); + _35 = ((*_3).1: usize); StorageLive(_36); - _35 = _5; - _36 = _33; - StorageLive(_37); - _37 = (*_35); - StorageLive(_38); - _38 = (*_36); - _0 = Le(move _37, move _38); - StorageDead(_38); - StorageDead(_37); + _36 = ((*_3).3: usize); + _0 = Le(move _35, move _36); StorageDead(_36); StorageDead(_35); - StorageDead(_33); StorageDead(_34); + StorageDead(_33); StorageDead(_32); + StorageDead(_30); goto -> bb7; } bb7: { - StorageDead(_31); + StorageDead(_29); goto -> bb9; } bb8: { - StorageDead(_17); StorageDead(_18); StorageDead(_16); _0 = const true; @@ -228,10 +201,6 @@ fn variant_a::{closure#0}(_1: &mut {closure@$DIR/slice_filter.rs:7:25: 7:39}, _2 bb9: { StorageDead(_23); StorageDead(_15); - StorageDead(_7); - StorageDead(_6); - StorageDead(_5); - StorageDead(_4); return; } } diff --git a/tests/mir-opt/reference_prop.reference_propagation.ReferencePropagation.diff b/tests/mir-opt/reference_prop.reference_propagation.ReferencePropagation.diff index 1be2ce8d0bb..0dfe8781c18 100644 --- a/tests/mir-opt/reference_prop.reference_propagation.ReferencePropagation.diff +++ b/tests/mir-opt/reference_prop.reference_propagation.ReferencePropagation.diff @@ -247,7 +247,8 @@ StorageLive(_21); _21 = &_20; StorageLive(_22); - _22 = (*_20); +- _22 = (*_20); ++ _22 = _19; StorageLive(_23); StorageLive(_24); _24 = _21; @@ -394,7 +395,8 @@ StorageLive(_62); _62 = &_61; StorageLive(_63); - _63 = (*_61); +- _63 = (*_61); ++ _63 = _60; StorageLive(_64); StorageLive(_65); _65 = (); 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 1e6a168f756..21486a8616a 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 @@ -260,7 +260,8 @@ StorageLive(_20); _20 = &_19; StorageLive(_21); - _21 = (*_19); +- _21 = (*_19); ++ _21 = _18; StorageLive(_22); StorageLive(_23); _23 = _20; @@ -429,7 +430,8 @@ StorageLive(_67); _67 = &_66; StorageLive(_68); - _68 = (*_66); +- _68 = (*_66); ++ _68 = _65; StorageLive(_69); StorageLive(_70); _70 = (); diff --git a/tests/mir-opt/reference_prop.rs b/tests/mir-opt/reference_prop.rs index 70587dff0b5..ad0feab559f 100644 --- a/tests/mir-opt/reference_prop.rs +++ b/tests/mir-opt/reference_prop.rs @@ -49,7 +49,7 @@ fn reference_propagation<'a, T: Copy>(single: &'a T, mut multiple: &'a T) { // CHECK: [[a:_.*]] = const 5_usize; // CHECK: [[b:_.*]] = &[[a]]; // CHECK: [[d:_.*]] = &[[b]]; - // CHECK: [[c:_.*]] = (*[[b]]); + // CHECK: [[c:_.*]] = [[a]]; let a = 5_usize; let b = &a; @@ -138,8 +138,7 @@ fn reference_propagation<'a, T: Copy>(single: &'a T, mut multiple: &'a T) { // CHECK: [[a:_.*]] = const 5_usize; // CHECK: [[b:_.*]] = &[[a]]; // CHECK: [[d:_.*]] = &[[b]]; - // FIXME this could be [[a]] - // CHECK: [[c:_.*]] = (*[[b]]); + // CHECK: [[c:_.*]] = [[a]]; let a = 5_usize; let b = &a; @@ -363,7 +362,7 @@ fn reference_propagation_const_ptr<T: Copy>(single: *const T, mut multiple: *con // CHECK: [[a:_.*]] = const 5_usize; // CHECK: [[b:_.*]] = &raw const [[a]]; // CHECK: [[d:_.*]] = &[[b]]; - // CHECK: [[c:_.*]] = (*[[b]]); + // CHECK: [[c:_.*]] = [[a]]; let a = 5_usize; let b = &raw const a; @@ -467,8 +466,7 @@ fn reference_propagation_const_ptr<T: Copy>(single: *const T, mut multiple: *con // CHECK: [[a:_.*]] = const 5_usize; // CHECK: [[b:_.*]] = &raw const [[a]]; // CHECK: [[d:_.*]] = &[[b]]; - // FIXME this could be [[a]] - // CHECK: [[c:_.*]] = (*[[b]]); + // CHECK: [[c:_.*]] = [[a]]; let a = 5_usize; let b = &raw const a; |