Skip to content

Commit 6fd0e35

Browse files
ohadravidohadravid
committed
Fixed the implementation to account for borrowed locals
1 parent 6078a69 commit 6fd0e35

19 files changed

+102
-57
lines changed

‎compiler/rustc_mir_transform/src/copy_prop.rs

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,19 +32,28 @@ impl<'tcx> crate::MirPass<'tcx> for CopyProp {
3232

3333
let typing_env = body.typing_env(tcx);
3434
let ssa = SsaLocals::new(tcx, body, typing_env);
35-
debug!(borrowed_locals = ?ssa.borrowed_locals());
35+
let borrowed_locals = ssa.borrowed_locals().clone();
36+
37+
debug!(?borrowed_locals);
3638
debug!(copy_classes = ?ssa.copy_classes());
3739

3840
let fully_moved = fully_moved_locals(&ssa, body);
3941
debug!(?fully_moved);
4042

4143
let mut head_storage_to_check = DenseBitSet::new_empty(fully_moved.domain_size());
44+
let mut storage_to_remove = DenseBitSet::new_empty(fully_moved.domain_size());
4245

4346
for (local, &head) in ssa.copy_classes().iter_enumerated() {
4447
if local != head {
4548
// We need to determine if we can keep the head's storage statements (which enables better optimizations).
4649
// For every local's usage location, if the head is maybe-uninitialized, we'll need to remove it's storage statements.
4750
head_storage_to_check.insert(head);
51+
52+
if borrowed_locals.contains(local) {
53+
// To keep the storage of a head, we require that none of the locals in it's copy class are borrowed,
54+
// since otherwise we cannot easily identify when it is used.
55+
storage_to_remove.insert(head);
56+
}
4857
}
4958
}
5059

@@ -56,11 +65,6 @@ impl<'tcx> crate::MirPass<'tcx> for CopyProp {
5665
.iterate_to_fixpoint(tcx, body, Some("mir_opt::copy_prop"))
5766
.into_results_cursor(body);
5867

59-
// To keep the storage of a head, we require that none of the locals in it's copy class are borrowed,
60-
// since otherwise we cannot easily identify when it is used.
61-
let mut storage_to_remove = ssa.borrowed_locals().clone();
62-
storage_to_remove.intersect(&head_storage_to_check);
63-
6468
let mut storage_checker = StorageChecker {
6569
maybe_uninit,
6670
copy_classes: ssa.copy_classes(),

‎tests/mir-opt/copy-prop/copy_prop_borrowed_storage_not_removed.f.CopyProp.diff

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
- // MIR for `f` before CopyProp
2+
+ // MIR for `f` after CopyProp
3+
4+
fn f(_1: (T, T)) -> T {
5+
let mut _0: T;
6+
let mut _2: T;
7+
let mut _3: T;
8+
let mut _4: &T;
9+
10+
bb0: {
11+
- StorageLive(_2);
12+
_2 = copy (_1.0: T);
13+
- _3 = copy _2;
14+
- _4 = &_3;
15+
- StorageDead(_2);
16+
+ _4 = &_2;
17+
_0 = copy (*_4);
18+
return;
19+
}
20+
}
21+

‎tests/mir-opt/copy-prop/copy_prop_borrowed_storage_not_removed.rs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// skip-filecheck
2+
//@ test-mir-pass: CopyProp
3+
4+
#![feature(custom_mir, core_intrinsics, freeze)]
5+
6+
// Check that we remove the storage statements if one of the locals is borrowed,
7+
// and the head isn't borrowed.
8+
9+
use std::intrinsics::mir::*;
10+
use std::marker::Freeze;
11+
12+
// EMIT_MIR copy_prop_borrowed_storage_not_removed.f.CopyProp.diff
13+
14+
#[custom_mir(dialect = "runtime")]
15+
pub fn f<T: Copy + Freeze>(_1: (T, T)) -> T {
16+
mir! {
17+
let _2: T;
18+
let _3: T;
19+
let _4: &T;
20+
{
21+
StorageLive(_2);
22+
_2 = _1.0;
23+
_3 = _2;
24+
_4 = &_3;
25+
StorageDead(_2);
26+
RET = *_4;
27+
Return()
28+
}
29+
}
30+
}

‎tests/mir-opt/copy-prop/issue_141649.main.CopyProp.panic-abort.diff

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@
7878
StorageDead(_6);
7979
- StorageDead(_5);
8080
- StorageLive(_9);
81-
- StorageLive(_10);
81+
StorageLive(_10);
8282
_10 = S(const 5_usize, const 6_usize);
8383
StorageLive(_11);
8484
_11 = &_10;
@@ -104,7 +104,7 @@
104104
StorageDead(_14);
105105
- _9 = const ();
106106
StorageDead(_11);
107-
- StorageDead(_10);
107+
StorageDead(_10);
108108
- StorageDead(_9);
109109
- StorageLive(_16);
110110
StorageLive(_17);

‎tests/mir-opt/copy-prop/issue_141649.main.CopyProp.panic-unwind.diff

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@
7878
StorageDead(_6);
7979
- StorageDead(_5);
8080
- StorageLive(_9);
81-
- StorageLive(_10);
81+
StorageLive(_10);
8282
_10 = S(const 5_usize, const 6_usize);
8383
StorageLive(_11);
8484
_11 = &_10;
@@ -104,7 +104,7 @@
104104
StorageDead(_14);
105105
- _9 = const ();
106106
StorageDead(_11);
107-
- StorageDead(_10);
107+
StorageDead(_10);
108108
- StorageDead(_9);
109109
- StorageLive(_16);
110110
StorageLive(_17);

‎tests/mir-opt/dont_reset_cast_kind_without_updating_operand.test.GVN.32bit.panic-abort.diff

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,8 @@
7878
- _12 = AlignOf(());
7979
+ _11 = const 0_usize;
8080
+ _12 = const 1_usize;
81-
StorageLive(_14);
8281
StorageLive(_16);
83-
StorageLive(_17);
82+
StorageLive(_14);
8483
StorageLive(_19);
8584
_19 = const false;
8685
- switchInt(move _19) -> [0: bb6, otherwise: bb5];
@@ -103,15 +102,16 @@
103102
}
104103

105104
bb4: {
105+
StorageLive(_17);
106106
_17 = copy ((_15 as Ok).0: std::ptr::NonNull<[u8]>);
107107
StorageLive(_22);
108108
_22 = copy _17 as *mut [u8] (Transmute);
109109
_13 = copy _22 as *mut u8 (PtrToPtr);
110110
StorageDead(_22);
111-
StorageDead(_15);
112111
StorageDead(_17);
113-
StorageDead(_16);
112+
StorageDead(_15);
114113
StorageDead(_14);
114+
StorageDead(_16);
115115
_3 = ShallowInitBox(move _13, ());
116116
StorageDead(_13);
117117
StorageDead(_12);

‎tests/mir-opt/dont_reset_cast_kind_without_updating_operand.test.GVN.64bit.panic-abort.diff

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,8 @@
7878
- _12 = AlignOf(());
7979
+ _11 = const 0_usize;
8080
+ _12 = const 1_usize;
81-
StorageLive(_14);
8281
StorageLive(_16);
83-
StorageLive(_17);
82+
StorageLive(_14);
8483
StorageLive(_19);
8584
_19 = const false;
8685
- switchInt(move _19) -> [0: bb6, otherwise: bb5];
@@ -103,15 +102,16 @@
103102
}
104103

105104
bb4: {
105+
StorageLive(_17);
106106
_17 = copy ((_15 as Ok).0: std::ptr::NonNull<[u8]>);
107107
StorageLive(_22);
108108
_22 = copy _17 as *mut [u8] (Transmute);
109109
_13 = copy _22 as *mut u8 (PtrToPtr);
110110
StorageDead(_22);
111-
StorageDead(_15);
112111
StorageDead(_17);
113-
StorageDead(_16);
112+
StorageDead(_15);
114113
StorageDead(_14);
114+
StorageDead(_16);
115115
_3 = ShallowInitBox(move _13, ());
116116
StorageDead(_13);
117117
StorageDead(_12);

‎tests/mir-opt/jump_threading.identity.JumpThreading.panic-abort.diff

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,6 @@
4747
StorageLive(_4);
4848
_4 = copy _1;
4949
StorageLive(_10);
50-
StorageLive(_11);
51-
StorageLive(_12);
5250
_10 = discriminant(_4);
5351
switchInt(move _10) -> [0: bb7, 1: bb6, otherwise: bb1];
5452
}
@@ -92,8 +90,6 @@
9290
}
9391

9492
bb5: {
95-
StorageDead(_12);
96-
StorageDead(_11);
9793
StorageDead(_10);
9894
StorageDead(_4);
9995
_5 = discriminant(_3);
@@ -102,24 +98,26 @@
10298
}
10399

104100
bb6: {
101+
StorageLive(_12);
105102
_12 = move ((_4 as Err).0: i32);
106103
StorageLive(_13);
107104
_13 = Result::<Infallible, i32>::Err(copy _12);
108105
_3 = ControlFlow::<Result<Infallible, i32>, i32>::Break(move _13);
109106
StorageDead(_13);
107+
StorageDead(_12);
110108
- goto -> bb5;
111109
+ goto -> bb8;
112110
}
113111

114112
bb7: {
113+
StorageLive(_11);
115114
_11 = move ((_4 as Ok).0: i32);
116115
_3 = ControlFlow::<Result<Infallible, i32>, i32>::Continue(copy _11);
116+
StorageDead(_11);
117117
goto -> bb5;
118118
+ }
119119
+
120120
+ bb8: {
121-
+ StorageDead(_12);
122-
+ StorageDead(_11);
123121
+ StorageDead(_10);
124122
+ StorageDead(_4);
125123
+ _5 = discriminant(_3);

‎tests/mir-opt/jump_threading.identity.JumpThreading.panic-unwind.diff

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -47,8 +47,6 @@
4747
StorageLive(_4);
4848
_4 = copy _1;
4949
StorageLive(_10);
50-
StorageLive(_11);
51-
StorageLive(_12);
5250
_10 = discriminant(_4);
5351
switchInt(move _10) -> [0: bb7, 1: bb6, otherwise: bb1];
5452
}
@@ -92,8 +90,6 @@
9290
}
9391

9492
bb5: {
95-
StorageDead(_12);
96-
StorageDead(_11);
9793
StorageDead(_10);
9894
StorageDead(_4);
9995
_5 = discriminant(_3);
@@ -102,24 +98,26 @@
10298
}
10399

104100
bb6: {
101+
StorageLive(_12);
105102
_12 = move ((_4 as Err).0: i32);
106103
StorageLive(_13);
107104
_13 = Result::<Infallible, i32>::Err(copy _12);
108105
_3 = ControlFlow::<Result<Infallible, i32>, i32>::Break(move _13);
109106
StorageDead(_13);
107+
StorageDead(_12);
110108
- goto -> bb5;
111109
+ goto -> bb8;
112110
}
113111

114112
bb7: {
113+
StorageLive(_11);
115114
_11 = move ((_4 as Ok).0: i32);
116115
_3 = ControlFlow::<Result<Infallible, i32>, i32>::Continue(copy _11);
116+
StorageDead(_11);
117117
goto -> bb5;
118118
+ }
119119
+
120120
+ bb8: {
121-
+ StorageDead(_12);
122-
+ StorageDead(_11);
123121
+ StorageDead(_10);
124122
+ StorageDead(_4);
125123
+ _5 = discriminant(_3);

‎tests/mir-opt/pre-codegen/derived_ord.demo_le.PreCodegen.after.mir

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ fn demo_le(_1: &MultiField, _2: &MultiField) -> bool {
3737
}
3838

3939
bb0: {
40-
StorageLive(_12);
4140
StorageLive(_11);
4241
StorageLive(_5);
4342
StorageLive(_6);
@@ -80,13 +79,14 @@ fn demo_le(_1: &MultiField, _2: &MultiField) -> bool {
8079
}
8180

8281
bb3: {
82+
StorageLive(_12);
8383
_12 = move ((_11 as Some).0: std::cmp::Ordering);
8484
StorageLive(_13);
8585
_13 = discriminant(_12);
8686
_0 = Le(move _13, const 0_i8);
8787
StorageDead(_13);
88-
StorageDead(_11);
8988
StorageDead(_12);
89+
StorageDead(_11);
9090
return;
9191
}
9292
}

‎tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.32bit.panic-abort.diff

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,21 +84,20 @@
8484
StorageDead(_8);
8585
StorageDead(_7);
8686
StorageLive(_12);
87-
StorageLive(_16);
8887
_12 = discriminant(_6);
8988
switchInt(move _12) -> [0: bb6, 1: bb5, otherwise: bb1];
9089
}
9190

9291
bb5: {
9392
StorageLive(_15);
93+
StorageLive(_16);
9494
_16 = &_13;
9595
_15 = copy _16 as &dyn std::fmt::Debug (PointerCoercion(Unsize, Implicit));
9696
_14 = result::unwrap_failed(const "called `Result::unwrap()` on an `Err` value", move _15) -> unwind unreachable;
9797
}
9898

9999
bb6: {
100100
_5 = move ((_6 as Ok).0: std::ptr::NonNull<[u8]>);
101-
StorageDead(_16);
102101
StorageDead(_12);
103102
StorageDead(_6);
104103
_4 = copy _5 as *mut [u8] (Transmute);

‎tests/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.64bit.panic-abort.diff

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,21 +84,20 @@
8484
StorageDead(_8);
8585
StorageDead(_7);
8686
StorageLive(_12);
87-
StorageLive(_16);
8887
_12 = discriminant(_6);
8988
switchInt(move _12) -> [0: bb6, 1: bb5, otherwise: bb1];
9089
}
9190

9291
bb5: {
9392
StorageLive(_15);
93+
StorageLive(_16);
9494
_16 = &_13;
9595
_15 = copy _16 as &dyn std::fmt::Debug (PointerCoercion(Unsize, Implicit));
9696
_14 = result::unwrap_failed(const "called `Result::unwrap()` on an `Err` value", move _15) -> unwind unreachable;
9797
}
9898

9999
bb6: {
100100
_5 = move ((_6 as Ok).0: std::ptr::NonNull<[u8]>);
101-
StorageDead(_16);
102101
StorageDead(_12);
103102
StorageDead(_6);
104103
_4 = copy _5 as *mut [u8] (Transmute);

0 commit comments

Comments
 (0)