Skip to content

Commit f58c9a8

Browse files
tmiaskotmiasko
committed
Turns moves into copies after copy propagation
Previously copy propagation presumed that there is further unspecified distinction between move operands and copy operands in assignments and propagated moves from assignments into terminators. This is inconsistent with current operational semantics. Turn moves into copies after copy propagation to preserve existing behavior. Fixes #137936. Fixes #146423.
1 parent 28c4c7d commit f58c9a8

File tree

86 files changed

+261
-321
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

86 files changed

+261
-321
lines changed

‎compiler/rustc_mir_transform/src/copy_prop.rs‎

Lines changed: 1 addition & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,7 @@ impl<'tcx> crate::MirPass<'tcx> for CopyProp {
4646
return;
4747
}
4848

49-
let fully_moved = fully_moved_locals(&ssa, body);
50-
debug!(?fully_moved);
51-
52-
Replacer { tcx, copy_classes: ssa.copy_classes(), fully_moved, storage_to_remove }
49+
Replacer { tcx, copy_classes: ssa.copy_classes(), storage_to_remove }
5350
.visit_body_preserves_cfg(body);
5451

5552
crate::simplify::remove_unused_definitions(body);
@@ -60,43 +57,9 @@ impl<'tcx> crate::MirPass<'tcx> for CopyProp {
6057
}
6158
}
6259

63-
/// `SsaLocals` computed equivalence classes between locals considering copy/move assignments.
64-
///
65-
/// This function also returns whether all the `move?` in the pattern are `move` and not copies.
66-
/// A local which is in the bitset can be replaced by `move _a`. Otherwise, it must be
67-
/// replaced by `copy _a`, as we cannot move multiple times from `_a`.
68-
///
69-
/// If an operand copies `_c`, it must happen before the assignment `_d = _c`, otherwise it is UB.
70-
/// This means that replacing it by a copy of `_a` if ok, since this copy happens before `_c` is
71-
/// moved, and therefore that `_d` is moved.
72-
#[instrument(level = "trace", skip(ssa, body))]
73-
fn fully_moved_locals(ssa: &SsaLocals, body: &Body<'_>) -> DenseBitSet<Local> {
74-
let mut fully_moved = DenseBitSet::new_filled(body.local_decls.len());
75-
76-
for (_, rvalue, _) in ssa.assignments(body) {
77-
let Rvalue::Use(Operand::Copy(place) | Operand::Move(place)) = rvalue else {
78-
continue;
79-
};
80-
81-
let Some(rhs) = place.as_local() else { continue };
82-
if !ssa.is_ssa(rhs) {
83-
continue;
84-
}
85-
86-
if let Rvalue::Use(Operand::Copy(_)) = rvalue {
87-
fully_moved.remove(rhs);
88-
}
89-
}
90-
91-
ssa.meet_copy_equivalence(&mut fully_moved);
92-
93-
fully_moved
94-
}
95-
9660
/// Utility to help performing substitution of `*pattern` by `target`.
9761
struct Replacer<'a, 'tcx> {
9862
tcx: TyCtxt<'tcx>,
99-
fully_moved: DenseBitSet<Local>,
10063
storage_to_remove: DenseBitSet<Local>,
10164
copy_classes: &'a IndexSlice<Local, Local>,
10265
}
@@ -123,7 +86,6 @@ impl<'tcx> MutVisitor<'tcx> for Replacer<'_, 'tcx> {
12386
// A move out of a projection of a copy is equivalent to a copy of the original
12487
// projection.
12588
&& !place.is_indirect_first_projection()
126-
&& !self.fully_moved.contains(place.local)
12789
{
12890
*operand = Operand::Copy(place);
12991
}

‎tests/mir-opt/copy-prop/borrowed_local.borrow_in_loop.CopyProp.panic-abort.diff‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,14 @@
4646
- StorageLive(_6);
4747
StorageLive(_7);
4848
_7 = copy (*_2);
49-
_6 = Not(move _7);
49+
- _6 = Not(move _7);
50+
+ _6 = Not(copy _7);
5051
StorageDead(_7);
5152
- StorageLive(_8);
5253
StorageLive(_9);
5354
_9 = copy (*_2);
54-
_8 = Not(move _9);
55+
- _8 = Not(move _9);
56+
+ _8 = Not(copy _9);
5557
StorageDead(_9);
5658
- StorageLive(_10);
5759
- _10 = copy _6;
@@ -62,7 +64,8 @@
6264
StorageLive(_12);
6365
_12 = &_1;
6466
_11 = &(*_12);
65-
_2 = move _11;
67+
- _2 = move _11;
68+
+ _2 = copy _11;
6669
StorageDead(_11);
6770
StorageDead(_12);
6871
StorageLive(_13);
@@ -71,8 +74,9 @@
7174
- StorageLive(_15);
7275
- _15 = copy _8;
7376
- _13 = Ne(move _14, move _15);
77+
- switchInt(move _13) -> [0: bb3, otherwise: bb2];
7478
+ _13 = Ne(copy _6, copy _8);
75-
switchInt(move _13) -> [0: bb3, otherwise: bb2];
79+
+ switchInt(copy _13) -> [0: bb3, otherwise: bb2];
7680
}
7781

7882
bb2: {

‎tests/mir-opt/copy-prop/borrowed_local.borrow_in_loop.CopyProp.panic-unwind.diff‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,14 @@
4646
- StorageLive(_6);
4747
StorageLive(_7);
4848
_7 = copy (*_2);
49-
_6 = Not(move _7);
49+
- _6 = Not(move _7);
50+
+ _6 = Not(copy _7);
5051
StorageDead(_7);
5152
- StorageLive(_8);
5253
StorageLive(_9);
5354
_9 = copy (*_2);
54-
_8 = Not(move _9);
55+
- _8 = Not(move _9);
56+
+ _8 = Not(copy _9);
5557
StorageDead(_9);
5658
- StorageLive(_10);
5759
- _10 = copy _6;
@@ -62,7 +64,8 @@
6264
StorageLive(_12);
6365
_12 = &_1;
6466
_11 = &(*_12);
65-
_2 = move _11;
67+
- _2 = move _11;
68+
+ _2 = copy _11;
6669
StorageDead(_11);
6770
StorageDead(_12);
6871
StorageLive(_13);
@@ -71,8 +74,9 @@
7174
- StorageLive(_15);
7275
- _15 = copy _8;
7376
- _13 = Ne(move _14, move _15);
77+
- switchInt(move _13) -> [0: bb3, otherwise: bb2];
7478
+ _13 = Ne(copy _6, copy _8);
75-
switchInt(move _13) -> [0: bb3, otherwise: bb2];
79+
+ switchInt(copy _13) -> [0: bb3, otherwise: bb2];
7680
}
7781

7882
bb2: {

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@
3838
StorageLive(_5);
3939
StorageLive(_6);
4040
_6 = copy _1;
41-
_5 = std::mem::drop::<i32>(move _6) -> [return: bb2, unwind unreachable];
41+
- _5 = std::mem::drop::<i32>(move _6) -> [return: bb2, unwind unreachable];
42+
+ _5 = std::mem::drop::<i32>(copy _6) -> [return: bb2, unwind unreachable];
4243
}
4344

4445
bb2: {

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@
3838
StorageLive(_5);
3939
StorageLive(_6);
4040
_6 = copy _1;
41-
_5 = std::mem::drop::<i32>(move _6) -> [return: bb2, unwind continue];
41+
- _5 = std::mem::drop::<i32>(move _6) -> [return: bb2, unwind continue];
42+
+ _5 = std::mem::drop::<i32>(copy _6) -> [return: bb2, unwind continue];
4243
}
4344

4445
bb2: {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ fn f(_1: usize) -> usize {
1616
_1 = copy _2;
1717
StorageLive(_4);
1818
_4 = copy _1;
19-
_0 = id::<usize>(move _4) -> [return: bb1, unwind unreachable];
19+
_0 = id::<usize>(copy _4) -> [return: bb1, unwind unreachable];
2020
}
2121

2222
bb1: {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ fn f(_1: usize) -> usize {
1616
_1 = copy _2;
1717
StorageLive(_4);
1818
_4 = copy _1;
19-
_0 = id::<usize>(move _4) -> [return: bb1, unwind continue];
19+
_0 = id::<usize>(copy _4) -> [return: bb1, unwind continue];
2020
}
2121

2222
bb1: {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ fn f(_1: usize) -> usize {
1616
_1 = copy _2;
1717
StorageLive(_4);
1818
_4 = copy _1;
19-
_0 = id::<usize>(move _4) -> [return: bb1, unwind unreachable];
19+
_0 = id::<usize>(copy _4) -> [return: bb1, unwind unreachable];
2020
}
2121

2222
bb1: {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ fn f(_1: usize) -> usize {
1616
_1 = copy _2;
1717
StorageLive(_4);
1818
_4 = copy _1;
19-
_0 = id::<usize>(move _4) -> [return: bb1, unwind continue];
19+
_0 = id::<usize>(copy _4) -> [return: bb1, unwind continue];
2020
}
2121

2222
bb1: {

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

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -46,22 +46,27 @@
4646
StorageLive(_6);
4747
StorageLive(_7);
4848
_7 = &_2;
49-
_6 = move _7 as &[i32] (PointerCoercion(Unsize, Implicit));
49+
- _6 = move _7 as &[i32] (PointerCoercion(Unsize, Implicit));
50+
+ _6 = copy _7 as &[i32] (PointerCoercion(Unsize, Implicit));
5051
StorageDead(_7);
51-
_5 = core::slice::<impl [i32]>::len(move _6) -> [return: bb1, unwind unreachable];
52+
- _5 = core::slice::<impl [i32]>::len(move _6) -> [return: bb1, unwind unreachable];
53+
+ _5 = core::slice::<impl [i32]>::len(copy _6) -> [return: bb1, unwind unreachable];
5254
}
5355

5456
bb1: {
5557
StorageDead(_6);
56-
_4 = std::ops::Range::<usize> { start: const 0_usize, end: move _5 };
58+
- _4 = std::ops::Range::<usize> { start: const 0_usize, end: move _5 };
59+
+ _4 = std::ops::Range::<usize> { start: const 0_usize, end: copy _5 };
5760
StorageDead(_5);
58-
_3 = <std::ops::Range<usize> as IntoIterator>::into_iter(move _4) -> [return: bb2, unwind unreachable];
61+
- _3 = <std::ops::Range<usize> as IntoIterator>::into_iter(move _4) -> [return: bb2, unwind unreachable];
62+
+ _3 = <std::ops::Range<usize> as IntoIterator>::into_iter(copy _4) -> [return: bb2, unwind unreachable];
5963
}
6064

6165
bb2: {
6266
StorageDead(_4);
6367
StorageLive(_8);
64-
_8 = move _3;
68+
- _8 = move _3;
69+
+ _8 = copy _3;
6570
goto -> bb3;
6671
}
6772

@@ -72,13 +77,15 @@
7277
StorageLive(_13);
7378
_13 = &mut _8;
7479
_12 = &mut (*_13);
75-
_11 = <std::ops::Range<usize> as Iterator>::next(move _12) -> [return: bb4, unwind unreachable];
80+
- _11 = <std::ops::Range<usize> as Iterator>::next(move _12) -> [return: bb4, unwind unreachable];
81+
+ _11 = <std::ops::Range<usize> as Iterator>::next(copy _12) -> [return: bb4, unwind unreachable];
7682
}
7783

7884
bb4: {
7985
StorageDead(_12);
8086
_14 = discriminant(_11);
81-
switchInt(move _14) -> [0: bb7, 1: bb6, otherwise: bb5];
87+
- switchInt(move _14) -> [0: bb7, 1: bb6, otherwise: bb5];
88+
+ switchInt(copy _14) -> [0: bb7, 1: bb6, otherwise: bb5];
8289
}
8390

8491
bb5: {
@@ -94,7 +101,7 @@
94101
- _19 = Lt(copy _18, const 4_usize);
95102
- assert(move _19, "index out of bounds: the length is {} but the index is {}", const 4_usize, copy _18) -> [success: bb8, unwind unreachable];
96103
+ _19 = Lt(copy _16, const 4_usize);
97-
+ assert(move _19, "index out of bounds: the length is {} but the index is {}", const 4_usize, copy _16) -> [success: bb8, unwind unreachable];
104+
+ assert(copy _19, "index out of bounds: the length is {} but the index is {}", const 4_usize, copy _16) -> [success: bb8, unwind unreachable];
98105
}
99106

100107
bb7: {
@@ -111,8 +118,9 @@
111118

112119
bb8: {
113120
- _17 = copy _2[_18];
121+
- _1 = Add(copy _1, move _17);
114122
+ _17 = copy _2[_16];
115-
_1 = Add(copy _1, move _17);
123+
+ _1 = Add(copy _1, copy _17);
116124
StorageDead(_17);
117125
- StorageDead(_18);
118126
- _10 = const ();

0 commit comments

Comments
 (0)