Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not consider &mut *x as mutating x in CopyProp #108178

Merged
merged 2 commits into from
Mar 9, 2023
+294 −16
Merged

Do not consider &mut *x as mutating x in CopyProp #108178

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2a32a2b
Special case deref projections in SsaVisitor.
cjgillot Feb 17, 2023
209eb8a
Do not grow `assignment_order` needlessly.
cjgillot Feb 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 40 additions & 16 deletions compiler/rustc_mir_transform/src/ssa.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl SsaLocals {
body: &Body<'tcx>,
borrowed_locals: &BitSet<Local>,
) -> SsaLocals {
let assignment_order = Vec::new();
let assignment_order = Vec::with_capacity(body.local_decls.len());

let assignments = IndexVec::from_elem(Set1::Empty, &body.local_decls);
let dominators =
Expand Down Expand Up @@ -179,37 +179,61 @@ struct SsaVisitor {
assignment_order: Vec<Local>,
}

impl SsaVisitor {
fn check_assignment_dominates(&mut self, local: Local, loc: Location) {
let set = &mut self.assignments[local];
let assign_dominates = match *set {
Set1::Empty | Set1::Many => false,
Set1::One(LocationExtended::Arg) => true,
Set1::One(LocationExtended::Plain(assign)) => {
assign.successor_within_block().dominates(loc, &self.dominators)
}
};
// We are visiting a use that is not dominated by an assignment.
// Either there is a cycle involved, or we are reading for uninitialized local.
// Bail out.
if !assign_dominates {
*set = Set1::Many;
}
}
}

impl<'tcx> Visitor<'tcx> for SsaVisitor {
fn visit_local(&mut self, local: Local, ctxt: PlaceContext, loc: Location) {
match ctxt {
PlaceContext::MutatingUse(MutatingUseContext::Store) => {
self.assignments[local].insert(LocationExtended::Plain(loc));
self.assignment_order.push(local);
if let Set1::One(_) = self.assignments[local] {
// Only record if SSA-like, to avoid growing the vector needlessly.
self.assignment_order.push(local);
}
}
// Anything can happen with raw pointers, so remove them.
PlaceContext::NonMutatingUse(NonMutatingUseContext::AddressOf)
| PlaceContext::MutatingUse(_) => self.assignments[local] = Set1::Many,
// Immutable borrows are taken into account in `SsaLocals::new` by
// removing non-freeze locals.
PlaceContext::NonMutatingUse(_) => {
let set = &mut self.assignments[local];
let assign_dominates = match *set {
Set1::Empty | Set1::Many => false,
Set1::One(LocationExtended::Arg) => true,
Set1::One(LocationExtended::Plain(assign)) => {
assign.successor_within_block().dominates(loc, &self.dominators)
}
};
// We are visiting a use that is not dominated by an assignment.
// Either there is a cycle involved, or we are reading for uninitialized local.
// Bail out.
if !assign_dominates {
*set = Set1::Many;
}
self.check_assignment_dominates(local, loc);
}
PlaceContext::NonUse(_) => {}
}
}

fn visit_place(&mut self, place: &Place<'tcx>, ctxt: PlaceContext, loc: Location) {
if place.projection.first() == Some(&PlaceElem::Deref) {
// Do not do anything for storage statements and debuginfo.
if ctxt.is_use() {
// A use through a `deref` only reads from the local, and cannot write to it.
let new_ctxt = PlaceContext::NonMutatingUse(NonMutatingUseContext::Projection);

self.visit_projection(place.as_ref(), new_ctxt, loc);
self.check_assignment_dominates(place.local, loc);
}
return;
}
self.super_place(place, ctxt, loc);
}
}

#[instrument(level = "trace", skip(ssa, body))]
Expand Down
56 changes: 56 additions & 0 deletions tests/mir-opt/copy-prop/reborrow.demiraw.CopyProp.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
- // MIR for `demiraw` before CopyProp
+ // MIR for `demiraw` after CopyProp

fn demiraw(_1: u8) -> () {
debug x => _1; // in scope 0 at $DIR/reborrow.rs:+0:12: +0:17
let mut _0: (); // return place in scope 0 at $DIR/reborrow.rs:+0:23: +0:23
let _2: *mut u8; // in scope 0 at $DIR/reborrow.rs:+1:9: +1:10
let mut _4: &mut u8; // in scope 0 at $DIR/reborrow.rs:+2:22: +2:29
let _6: (); // in scope 0 at $DIR/reborrow.rs:+4:5: +4:14
let mut _7: *mut u8; // in scope 0 at $DIR/reborrow.rs:+4:12: +4:13
scope 1 {
debug a => _2; // in scope 1 at $DIR/reborrow.rs:+1:9: +1:10
let _3: &mut u8; // in scope 1 at $DIR/reborrow.rs:+2:9: +2:10
scope 2 {
debug b => _3; // in scope 2 at $DIR/reborrow.rs:+2:9: +2:10
let _5: *mut u8; // in scope 2 at $DIR/reborrow.rs:+3:9: +3:10
scope 4 {
- debug c => _5; // in scope 4 at $DIR/reborrow.rs:+3:9: +3:10
+ debug c => _2; // in scope 4 at $DIR/reborrow.rs:+3:9: +3:10
}
}
scope 3 {
}
}

bb0: {
- StorageLive(_2); // scope 0 at $DIR/reborrow.rs:+1:9: +1:10
_2 = &raw mut _1; // scope 0 at $DIR/reborrow.rs:+1:13: +1:23
StorageLive(_3); // scope 1 at $DIR/reborrow.rs:+2:9: +2:10
StorageLive(_4); // scope 1 at $DIR/reborrow.rs:+2:22: +2:29
_4 = &mut (*_2); // scope 3 at $DIR/reborrow.rs:+2:22: +2:29
_3 = &mut (*_4); // scope 1 at $DIR/reborrow.rs:+2:22: +2:29
StorageDead(_4); // scope 1 at $DIR/reborrow.rs:+2:31: +2:32
- StorageLive(_5); // scope 2 at $DIR/reborrow.rs:+3:9: +3:10
- _5 = _2; // scope 2 at $DIR/reborrow.rs:+3:13: +3:14
StorageLive(_6); // scope 4 at $DIR/reborrow.rs:+4:5: +4:14
- StorageLive(_7); // scope 4 at $DIR/reborrow.rs:+4:12: +4:13
- _7 = _5; // scope 4 at $DIR/reborrow.rs:+4:12: +4:13
- _6 = opaque::<*mut u8>(move _7) -> bb1; // scope 4 at $DIR/reborrow.rs:+4:5: +4:14
+ _6 = opaque::<*mut u8>(_2) -> bb1; // scope 4 at $DIR/reborrow.rs:+4:5: +4:14
// mir::Constant
// + span: $DIR/reborrow.rs:38:5: 38:11
// + literal: Const { ty: fn(*mut u8) {opaque::<*mut u8>}, val: Value(<ZST>) }
}

bb1: {
- StorageDead(_7); // scope 4 at $DIR/reborrow.rs:+4:13: +4:14
StorageDead(_6); // scope 4 at $DIR/reborrow.rs:+4:14: +4:15
_0 = const (); // scope 0 at $DIR/reborrow.rs:+0:23: +5:2
- StorageDead(_5); // scope 2 at $DIR/reborrow.rs:+5:1: +5:2
StorageDead(_3); // scope 1 at $DIR/reborrow.rs:+5:1: +5:2
- StorageDead(_2); // scope 0 at $DIR/reborrow.rs:+5:1: +5:2
return; // scope 0 at $DIR/reborrow.rs:+5:2: +5:2
}
}

52 changes: 52 additions & 0 deletions tests/mir-opt/copy-prop/reborrow.miraw.CopyProp.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
- // MIR for `miraw` before CopyProp
+ // MIR for `miraw` after CopyProp

fn miraw(_1: u8) -> () {
debug x => _1; // in scope 0 at $DIR/reborrow.rs:+0:10: +0:15
let mut _0: (); // return place in scope 0 at $DIR/reborrow.rs:+0:21: +0:21
let _2: *mut u8; // in scope 0 at $DIR/reborrow.rs:+1:9: +1:10
let _5: (); // in scope 0 at $DIR/reborrow.rs:+4:5: +4:14
let mut _6: *mut u8; // in scope 0 at $DIR/reborrow.rs:+4:12: +4:13
scope 1 {
debug a => _2; // in scope 1 at $DIR/reborrow.rs:+1:9: +1:10
let _3: *mut u8; // in scope 1 at $DIR/reborrow.rs:+2:9: +2:10
scope 2 {
debug b => _3; // in scope 2 at $DIR/reborrow.rs:+2:9: +2:10
let _4: *mut u8; // in scope 2 at $DIR/reborrow.rs:+3:9: +3:10
scope 4 {
- debug c => _4; // in scope 4 at $DIR/reborrow.rs:+3:9: +3:10
+ debug c => _2; // in scope 4 at $DIR/reborrow.rs:+3:9: +3:10
}
}
scope 3 {
}
}

bb0: {
- StorageLive(_2); // scope 0 at $DIR/reborrow.rs:+1:9: +1:10
_2 = &raw mut _1; // scope 0 at $DIR/reborrow.rs:+1:13: +1:23
StorageLive(_3); // scope 1 at $DIR/reborrow.rs:+2:9: +2:10
_3 = &raw mut (*_2); // scope 3 at $DIR/reborrow.rs:+2:22: +2:33
- StorageLive(_4); // scope 2 at $DIR/reborrow.rs:+3:9: +3:10
- _4 = _2; // scope 2 at $DIR/reborrow.rs:+3:13: +3:14
StorageLive(_5); // scope 4 at $DIR/reborrow.rs:+4:5: +4:14
- StorageLive(_6); // scope 4 at $DIR/reborrow.rs:+4:12: +4:13
- _6 = _4; // scope 4 at $DIR/reborrow.rs:+4:12: +4:13
- _5 = opaque::<*mut u8>(move _6) -> bb1; // scope 4 at $DIR/reborrow.rs:+4:5: +4:14
+ _5 = opaque::<*mut u8>(_2) -> bb1; // scope 4 at $DIR/reborrow.rs:+4:5: +4:14
// mir::Constant
// + span: $DIR/reborrow.rs:30:5: 30:11
// + literal: Const { ty: fn(*mut u8) {opaque::<*mut u8>}, val: Value(<ZST>) }
}

bb1: {
- StorageDead(_6); // scope 4 at $DIR/reborrow.rs:+4:13: +4:14
StorageDead(_5); // scope 4 at $DIR/reborrow.rs:+4:14: +4:15
_0 = const (); // scope 0 at $DIR/reborrow.rs:+0:21: +5:2
- StorageDead(_4); // scope 2 at $DIR/reborrow.rs:+5:1: +5:2
StorageDead(_3); // scope 1 at $DIR/reborrow.rs:+5:1: +5:2
- StorageDead(_2); // scope 0 at $DIR/reborrow.rs:+5:1: +5:2
return; // scope 0 at $DIR/reborrow.rs:+5:2: +5:2
}
}

50 changes: 50 additions & 0 deletions tests/mir-opt/copy-prop/reborrow.remut.CopyProp.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
- // MIR for `remut` before CopyProp
+ // MIR for `remut` after CopyProp

fn remut(_1: u8) -> () {
debug x => _1; // in scope 0 at $DIR/reborrow.rs:+0:10: +0:15
let mut _0: (); // return place in scope 0 at $DIR/reborrow.rs:+0:21: +0:21
let _2: &mut u8; // in scope 0 at $DIR/reborrow.rs:+1:9: +1:10
let _5: (); // in scope 0 at $DIR/reborrow.rs:+4:5: +4:14
let mut _6: &mut u8; // in scope 0 at $DIR/reborrow.rs:+4:12: +4:13
scope 1 {
debug a => _2; // in scope 1 at $DIR/reborrow.rs:+1:9: +1:10
let _3: &mut u8; // in scope 1 at $DIR/reborrow.rs:+2:9: +2:10
scope 2 {
debug b => _3; // in scope 2 at $DIR/reborrow.rs:+2:9: +2:10
let _4: &mut u8; // in scope 2 at $DIR/reborrow.rs:+3:9: +3:10
scope 3 {
- debug c => _4; // in scope 3 at $DIR/reborrow.rs:+3:9: +3:10
+ debug c => _2; // in scope 3 at $DIR/reborrow.rs:+3:9: +3:10
}
}
}

bb0: {
- StorageLive(_2); // scope 0 at $DIR/reborrow.rs:+1:9: +1:10
_2 = &mut _1; // scope 0 at $DIR/reborrow.rs:+1:13: +1:19
StorageLive(_3); // scope 1 at $DIR/reborrow.rs:+2:9: +2:10
_3 = &mut (*_2); // scope 1 at $DIR/reborrow.rs:+2:13: +2:20
- StorageLive(_4); // scope 2 at $DIR/reborrow.rs:+3:9: +3:10
- _4 = move _2; // scope 2 at $DIR/reborrow.rs:+3:13: +3:14
StorageLive(_5); // scope 3 at $DIR/reborrow.rs:+4:5: +4:14
- StorageLive(_6); // scope 3 at $DIR/reborrow.rs:+4:12: +4:13
- _6 = move _4; // scope 3 at $DIR/reborrow.rs:+4:12: +4:13
- _5 = opaque::<&mut u8>(move _6) -> bb1; // scope 3 at $DIR/reborrow.rs:+4:5: +4:14
+ _5 = opaque::<&mut u8>(move _2) -> bb1; // scope 3 at $DIR/reborrow.rs:+4:5: +4:14
// mir::Constant
// + span: $DIR/reborrow.rs:14:5: 14:11
// + literal: Const { ty: fn(&mut u8) {opaque::<&mut u8>}, val: Value(<ZST>) }
}

bb1: {
- StorageDead(_6); // scope 3 at $DIR/reborrow.rs:+4:13: +4:14
StorageDead(_5); // scope 3 at $DIR/reborrow.rs:+4:14: +4:15
_0 = const (); // scope 0 at $DIR/reborrow.rs:+0:21: +5:2
- StorageDead(_4); // scope 2 at $DIR/reborrow.rs:+5:1: +5:2
StorageDead(_3); // scope 1 at $DIR/reborrow.rs:+5:1: +5:2
- StorageDead(_2); // scope 0 at $DIR/reborrow.rs:+5:1: +5:2
return; // scope 0 at $DIR/reborrow.rs:+5:2: +5:2
}
}

50 changes: 50 additions & 0 deletions tests/mir-opt/copy-prop/reborrow.reraw.CopyProp.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
- // MIR for `reraw` before CopyProp
+ // MIR for `reraw` after CopyProp

fn reraw(_1: u8) -> () {
debug x => _1; // in scope 0 at $DIR/reborrow.rs:+0:10: +0:15
let mut _0: (); // return place in scope 0 at $DIR/reborrow.rs:+0:21: +0:21
let _2: &mut u8; // in scope 0 at $DIR/reborrow.rs:+1:9: +1:10
let _5: (); // in scope 0 at $DIR/reborrow.rs:+4:5: +4:14
let mut _6: &mut u8; // in scope 0 at $DIR/reborrow.rs:+4:12: +4:13
scope 1 {
debug a => _2; // in scope 1 at $DIR/reborrow.rs:+1:9: +1:10
let _3: *mut u8; // in scope 1 at $DIR/reborrow.rs:+2:9: +2:10
scope 2 {
debug b => _3; // in scope 2 at $DIR/reborrow.rs:+2:9: +2:10
let _4: &mut u8; // in scope 2 at $DIR/reborrow.rs:+3:9: +3:10
scope 3 {
- debug c => _4; // in scope 3 at $DIR/reborrow.rs:+3:9: +3:10
+ debug c => _2; // in scope 3 at $DIR/reborrow.rs:+3:9: +3:10
}
}
}

bb0: {
- StorageLive(_2); // scope 0 at $DIR/reborrow.rs:+1:9: +1:10
_2 = &mut _1; // scope 0 at $DIR/reborrow.rs:+1:13: +1:19
StorageLive(_3); // scope 1 at $DIR/reborrow.rs:+2:9: +2:10
_3 = &raw mut (*_2); // scope 1 at $DIR/reborrow.rs:+2:13: +2:24
- StorageLive(_4); // scope 2 at $DIR/reborrow.rs:+3:9: +3:10
- _4 = move _2; // scope 2 at $DIR/reborrow.rs:+3:13: +3:14
StorageLive(_5); // scope 3 at $DIR/reborrow.rs:+4:5: +4:14
- StorageLive(_6); // scope 3 at $DIR/reborrow.rs:+4:12: +4:13
- _6 = move _4; // scope 3 at $DIR/reborrow.rs:+4:12: +4:13
- _5 = opaque::<&mut u8>(move _6) -> bb1; // scope 3 at $DIR/reborrow.rs:+4:5: +4:14
+ _5 = opaque::<&mut u8>(move _2) -> bb1; // scope 3 at $DIR/reborrow.rs:+4:5: +4:14
// mir::Constant
// + span: $DIR/reborrow.rs:22:5: 22:11
// + literal: Const { ty: fn(&mut u8) {opaque::<&mut u8>}, val: Value(<ZST>) }
}

bb1: {
- StorageDead(_6); // scope 3 at $DIR/reborrow.rs:+4:13: +4:14
StorageDead(_5); // scope 3 at $DIR/reborrow.rs:+4:14: +4:15
_0 = const (); // scope 0 at $DIR/reborrow.rs:+0:21: +5:2
- StorageDead(_4); // scope 2 at $DIR/reborrow.rs:+5:1: +5:2
StorageDead(_3); // scope 1 at $DIR/reborrow.rs:+5:1: +5:2
- StorageDead(_2); // scope 0 at $DIR/reborrow.rs:+5:1: +5:2
return; // scope 0 at $DIR/reborrow.rs:+5:2: +5:2
}
}

46 changes: 46 additions & 0 deletions tests/mir-opt/copy-prop/reborrow.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
// Check that CopyProp considers reborrows as not mutating the pointer.
// unit-test: CopyProp

#![feature(raw_ref_op)]

#[inline(never)]
fn opaque(_: impl Sized) {}

// EMIT_MIR reborrow.remut.CopyProp.diff
fn remut(mut x: u8) {
let a = &mut x;
let b = &mut *a; //< this cannot mutate a.
let c = a; //< so `c` and `a` can be merged.
opaque(c);
}

// EMIT_MIR reborrow.reraw.CopyProp.diff
fn reraw(mut x: u8) {
let a = &mut x;
let b = &raw mut *a; //< this cannot mutate a.
let c = a; //< so `c` and `a` can be merged.
opaque(c);
}

// EMIT_MIR reborrow.miraw.CopyProp.diff
fn miraw(mut x: u8) {
let a = &raw mut x;
let b = unsafe { &raw mut *a }; //< this cannot mutate a.
let c = a; //< so `c` and `a` can be merged.
opaque(c);
}

// EMIT_MIR reborrow.demiraw.CopyProp.diff
fn demiraw(mut x: u8) {
let a = &raw mut x;
let b = unsafe { &mut *a }; //< this cannot mutate a.
let c = a; //< so `c` and `a` can be merged.
opaque(c);
}

fn main() {
remut(0);
reraw(0);
miraw(0);
demiraw(0);
}