Skip to content

Commit d29dc05

Browse files
committed
Do not merge locals that have their address taken.
1 parent 9096d31 commit d29dc05

21 files changed

+329
-168
lines changed

compiler/rustc_mir_transform/src/copy_prop.rs

+58-13
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use rustc_index::vec::IndexVec;
33
use rustc_middle::mir::visit::*;
44
use rustc_middle::mir::*;
55
use rustc_middle::ty::TyCtxt;
6+
use rustc_mir_dataflow::impls::borrowed_locals;
67

78
use crate::ssa::SsaLocals;
89
use crate::MirPass;
@@ -33,7 +34,8 @@ impl<'tcx> MirPass<'tcx> for CopyProp {
3334

3435
fn propagate_ssa<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
3536
let param_env = tcx.param_env_reveal_all_normalized(body.source.def_id());
36-
let ssa = SsaLocals::new(tcx, param_env, body);
37+
let borrowed_locals = borrowed_locals(body);
38+
let ssa = SsaLocals::new(tcx, param_env, body, &borrowed_locals);
3739

3840
let fully_moved = fully_moved_locals(&ssa, body);
3941
debug!(?fully_moved);
@@ -42,14 +44,19 @@ fn propagate_ssa<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
4244
for (local, &head) in ssa.copy_classes().iter_enumerated() {
4345
if local != head {
4446
storage_to_remove.insert(head);
45-
storage_to_remove.insert(local);
4647
}
4748
}
4849

4950
let any_replacement = ssa.copy_classes().iter_enumerated().any(|(l, &h)| l != h);
5051

51-
Replacer { tcx, copy_classes: &ssa.copy_classes(), fully_moved, storage_to_remove }
52-
.visit_body_preserves_cfg(body);
52+
Replacer {
53+
tcx,
54+
copy_classes: &ssa.copy_classes(),
55+
fully_moved,
56+
borrowed_locals,
57+
storage_to_remove,
58+
}
59+
.visit_body_preserves_cfg(body);
5360

5461
if any_replacement {
5562
crate::simplify::remove_unused_definitions(body);
@@ -94,6 +101,7 @@ struct Replacer<'a, 'tcx> {
94101
tcx: TyCtxt<'tcx>,
95102
fully_moved: BitSet<Local>,
96103
storage_to_remove: BitSet<Local>,
104+
borrowed_locals: BitSet<Local>,
97105
copy_classes: &'a IndexVec<Local, Local>,
98106
}
99107

@@ -102,8 +110,45 @@ impl<'tcx> MutVisitor<'tcx> for Replacer<'_, 'tcx> {
102110
self.tcx
103111
}
104112

105-
fn visit_local(&mut self, local: &mut Local, _: PlaceContext, _: Location) {
106-
*local = self.copy_classes[*local];
113+
fn visit_local(&mut self, local: &mut Local, ctxt: PlaceContext, _: Location) {
114+
let new_local = self.copy_classes[*local];
115+
match ctxt {
116+
// Do not modify the local in storage statements.
117+
PlaceContext::NonUse(NonUseContext::StorageLive | NonUseContext::StorageDead) => {}
118+
// The local should have been marked as non-SSA.
119+
PlaceContext::MutatingUse(_) => assert_eq!(*local, new_local),
120+
// We access the value.
121+
_ => *local = new_local,
122+
}
123+
}
124+
125+
fn visit_place(&mut self, place: &mut Place<'tcx>, ctxt: PlaceContext, loc: Location) {
126+
if let Some(new_projection) = self.process_projection(&place.projection, loc) {
127+
place.projection = self.tcx().intern_place_elems(&new_projection);
128+
}
129+
130+
let observes_address = match ctxt {
131+
PlaceContext::NonMutatingUse(
132+
NonMutatingUseContext::SharedBorrow
133+
| NonMutatingUseContext::ShallowBorrow
134+
| NonMutatingUseContext::UniqueBorrow
135+
| NonMutatingUseContext::AddressOf,
136+
) => true,
137+
// For debuginfo, merging locals is ok.
138+
PlaceContext::NonUse(NonUseContext::VarDebugInfo) => {
139+
self.borrowed_locals.contains(place.local)
140+
}
141+
_ => false,
142+
};
143+
if observes_address && !place.is_indirect() {
144+
// We observe the address of `place.local`. Do not replace it.
145+
} else {
146+
self.visit_local(
147+
&mut place.local,
148+
PlaceContext::NonMutatingUse(NonMutatingUseContext::Copy),
149+
loc,
150+
)
151+
}
107152
}
108153

109154
fn visit_operand(&mut self, operand: &mut Operand<'tcx>, loc: Location) {
@@ -117,17 +162,17 @@ impl<'tcx> MutVisitor<'tcx> for Replacer<'_, 'tcx> {
117162
}
118163

119164
fn visit_statement(&mut self, stmt: &mut Statement<'tcx>, loc: Location) {
120-
if let StatementKind::StorageLive(l) | StatementKind::StorageDead(l) = stmt.kind
165+
if let StatementKind::StorageDead(l) = stmt.kind
121166
&& self.storage_to_remove.contains(l)
122167
{
123168
stmt.make_nop();
124-
}
125-
if let StatementKind::Assign(box (ref place, _)) = stmt.kind
126-
&& let Some(l) = place.as_local()
127-
&& self.copy_classes[l] != l
169+
} else if let StatementKind::Assign(box (ref place, ref mut rvalue)) = stmt.kind
170+
&& place.as_local().is_some()
128171
{
129-
stmt.make_nop();
172+
// Do not replace assignments.
173+
self.visit_rvalue(rvalue, loc)
174+
} else {
175+
self.super_statement(stmt, loc);
130176
}
131-
self.super_statement(stmt, loc);
132177
}
133178
}

compiler/rustc_mir_transform/src/ssa.rs

+7-4
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ use rustc_middle::middle::resolve_lifetime::Set1;
66
use rustc_middle::mir::visit::*;
77
use rustc_middle::mir::*;
88
use rustc_middle::ty::{ParamEnv, TyCtxt};
9-
use rustc_mir_dataflow::impls::borrowed_locals;
109

1110
#[derive(Debug)]
1211
pub struct SsaLocals {
@@ -21,19 +20,23 @@ pub struct SsaLocals {
2120
}
2221

2322
impl SsaLocals {
24-
pub fn new<'tcx>(tcx: TyCtxt<'tcx>, param_env: ParamEnv<'tcx>, body: &Body<'tcx>) -> SsaLocals {
23+
pub fn new<'tcx>(
24+
tcx: TyCtxt<'tcx>,
25+
param_env: ParamEnv<'tcx>,
26+
body: &Body<'tcx>,
27+
borrowed_locals: &BitSet<Local>,
28+
) -> SsaLocals {
2529
let assignment_order = Vec::new();
2630

2731
let assignments = IndexVec::from_elem(Set1::Empty, &body.local_decls);
2832
let dominators = body.basic_blocks.dominators();
2933
let mut visitor = SsaVisitor { assignments, assignment_order, dominators };
3034

31-
let borrowed = borrowed_locals(body);
3235
for (local, decl) in body.local_decls.iter_enumerated() {
3336
if matches!(body.local_kind(local), LocalKind::Arg) {
3437
visitor.assignments[local] = Set1::One(LocationExtended::Arg);
3538
}
36-
if borrowed.contains(local) && !decl.ty.is_freeze(tcx, param_env) {
39+
if borrowed_locals.contains(local) && !decl.ty.is_freeze(tcx, param_env) {
3740
visitor.assignments[local] = Set1::Many;
3841
}
3942
}

tests/mir-opt/const_debuginfo.main.ConstDebugInfo.diff

+3
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,11 @@
5656
}
5757

5858
bb0: {
59+
StorageLive(_1); // scope 0 at $DIR/const_debuginfo.rs:+1:9: +1:10
5960
_1 = const 1_u8; // scope 0 at $DIR/const_debuginfo.rs:+1:13: +1:16
61+
StorageLive(_2); // scope 1 at $DIR/const_debuginfo.rs:+2:9: +2:10
6062
_2 = const 2_u8; // scope 1 at $DIR/const_debuginfo.rs:+2:13: +2:16
63+
StorageLive(_3); // scope 2 at $DIR/const_debuginfo.rs:+3:9: +3:10
6164
_3 = const 3_u8; // scope 2 at $DIR/const_debuginfo.rs:+3:13: +3:16
6265
StorageLive(_4); // scope 3 at $DIR/const_debuginfo.rs:+4:9: +4:12
6366
StorageLive(_5); // scope 3 at $DIR/const_debuginfo.rs:+4:15: +4:20

tests/mir-opt/const_prop/bad_op_mod_by_zero.main.ConstProp.diff

+1
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
}
1919

2020
bb0: {
21+
StorageLive(_1); // scope 0 at $DIR/bad_op_mod_by_zero.rs:+1:9: +1:10
2122
_1 = const 0_i32; // scope 0 at $DIR/bad_op_mod_by_zero.rs:+1:13: +1:14
2223
StorageLive(_2); // scope 1 at $DIR/bad_op_mod_by_zero.rs:+2:9: +2:11
2324
- _4 = Eq(_1, const 0_i32); // scope 1 at $DIR/bad_op_mod_by_zero.rs:+2:14: +2:19

tests/mir-opt/const_prop/scalar_literal_propagation.main.ConstProp.diff

+1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
}
1212

1313
bb0: {
14+
StorageLive(_1); // scope 0 at $DIR/scalar_literal_propagation.rs:+1:9: +1:10
1415
_1 = const 1_u32; // scope 0 at $DIR/scalar_literal_propagation.rs:+1:13: +1:14
1516
StorageLive(_2); // scope 1 at $DIR/scalar_literal_propagation.rs:+2:5: +2:15
1617
- _2 = consume(_1) -> bb1; // scope 1 at $DIR/scalar_literal_propagation.rs:+2:5: +2:15
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
- // MIR for `f` before CopyProp
2+
+ // MIR for `f` after CopyProp
3+
4+
fn f() -> bool {
5+
let mut _0: bool; // return place in scope 0 at $DIR/borrowed_local.rs:+0:11: +0:15
6+
let mut _1: u8; // in scope 0 at $SRC_DIR/core/src/intrinsics/mir.rs:LL:COL
7+
let mut _2: &u8; // in scope 0 at $SRC_DIR/core/src/intrinsics/mir.rs:LL:COL
8+
let mut _3: u8; // in scope 0 at $SRC_DIR/core/src/intrinsics/mir.rs:LL:COL
9+
let mut _4: &u8; // in scope 0 at $SRC_DIR/core/src/intrinsics/mir.rs:LL:COL
10+
11+
bb0: {
12+
_1 = const 5_u8; // scope 0 at $SRC_DIR/core/src/intrinsics/mir.rs:LL:COL
13+
_2 = &_1; // scope 0 at $SRC_DIR/core/src/intrinsics/mir.rs:LL:COL
14+
_3 = _1; // scope 0 at $SRC_DIR/core/src/intrinsics/mir.rs:LL:COL
15+
_4 = &_3; // scope 0 at $SRC_DIR/core/src/intrinsics/mir.rs:LL:COL
16+
_0 = cmp_ref(_2, _4) -> bb1; // scope 0 at $DIR/borrowed_local.rs:+8:13: +8:45
17+
// mir::Constant
18+
// + span: $DIR/borrowed_local.rs:23:29: 23:36
19+
// + literal: Const { ty: for<'a, 'b> fn(&'a u8, &'b u8) -> bool {cmp_ref}, val: Value(<ZST>) }
20+
}
21+
22+
bb1: {
23+
- _0 = opaque::<u8>(_3) -> bb2; // scope 0 at $DIR/borrowed_local.rs:+12:13: +12:38
24+
+ _0 = opaque::<u8>(_1) -> bb2; // scope 0 at $DIR/borrowed_local.rs:+12:13: +12:38
25+
// mir::Constant
26+
// + span: $DIR/borrowed_local.rs:27:28: 27:34
27+
// + literal: Const { ty: fn(u8) -> bool {opaque::<u8>}, val: Value(<ZST>) }
28+
}
29+
30+
bb2: {
31+
return; // scope 0 at $DIR/borrowed_local.rs:+15:13: +15:21
32+
}
33+
}
34+
+39
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
// unit-test: CopyProp
2+
3+
#![feature(custom_mir, core_intrinsics)]
4+
#![allow(unused_assignments)]
5+
extern crate core;
6+
use core::intrinsics::mir::*;
7+
8+
fn opaque(_: impl Sized) -> bool { true }
9+
10+
fn cmp_ref(a: &u8, b: &u8) -> bool {
11+
std::ptr::eq(a as *const u8, b as *const u8)
12+
}
13+
14+
#[custom_mir(dialect = "analysis", phase = "post-cleanup")]
15+
fn f() -> bool {
16+
mir!(
17+
{
18+
let a = 5_u8;
19+
let r1 = &a;
20+
let b = a;
21+
// We cannot propagate the place `a`.
22+
let r2 = &b;
23+
Call(RET, next, cmp_ref(r1, r2))
24+
}
25+
next = {
26+
// But we can propagate the value `a`.
27+
Call(RET, ret, opaque(b))
28+
}
29+
ret = {
30+
Return()
31+
}
32+
)
33+
}
34+
35+
fn main() {
36+
assert!(!f());
37+
}
38+
39+
// EMIT_MIR borrowed_local.f.CopyProp.diff

tests/mir-opt/copy-prop/cycle.main.CopyProp.diff

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
}
3030

3131
bb1: {
32-
- StorageLive(_2); // scope 1 at $DIR/cycle.rs:+2:9: +2:10
32+
StorageLive(_2); // scope 1 at $DIR/cycle.rs:+2:9: +2:10
3333
_2 = _1; // scope 1 at $DIR/cycle.rs:+2:13: +2:14
3434
- StorageLive(_3); // scope 2 at $DIR/cycle.rs:+3:9: +3:10
3535
- _3 = _2; // scope 2 at $DIR/cycle.rs:+3:13: +3:14

tests/mir-opt/copy-prop/dead_stores_79191.f.CopyProp.after.mir

+1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ fn f(_1: usize) -> usize {
1111
}
1212

1313
bb0: {
14+
StorageLive(_2); // scope 0 at $DIR/dead_stores_79191.rs:+1:9: +1:10
1415
_2 = _1; // scope 0 at $DIR/dead_stores_79191.rs:+1:13: +1:14
1516
_1 = const 5_usize; // scope 1 at $DIR/dead_stores_79191.rs:+2:5: +2:10
1617
_1 = _2; // scope 1 at $DIR/dead_stores_79191.rs:+3:5: +3:10

tests/mir-opt/copy-prop/dead_stores_better.f.CopyProp.after.mir

+1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ fn f(_1: usize) -> usize {
1111
}
1212

1313
bb0: {
14+
StorageLive(_2); // scope 0 at $DIR/dead_stores_better.rs:+1:9: +1:10
1415
_2 = _1; // scope 0 at $DIR/dead_stores_better.rs:+1:13: +1:14
1516
_1 = const 5_usize; // scope 1 at $DIR/dead_stores_better.rs:+2:5: +2:10
1617
_1 = _2; // scope 1 at $DIR/dead_stores_better.rs:+3:5: +3:10

tests/mir-opt/dataflow-const-prop/inherit_overflow.main.DataflowConstProp.diff

+2
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,9 @@
1616
}
1717

1818
bb0: {
19+
StorageLive(_1); // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
1920
_1 = const u8::MAX; // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
21+
StorageLive(_2); // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
2022
_2 = const 1_u8; // scope 0 at $DIR/inherit_overflow.rs:+3:13: +3:47
2123
_5 = CheckedAdd(const u8::MAX, const 1_u8); // scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL
2224
assert(!move (_5.1: bool), "attempt to compute `{} + {}`, which would overflow", const u8::MAX, const 1_u8) -> bb1; // scope 2 at $SRC_DIR/core/src/ops/arith.rs:LL:COL

tests/mir-opt/funky_arms.float_to_exponential_common.ConstProp.diff

+1
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@
7979
}
8080

8181
bb6: {
82+
StorageLive(_10); // scope 3 at $DIR/funky_arms.rs:+13:17: +13:26
8283
_10 = ((_7 as Some).0: usize); // scope 3 at $DIR/funky_arms.rs:+13:17: +13:26
8384
StorageLive(_11); // scope 3 at $DIR/funky_arms.rs:+15:43: +15:46
8485
_11 = &mut (*_1); // scope 3 at $DIR/funky_arms.rs:+15:43: +15:46

tests/mir-opt/issue_101973.inner.ConstProp.diff

+2
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
bb0: {
3434
StorageLive(_2); // scope 0 at $DIR/issue_101973.rs:+1:5: +1:65
3535
StorageLive(_3); // scope 0 at $DIR/issue_101973.rs:+1:5: +1:58
36+
StorageLive(_4); // scope 0 at $DIR/issue_101973.rs:+1:5: +1:17
3637
StorageLive(_12); // scope 2 at $DIR/issue_101973.rs:7:12: 7:27
3738
StorageLive(_13); // scope 2 at $DIR/issue_101973.rs:7:12: 7:20
3839
_14 = CheckedShr(_1, const 0_i32); // scope 2 at $DIR/issue_101973.rs:7:12: 7:20
@@ -62,6 +63,7 @@
6263
StorageDead(_13); // scope 2 at $DIR/issue_101973.rs:7:26: 7:27
6364
_4 = BitOr(const 0_u32, move _12); // scope 2 at $DIR/issue_101973.rs:7:5: 7:27
6465
StorageDead(_12); // scope 2 at $DIR/issue_101973.rs:7:26: 7:27
66+
StorageLive(_6); // scope 0 at $DIR/issue_101973.rs:+1:31: +1:57
6567
StorageLive(_7); // scope 0 at $DIR/issue_101973.rs:+1:31: +1:52
6668
StorageLive(_8); // scope 0 at $DIR/issue_101973.rs:+1:32: +1:45
6769
_10 = CheckedShr(_1, const 8_i32); // scope 0 at $DIR/issue_101973.rs:+1:32: +1:45

tests/mir-opt/issue_76432.test.SimplifyComparisonIntegral.diff

+1
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929

3030
bb0: {
3131
StorageLive(_2); // scope 0 at $DIR/issue_76432.rs:+1:9: +1:10
32+
StorageLive(_4); // scope 0 at $DIR/issue_76432.rs:+1:19: +1:29
3233
StorageLive(_5); // scope 0 at $DIR/issue_76432.rs:+1:20: +1:29
3334
_5 = [_1, _1, _1]; // scope 0 at $DIR/issue_76432.rs:+1:20: +1:29
3435
_4 = &_5; // scope 0 at $DIR/issue_76432.rs:+1:19: +1:29

tests/mir-opt/simplify_match.main.ConstProp.diff

+1
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
}
1111

1212
bb0: {
13+
StorageLive(_2); // scope 0 at $DIR/simplify_match.rs:+1:17: +1:18
1314
_2 = const false; // scope 0 at $DIR/simplify_match.rs:+1:21: +1:26
1415
- switchInt(_2) -> [0: bb1, otherwise: bb2]; // scope 0 at $DIR/simplify_match.rs:+1:5: +1:31
1516
+ switchInt(const false) -> [0: bb1, otherwise: bb2]; // scope 0 at $DIR/simplify_match.rs:+1:5: +1:31

0 commit comments

Comments
 (0)