Skip to content

Convert moves of references to copies in ReferencePropagation #142185

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Convert moves of references to copies in ReferencePropagation #142185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
39 changes: 36 additions & 3 deletions compiler/rustc_mir_transform/src/ref_prop.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ impl<'tcx> crate::MirPass<'tcx> for ReferencePropagation {
#[instrument(level = "trace", skip(self, tcx, body))]
fn run_pass(&self, tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
debug!(def_id = ?body.source.def_id());
move_to_copy_pointers(tcx, body);
while propagate_ssa(tcx, body) {}
}

Expand All @@ -87,11 +88,43 @@ impl<'tcx> crate::MirPass<'tcx> for ReferencePropagation {
}
}

/// The SSA analysis done by [`SsaLocals`] treats [`Operand::Move`] as a read, even though in
/// general [`Operand::Move`] represents pass-by-pointer where the callee can overwrite the
/// pointee (Miri always considers the place deinitialized). CopyProp has a similar trick to
/// turn [`Operand::Move`] into [`Operand::Copy`] when required for an optimization, but in this
/// pass we just turn all moves of pointers into copies because pointers should be by-value anyway.
fn move_to_copy_pointers<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a doc-comment explaining why we need to transform those moves? A summary of your PR comment for instance?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doc comment added!

let mut visitor = MoveToCopyVisitor { tcx, local_decls: &body.local_decls };
for (bb, data) in body.basic_blocks.as_mut_preserves_cfg().iter_enumerated_mut() {
visitor.visit_basic_block_data(bb, data);
}

struct MoveToCopyVisitor<'a, 'tcx> {
tcx: TyCtxt<'tcx>,
local_decls: &'a IndexVec<Local, LocalDecl<'tcx>>,
}

impl<'a, 'tcx> MutVisitor<'tcx> for MoveToCopyVisitor<'a, 'tcx> {
fn tcx(&self) -> TyCtxt<'tcx> {
self.tcx
}

fn visit_operand(&mut self, operand: &mut Operand<'tcx>, loc: Location) {
if let Operand::Move(place) = *operand {
if place.ty(self.local_decls, self.tcx).ty.is_any_ptr() {
*operand = Operand::Copy(place);
}
}
self.super_operand(operand, loc);
}
}
}

fn propagate_ssa<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>) -> bool {
let typing_env = body.typing_env(tcx);
let ssa = SsaLocals::new(tcx, body, typing_env);

let mut replacer = compute_replacement(tcx, body, &ssa);
let mut replacer = compute_replacement(tcx, body, ssa);
debug!(?replacer.targets);
debug!(?replacer.allowed_replacements);
debug!(?replacer.storage_to_remove);
Expand Down Expand Up @@ -119,7 +152,7 @@ enum Value<'tcx> {
fn compute_replacement<'tcx>(
tcx: TyCtxt<'tcx>,
body: &Body<'tcx>,
ssa: &SsaLocals,
ssa: SsaLocals,
) -> Replacer<'tcx> {
let always_live_locals = always_storage_live_locals(body);

Expand All @@ -138,7 +171,7 @@ fn compute_replacement<'tcx>(
// reborrowed references.
let mut storage_to_remove = DenseBitSet::new_empty(body.local_decls.len());

let fully_replaceable_locals = fully_replaceable_locals(ssa);
let fully_replaceable_locals = fully_replaceable_locals(&ssa);

// Returns true iff we can use `place` as a pointee.
//
Expand Down
2 changes: 1 addition & 1 deletion tests/mir-opt/building/index_array_and_slice.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ struct WithSliceTail(f64, [i32]);
fn index_custom(custom: &WithSliceTail, index: usize) -> &i32 {
// CHECK: bb0:
// CHECK: [[PTR:_.+]] = &raw const (fake) ((*_1).1: [i32]);
// CHECK: [[LEN:_.+]] = PtrMetadata(move [[PTR]]);
// CHECK: [[LEN:_.+]] = PtrMetadata(copy [[PTR]]);
// CHECK: [[LT:_.+]] = Lt(copy _2, copy [[LEN]]);
// CHECK: assert(move [[LT]], "index out of bounds{{.+}}", move [[LEN]], copy _2) -> [success: bb1,

Expand Down
2 changes: 1 addition & 1 deletion tests/mir-opt/dont_reset_cast_kind_without_updating_operand.test.GVN.32bit.panic-abort.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@
StorageDead(_17);
StorageDead(_16);
StorageDead(_14);
_3 = ShallowInitBox(move _13, ());
_3 = ShallowInitBox(copy _13, ());
StorageDead(_13);
StorageDead(_12);
StorageDead(_11);
Expand Down
2 changes: 1 addition & 1 deletion tests/mir-opt/dont_reset_cast_kind_without_updating_operand.test.GVN.64bit.panic-abort.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@
StorageDead(_17);
StorageDead(_16);
StorageDead(_14);
_3 = ShallowInitBox(move _13, ());
_3 = ShallowInitBox(copy _13, ());
StorageDead(_13);
StorageDead(_12);
StorageDead(_11);
Expand Down
2 changes: 1 addition & 1 deletion ...s/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.32bit.panic-abort.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@
StorageDead(_6);
_4 = copy _5 as *mut [u8] (Transmute);
StorageDead(_5);
_3 = move _4 as *mut u8 (PtrToPtr);
_3 = copy _4 as *mut u8 (PtrToPtr);
StorageDead(_4);
StorageDead(_3);
- StorageDead(_1);
Expand Down
2 changes: 1 addition & 1 deletion .../mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.32bit.panic-unwind.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
StorageDead(_6);
_4 = copy _5 as *mut [u8] (Transmute);
StorageDead(_5);
_3 = move _4 as *mut u8 (PtrToPtr);
_3 = copy _4 as *mut u8 (PtrToPtr);
StorageDead(_4);
StorageDead(_3);
- StorageDead(_1);
Expand Down
2 changes: 1 addition & 1 deletion ...s/mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.64bit.panic-abort.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@
StorageDead(_6);
_4 = copy _5 as *mut [u8] (Transmute);
StorageDead(_5);
_3 = move _4 as *mut u8 (PtrToPtr);
_3 = copy _4 as *mut u8 (PtrToPtr);
StorageDead(_4);
StorageDead(_3);
- StorageDead(_1);
Expand Down
2 changes: 1 addition & 1 deletion .../mir-opt/pre-codegen/issue_117368_print_invalid_constant.main.GVN.64bit.panic-unwind.diff
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
StorageDead(_6);
_4 = copy _5 as *mut [u8] (Transmute);
StorageDead(_5);
_3 = move _4 as *mut u8 (PtrToPtr);
_3 = copy _4 as *mut u8 (PtrToPtr);
StorageDead(_4);
StorageDead(_3);
- StorageDead(_1);
Expand Down
2 changes: 1 addition & 1 deletion tests/mir-opt/pre-codegen/slice_index.slice_get_mut_usize.PreCodegen.after.panic-abort.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ fn slice_get_mut_usize(_1: &mut [u32], _2: usize) -> Option<&mut u32> {
StorageDead(_3);
StorageLive(_5);
_5 = &mut (*_1)[_2];
_0 = Option::<&mut u32>::Some(move _5);
_0 = Option::<&mut u32>::Some(copy _5);
StorageDead(_5);
goto -> bb3;
}
Expand Down
2 changes: 1 addition & 1 deletion tests/mir-opt/pre-codegen/slice_index.slice_get_mut_usize.PreCodegen.after.panic-unwind.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ fn slice_get_mut_usize(_1: &mut [u32], _2: usize) -> Option<&mut u32> {
StorageDead(_3);
StorageLive(_5);
_5 = &mut (*_1)[_2];
_0 = Option::<&mut u32>::Some(move _5);
_0 = Option::<&mut u32>::Some(copy _5);
StorageDead(_5);
goto -> bb3;
}
Expand Down
12 changes: 6 additions & 6 deletions tests/mir-opt/pre-codegen/slice_iter.enumerated_loop.PreCodegen.after.panic-abort.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () {
_4 = &raw const (*_1);
StorageLive(_5);
_5 = copy _4 as *const T (PtrToPtr);
_6 = NonNull::<T> { pointer: move _5 };
_6 = NonNull::<T> { pointer: copy _5 };
StorageDead(_5);
StorageLive(_9);
switchInt(const <T as std::mem::SizedTypeProperties>::IS_ZST) -> [0: bb1, otherwise: bb2];
Expand All @@ -155,7 +155,7 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () {
_7 = copy _4 as *mut T (PtrToPtr);
_8 = Offset(copy _7, copy _3);
StorageDead(_7);
_9 = move _8 as *const T (PtrToPtr);
_9 = copy _8 as *const T (PtrToPtr);
StorageDead(_8);
goto -> bb3;
}
Expand Down Expand Up @@ -202,7 +202,7 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () {
_17 = copy _14 as *mut T (Transmute);
StorageLive(_18);
_18 = copy _16 as *mut T (Transmute);
_19 = Eq(move _17, move _18);
_19 = Eq(copy _17, copy _18);
StorageDead(_18);
StorageDead(_17);
switchInt(move _19) -> [0: bb6, otherwise: bb7];
Expand All @@ -214,9 +214,9 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () {
StorageLive(_21);
StorageLive(_20);
_20 = copy _14 as *const T (Transmute);
_21 = Offset(move _20, const 1_usize);
_21 = Offset(copy _20, const 1_usize);
StorageDead(_20);
_22 = NonNull::<T> { pointer: move _21 };
_22 = NonNull::<T> { pointer: copy _21 };
StorageDead(_21);
_11 = move _22;
StorageDead(_22);
Expand Down Expand Up @@ -282,7 +282,7 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () {
StorageDead(_23);
StorageDead(_15);
StorageDead(_14);
_28 = move ((_27 as Some).0: &T);
_28 = copy ((_27 as Some).0: &T);
StorageDead(_27);
_29 = copy _13;
_30 = AddWithOverflow(copy _13, const 1_usize);
Expand Down
6 changes: 3 additions & 3 deletions tests/mir-opt/pre-codegen/slice_iter.enumerated_loop.PreCodegen.after.panic-unwind.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () {
_4 = &raw const (*_1);
StorageLive(_5);
_5 = copy _4 as *const T (PtrToPtr);
_6 = NonNull::<T> { pointer: move _5 };
_6 = NonNull::<T> { pointer: copy _5 };
StorageDead(_5);
StorageLive(_9);
switchInt(const <T as std::mem::SizedTypeProperties>::IS_ZST) -> [0: bb1, otherwise: bb2];
Expand All @@ -82,7 +82,7 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () {
_7 = copy _4 as *mut T (PtrToPtr);
_8 = Offset(copy _7, copy _3);
StorageDead(_7);
_9 = move _8 as *const T (PtrToPtr);
_9 = copy _8 as *const T (PtrToPtr);
StorageDead(_8);
goto -> bb3;
}
Expand All @@ -95,7 +95,7 @@ fn enumerated_loop(_1: &[T], _2: impl Fn(usize, &T)) -> () {
bb3: {
StorageLive(_10);
_10 = copy _9;
_11 = std::slice::Iter::<'_, T> { ptr: copy _6, end_or_len: move _10, _marker: const ZeroSized: PhantomData<&T> };
_11 = std::slice::Iter::<'_, T> { ptr: copy _6, end_or_len: copy _10, _marker: const ZeroSized: PhantomData<&T> };
StorageDead(_10);
StorageDead(_9);
StorageDead(_4);
Expand Down
10 changes: 5 additions & 5 deletions tests/mir-opt/pre-codegen/slice_iter.forward_loop.PreCodegen.after.panic-abort.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ fn forward_loop(_1: &[T], _2: impl Fn(&T)) -> () {
_4 = &raw const (*_1);
StorageLive(_5);
_5 = copy _4 as *const T (PtrToPtr);
_6 = NonNull::<T> { pointer: move _5 };
_6 = NonNull::<T> { pointer: copy _5 };
StorageDead(_5);
StorageLive(_9);
switchInt(const <T as std::mem::SizedTypeProperties>::IS_ZST) -> [0: bb1, otherwise: bb2];
Expand All @@ -122,7 +122,7 @@ fn forward_loop(_1: &[T], _2: impl Fn(&T)) -> () {
_7 = copy _4 as *mut T (PtrToPtr);
_8 = Offset(copy _7, copy _3);
StorageDead(_7);
_9 = move _8 as *const T (PtrToPtr);
_9 = copy _8 as *const T (PtrToPtr);
StorageDead(_8);
goto -> bb3;
}
Expand Down Expand Up @@ -164,7 +164,7 @@ fn forward_loop(_1: &[T], _2: impl Fn(&T)) -> () {
_16 = copy _13 as *mut T (Transmute);
StorageLive(_17);
_17 = copy _15 as *mut T (Transmute);
_18 = Eq(move _16, move _17);
_18 = Eq(copy _16, copy _17);
StorageDead(_17);
StorageDead(_16);
switchInt(move _18) -> [0: bb6, otherwise: bb7];
Expand All @@ -176,9 +176,9 @@ fn forward_loop(_1: &[T], _2: impl Fn(&T)) -> () {
StorageLive(_20);
StorageLive(_19);
_19 = copy _13 as *const T (Transmute);
_20 = Offset(move _19, const 1_usize);
_20 = Offset(copy _19, const 1_usize);
StorageDead(_19);
_21 = NonNull::<T> { pointer: move _20 };
_21 = NonNull::<T> { pointer: copy _20 };
StorageDead(_20);
_11 = move _21;
StorageDead(_21);
Expand Down
10 changes: 5 additions & 5 deletions tests/mir-opt/pre-codegen/slice_iter.forward_loop.PreCodegen.after.panic-unwind.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ fn forward_loop(_1: &[T], _2: impl Fn(&T)) -> () {
_4 = &raw const (*_1);
StorageLive(_5);
_5 = copy _4 as *const T (PtrToPtr);
_6 = NonNull::<T> { pointer: move _5 };
_6 = NonNull::<T> { pointer: copy _5 };
StorageDead(_5);
StorageLive(_9);
switchInt(const <T as std::mem::SizedTypeProperties>::IS_ZST) -> [0: bb1, otherwise: bb2];
Expand All @@ -122,7 +122,7 @@ fn forward_loop(_1: &[T], _2: impl Fn(&T)) -> () {
_7 = copy _4 as *mut T (PtrToPtr);
_8 = Offset(copy _7, copy _3);
StorageDead(_7);
_9 = move _8 as *const T (PtrToPtr);
_9 = copy _8 as *const T (PtrToPtr);
StorageDead(_8);
goto -> bb3;
}
Expand Down Expand Up @@ -164,7 +164,7 @@ fn forward_loop(_1: &[T], _2: impl Fn(&T)) -> () {
_16 = copy _13 as *mut T (Transmute);
StorageLive(_17);
_17 = copy _15 as *mut T (Transmute);
_18 = Eq(move _16, move _17);
_18 = Eq(copy _16, copy _17);
StorageDead(_17);
StorageDead(_16);
switchInt(move _18) -> [0: bb6, otherwise: bb7];
Expand All @@ -176,9 +176,9 @@ fn forward_loop(_1: &[T], _2: impl Fn(&T)) -> () {
StorageLive(_20);
StorageLive(_19);
_19 = copy _13 as *const T (Transmute);
_20 = Offset(move _19, const 1_usize);
_20 = Offset(copy _19, const 1_usize);
StorageDead(_19);
_21 = NonNull::<T> { pointer: move _20 };
_21 = NonNull::<T> { pointer: copy _20 };
StorageDead(_20);
_11 = move _21;
StorageDead(_21);
Expand Down
6 changes: 3 additions & 3 deletions tests/mir-opt/pre-codegen/slice_iter.reverse_loop.PreCodegen.after.panic-abort.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ fn reverse_loop(_1: &[T], _2: impl Fn(&T)) -> () {
_4 = &raw const (*_1);
StorageLive(_5);
_5 = copy _4 as *const T (PtrToPtr);
_6 = NonNull::<T> { pointer: move _5 };
_6 = NonNull::<T> { pointer: copy _5 };
StorageDead(_5);
StorageLive(_9);
switchInt(const <T as std::mem::SizedTypeProperties>::IS_ZST) -> [0: bb1, otherwise: bb2];
Expand All @@ -82,7 +82,7 @@ fn reverse_loop(_1: &[T], _2: impl Fn(&T)) -> () {
_7 = copy _4 as *mut T (PtrToPtr);
_8 = Offset(copy _7, copy _3);
StorageDead(_7);
_9 = move _8 as *const T (PtrToPtr);
_9 = copy _8 as *const T (PtrToPtr);
StorageDead(_8);
goto -> bb3;
}
Expand All @@ -95,7 +95,7 @@ fn reverse_loop(_1: &[T], _2: impl Fn(&T)) -> () {
bb3: {
StorageLive(_10);
_10 = copy _9;
_11 = std::slice::Iter::<'_, T> { ptr: copy _6, end_or_len: move _10, _marker: const ZeroSized: PhantomData<&T> };
_11 = std::slice::Iter::<'_, T> { ptr: copy _6, end_or_len: copy _10, _marker: const ZeroSized: PhantomData<&T> };
StorageDead(_10);
StorageDead(_9);
StorageDead(_4);
Expand Down
6 changes: 3 additions & 3 deletions tests/mir-opt/pre-codegen/slice_iter.reverse_loop.PreCodegen.after.panic-unwind.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ fn reverse_loop(_1: &[T], _2: impl Fn(&T)) -> () {
_4 = &raw const (*_1);
StorageLive(_5);
_5 = copy _4 as *const T (PtrToPtr);
_6 = NonNull::<T> { pointer: move _5 };
_6 = NonNull::<T> { pointer: copy _5 };
StorageDead(_5);
StorageLive(_9);
switchInt(const <T as std::mem::SizedTypeProperties>::IS_ZST) -> [0: bb1, otherwise: bb2];
Expand All @@ -82,7 +82,7 @@ fn reverse_loop(_1: &[T], _2: impl Fn(&T)) -> () {
_7 = copy _4 as *mut T (PtrToPtr);
_8 = Offset(copy _7, copy _3);
StorageDead(_7);
_9 = move _8 as *const T (PtrToPtr);
_9 = copy _8 as *const T (PtrToPtr);
StorageDead(_8);
goto -> bb3;
}
Expand All @@ -95,7 +95,7 @@ fn reverse_loop(_1: &[T], _2: impl Fn(&T)) -> () {
bb3: {
StorageLive(_10);
_10 = copy _9;
_11 = std::slice::Iter::<'_, T> { ptr: copy _6, end_or_len: move _10, _marker: const ZeroSized: PhantomData<&T> };
_11 = std::slice::Iter::<'_, T> { ptr: copy _6, end_or_len: copy _10, _marker: const ZeroSized: PhantomData<&T> };
StorageDead(_10);
StorageDead(_9);
StorageDead(_4);
Expand Down
4 changes: 2 additions & 2 deletions ...r-opt/pre-codegen/slice_iter.slice_iter_generic_is_empty.PreCodegen.after.panic-abort.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ fn slice_iter_generic_is_empty(_1: &std::slice::Iter<'_, T>) -> bool {
bb1: {
StorageLive(_2);
_2 = copy ((*_1).1: *const T);
_3 = move _2 as std::ptr::NonNull<T> (Transmute);
_3 = copy _2 as std::ptr::NonNull<T> (Transmute);
StorageDead(_2);
StorageLive(_5);
StorageLive(_4);
Expand All @@ -48,7 +48,7 @@ fn slice_iter_generic_is_empty(_1: &std::slice::Iter<'_, T>) -> bool {
StorageDead(_4);
StorageLive(_6);
_6 = copy _3 as *mut T (Transmute);
_0 = Eq(move _5, move _6);
_0 = Eq(copy _5, copy _6);
StorageDead(_6);
StorageDead(_5);
goto -> bb3;
Expand Down
4 changes: 2 additions & 2 deletions ...-opt/pre-codegen/slice_iter.slice_iter_generic_is_empty.PreCodegen.after.panic-unwind.mir
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ fn slice_iter_generic_is_empty(_1: &std::slice::Iter<'_, T>) -> bool {
bb1: {
StorageLive(_2);
_2 = copy ((*_1).1: *const T);
_3 = move _2 as std::ptr::NonNull<T> (Transmute);
_3 = copy _2 as std::ptr::NonNull<T> (Transmute);
StorageDead(_2);
StorageLive(_5);
StorageLive(_4);
Expand All @@ -48,7 +48,7 @@ fn slice_iter_generic_is_empty(_1: &std::slice::Iter<'_, T>) -> bool {
StorageDead(_4);
StorageLive(_6);
_6 = copy _3 as *mut T (Transmute);
_0 = Eq(move _5, move _6);
_0 = Eq(copy _5, copy _6);
StorageDead(_6);
StorageDead(_5);
goto -> bb3;
Expand Down
Loading
Loading