Skip to content

Commit

Permalink
Do not consider borrowed Freeze locals as SSA.
Browse files Browse the repository at this point in the history
  • Loading branch information
cjgillot committed May 9, 2023
1 parent 3490375 commit 3c43b61
Show file tree
Hide file tree
Showing 10 changed files with 69 additions and 43 deletions.
3 changes: 1 addition & 2 deletions compiler/rustc_mir_transform/src/copy_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 1 addition & 4 deletions compiler/rustc_mir_transform/src/normalize_array_len.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);
Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_mir_transform/src/ref_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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);
Expand Down
49 changes: 30 additions & 19 deletions compiler/rustc_mir_transform/src/ssa.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,23 @@
//! 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;
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)]
Expand Down Expand Up @@ -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<Local>,
) -> 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);
Expand All @@ -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() {
Expand Down Expand Up @@ -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] {
Expand All @@ -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);
Expand All @@ -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);
}
}

Expand Down
3 changes: 1 addition & 2 deletions tests/mir-opt/copy-prop/borrowed_local.f.CopyProp.diff
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@
}

bb1: {
- _0 = opaque::<u8>(_3) -> bb2; // scope 0 at $DIR/borrowed_local.rs:+12:13: +12:38
+ _0 = opaque::<u8>(_1) -> bb2; // scope 0 at $DIR/borrowed_local.rs:+12:13: +12:38
_0 = opaque::<u8>(_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::<u8>}, val: Value(<ZST>) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
16 changes: 8 additions & 8 deletions tests/mir-opt/slice_filter.variant_a-{closure#0}.CopyProp.diff
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}

Expand Down

0 comments on commit 3c43b61

Please sign in to comment.