user0/rust
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Turn moves into copies after copy propagation

Browse source 
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 https://github.com/rust-lang/rust/issues/137936.
Fixes https://github.com/rust-lang/rust/issues/146423.
This commit is contained in:
Tomasz Miąsko 2025-10-18 14:52:23 +02:00
parent 7281a3bc4b
commit 6bd1a031ab
13 changed files with 75 additions and 150 deletions
Download patch file Download diff file Expand all files Collapse all files

63
compiler/rustc_mir_transform/src/copy_prop.rs
View file

@ -16,7 +16,7 @@ use crate::ssa::SsaLocals;
/// _d = move? _c
/// where each of the locals is only assigned once.
///
/// We want to replace all those locals by `_a`, either copied or moved.
/// We want to replace all those locals by `copy _a`.
pub(super) struct CopyProp;
impl<'tcx> crate::MirPass<'tcx> for CopyProp {
@ -34,11 +34,13 @@ impl<'tcx> crate::MirPass<'tcx> for CopyProp {
debug!(copy_classes = ?ssa.copy_classes());
let mut any_replacement = false;
let mut storage_to_remove = DenseBitSet::new_empty(body.local_decls.len());
// Locals that participate in copy propagation either as a source or a destination.
let mut unified = DenseBitSet::new_empty(body.local_decls.len());
for (local, &head) in ssa.copy_classes().iter_enumerated() {
if local != head {
any_replacement = true;
storage_to_remove.insert(head);
unified.insert(head);
unified.insert(local);
}
}
@ -46,11 +48,7 @@ impl<'tcx> crate::MirPass<'tcx> for CopyProp {
return;
}
let fully_moved = fully_moved_locals(&ssa, body);
debug!(?fully_moved);
Replacer { tcx, copy_classes: ssa.copy_classes(), fully_moved, storage_to_remove }
.visit_body_preserves_cfg(body);
Replacer { tcx, copy_classes: ssa.copy_classes(), unified }.visit_body_preserves_cfg(body);
crate::simplify::remove_unused_definitions(body);
}
@ -60,44 +58,10 @@ impl<'tcx> crate::MirPass<'tcx> for CopyProp {
}
}
/// `SsaLocals` computed equivalence classes between locals considering copy/move assignments.
///
/// This function also returns whether all the `move?` in the pattern are `move` and not copies.
/// A local which is in the bitset can be replaced by `move _a`. Otherwise, it must be
/// replaced by `copy _a`, as we cannot move multiple times from `_a`.
///
/// If an operand copies `_c`, it must happen before the assignment `_d = _c`, otherwise it is UB.
/// This means that replacing it by a copy of `_a` if ok, since this copy happens before `_c` is
/// moved, and therefore that `_d` is moved.
#[instrument(level = "trace", skip(ssa, body))]
fn fully_moved_locals(ssa: &SsaLocals, body: &Body<'_>) -> DenseBitSet<Local> {
let mut fully_moved = DenseBitSet::new_filled(body.local_decls.len());
for (_, rvalue, _) in ssa.assignments(body) {
let Rvalue::Use(Operand::Copy(place) | Operand::Move(place)) = rvalue else {
continue;
};
let Some(rhs) = place.as_local() else { continue };
if !ssa.is_ssa(rhs) {
continue;
}
if let Rvalue::Use(Operand::Copy(_)) = rvalue {
fully_moved.remove(rhs);
}
}
ssa.meet_copy_equivalence(&mut fully_moved);
fully_moved
}
/// Utility to help performing substitution of `*pattern` by `target`.
struct Replacer<'a, 'tcx> {
tcx: TyCtxt<'tcx>,
fully_moved: DenseBitSet<Local>,
storage_to_remove: DenseBitSet<Local>,
unified: DenseBitSet<Local>,
copy_classes: &'a IndexSlice<Local, Local>,
}
@ -108,13 +72,7 @@ impl<'tcx> MutVisitor<'tcx> for Replacer<'_, 'tcx> {
#[tracing::instrument(level = "trace", skip(self))]
fn visit_local(&mut self, local: &mut Local, ctxt: PlaceContext, _: Location) {
let new_local = self.copy_classes[*local];
match ctxt {
// Do not modify the local in storage statements.
PlaceContext::NonUse(NonUseContext::StorageLive | NonUseContext::StorageDead) => {}
// We access the value.
_ => *local = new_local,
}
*local = self.copy_classes[*local];
}
#[tracing::instrument(level = "trace", skip(self))]
@ -123,7 +81,7 @@ impl<'tcx> MutVisitor<'tcx> for Replacer<'_, 'tcx> {
// A move out of a projection of a copy is equivalent to a copy of the original
// projection.
&& !place.is_indirect_first_projection()
&& !self.fully_moved.contains(place.local)
&& self.unified.contains(place.local)
{
*operand = Operand::Copy(place);
}
@ -134,10 +92,9 @@ impl<'tcx> MutVisitor<'tcx> for Replacer<'_, 'tcx> {
fn visit_statement(&mut self, stmt: &mut Statement<'tcx>, loc: Location) {
// When removing storage statements, we need to remove both (#107511).
if let StatementKind::StorageLive(l) | StatementKind::StorageDead(l) = stmt.kind
&& self.storage_to_remove.contains(l)
&& self.unified.contains(l)
{
stmt.make_nop(true);
return;
}
self.super_statement(stmt, loc);

2
tests/codegen-llvm/align-byval.rs
View file

@ -1,7 +1,7 @@
// ignore-tidy-linelength
//@ add-minicore
//@ revisions:m68k x86_64-linux x86_64-windows i686-linux i686-windows
//@ compile-flags: -Copt-level=1 -Cno-prepopulate-passes
//@[m68k] compile-flags: --target m68k-unknown-linux-gnu
//@[m68k] needs-llvm-components: m68k
//@[x86_64-linux] compile-flags: --target x86_64-unknown-linux-gnu

37
tests/mir-opt/copy-prop/move_arg.f.CopyProp.panic-abort.diff
View file

@ -1,37 +0,0 @@
- // MIR for `f` before CopyProp
+ // MIR for `f` after CopyProp
fn f(_1: T) -> () {
debug a => _1;
let mut _0: ();
let _2: T;
let _3: ();
let mut _4: T;
let mut _5: T;
scope 1 {
- debug b => _2;
+ debug b => _1;
}
bb0: {
- StorageLive(_2);
- _2 = copy _1;
StorageLive(_3);
- StorageLive(_4);
- _4 = copy _1;
- StorageLive(_5);
- _5 = copy _2;
- _3 = g::<T>(move _4, move _5) -> [return: bb1, unwind unreachable];
+ _3 = g::<T>(copy _1, copy _1) -> [return: bb1, unwind unreachable];
}
bb1: {
- StorageDead(_5);
- StorageDead(_4);
StorageDead(_3);
_0 = const ();
- StorageDead(_2);
return;
}
}

37
tests/mir-opt/copy-prop/move_arg.f.CopyProp.panic-unwind.diff
View file

@ -1,37 +0,0 @@
- // MIR for `f` before CopyProp
+ // MIR for `f` after CopyProp
fn f(_1: T) -> () {
debug a => _1;
let mut _0: ();
let _2: T;
let _3: ();
let mut _4: T;
let mut _5: T;
scope 1 {
- debug b => _2;
+ debug b => _1;
}
bb0: {
- StorageLive(_2);
- _2 = copy _1;
StorageLive(_3);
- StorageLive(_4);
- _4 = copy _1;
- StorageLive(_5);
- _5 = copy _2;
- _3 = g::<T>(move _4, move _5) -> [return: bb1, unwind continue];
+ _3 = g::<T>(copy _1, copy _1) -> [return: bb1, unwind continue];
}
bb1: {
- StorageDead(_5);
- StorageDead(_4);
StorageDead(_3);
_0 = const ();
- StorageDead(_2);
return;
}
}

54
tests/mir-opt/copy-prop/move_arg.rs
View file

@ -1,20 +1,46 @@
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
// Test that we do not move multiple times from the same local.
//@ test-mir-pass: CopyProp
//@ compile-flags: --crate-type=lib -Cpanic=abort
#![feature(custom_mir, core_intrinsics)]
use core::intrinsics::mir::*;
use core::mem::MaybeUninit;
extern crate core;
// EMIT_MIR move_arg.f.CopyProp.diff
pub fn f<T: Copy>(a: T) {
// CHECK-LABEL: fn f(
// CHECK: debug a => [[a:_.*]];
// CHECK: debug b => [[a]];
// CHECK: g::<T>(copy [[a]], copy [[a]])
let b = a;
g(a, b);
#[custom_mir(dialect = "runtime", phase = "initial")]
pub fn moved_and_copied<T: Copy>(_1: T) {
// CHECK-LABEL: fn moved_and_copied(
// CHECK: _0 = f::<T>(copy _1, copy _1)
mir! {
{
let _2 = _1;
Call(RET = f(Move(_1), Move(_2)), ReturnTo(bb1), UnwindUnreachable())
}
bb1 = {
Return()
}
}
}
#[custom_mir(dialect = "runtime", phase = "initial")]
pub fn moved_twice<T: Copy>(_1: MaybeUninit<T>) {
// In a future we would like to propagate moves instead of copies here. The resulting program
// would have an undefined behavior due to overlap in a call terminator, so we need to change
// operational semantics to explain why the original program has undefined behavior.
// https://github.com/rust-lang/unsafe-code-guidelines/issues/556
//
// CHECK-LABEL: fn moved_twice(
// CHECK: _0 = f::<MaybeUninit<T>>(copy _1, copy _1)
mir! {
{
let _2 = Move(_1);
let _3 = Move(_1);
Call(RET = f(Move(_2), Move(_3)), ReturnTo(bb1), UnwindUnreachable())
}
bb1 = {
Return()
}
}
}
#[inline(never)]
pub fn g<T: Copy>(_: T, _: T) {}
fn main() {
f(5)
}
pub fn f<T: Copy>(_: T, _: T) {}

2
tests/mir-opt/copy-prop/reborrow.remut.CopyProp.panic-abort.diff
View file

@ -31,7 +31,7 @@
- StorageLive(_6);
- _6 = move _4;
- _5 = opaque::<&mut u8>(move _6) -> [return: bb1, unwind unreachable];
+ _5 = opaque::<&mut u8>(move _2) -> [return: bb1, unwind unreachable];
+ _5 = opaque::<&mut u8>(copy _2) -> [return: bb1, unwind unreachable];
}
bb1: {

2
tests/mir-opt/copy-prop/reborrow.remut.CopyProp.panic-unwind.diff
View file

@ -31,7 +31,7 @@
- StorageLive(_6);
- _6 = move _4;
- _5 = opaque::<&mut u8>(move _6) -> [return: bb1, unwind continue];
+ _5 = opaque::<&mut u8>(move _2) -> [return: bb1, unwind continue];
+ _5 = opaque::<&mut u8>(copy _2) -> [return: bb1, unwind continue];
}
bb1: {

2
tests/mir-opt/copy-prop/reborrow.reraw.CopyProp.panic-abort.diff
View file

@ -31,7 +31,7 @@
- StorageLive(_6);
- _6 = move _4;
- _5 = opaque::<&mut u8>(move _6) -> [return: bb1, unwind unreachable];
+ _5 = opaque::<&mut u8>(move _2) -> [return: bb1, unwind unreachable];
+ _5 = opaque::<&mut u8>(copy _2) -> [return: bb1, unwind unreachable];
}
bb1: {

2
tests/mir-opt/copy-prop/reborrow.reraw.CopyProp.panic-unwind.diff
View file

@ -31,7 +31,7 @@
- StorageLive(_6);
- _6 = move _4;
- _5 = opaque::<&mut u8>(move _6) -> [return: bb1, unwind continue];
+ _5 = opaque::<&mut u8>(move _2) -> [return: bb1, unwind continue];
+ _5 = opaque::<&mut u8>(copy _2) -> [return: bb1, unwind continue];
}
bb1: {

2
tests/ui/lint/large_assignments/inline_mir.rs
View file

@ -20,5 +20,5 @@
pub fn main() {
let data = [10u8; 9999];
let cell = std::cell::UnsafeCell::new(data); //~ ERROR large_assignments
std::hint::black_box(cell);
std::hint::black_box(cell); //~ ERROR large_assignments
}

10
tests/ui/lint/large_assignments/inline_mir.stderr
View file

@ -11,5 +11,13 @@ note: the lint level is defined here
LL | #![deny(large_assignments)]
| ^^^^^^^^^^^^^^^^^
error: aborting due to 1 previous error
error: moving 9999 bytes
--> $DIR/inline_mir.rs:23:5
|
LL | std::hint::black_box(cell);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^ value moved from here
|
= note: The current maximum size is 1000, but it can be customized with the move_size_limit attribute: `#![move_size_limit = "..."]`
error: aborting due to 2 previous errors

2
tests/ui/lint/large_assignments/move_into_fn.rs
View file

@ -16,7 +16,7 @@ fn main() {
// Looking at llvm-ir output, we can see that there is no memcpy involved in
// this function call. Instead, just a pointer is passed to the function. So
// the lint shall not trigger here.
take_data(data);
take_data(data); //~ ERROR large_assignments
}
fn take_data(data: Data) {}

10
tests/ui/lint/large_assignments/move_into_fn.stderr
View file

@ -11,5 +11,13 @@ note: the lint level is defined here
LL | #![deny(large_assignments)]
| ^^^^^^^^^^^^^^^^^
error: aborting due to 1 previous error
error: moving 9999 bytes
--> $DIR/move_into_fn.rs:19:15
|
LL | take_data(data);
| ^^^^ value moved from here
|
= note: The current maximum size is 1000, but it can be customized with the move_size_limit attribute: `#![move_size_limit = "..."]`
error: aborting due to 2 previous errors