diff --git a/compiler/rustc_mir_transform/src/copy_prop.rs b/compiler/rustc_mir_transform/src/copy_prop.rs index 2e9c3a5bf3eeb..89ee96317ac93 100644 --- a/compiler/rustc_mir_transform/src/copy_prop.rs +++ b/compiler/rustc_mir_transform/src/copy_prop.rs @@ -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 { - 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, - storage_to_remove: DenseBitSet, + unified: DenseBitSet, copy_classes: &'a IndexSlice, } @@ -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); diff --git a/tests/mir-opt/copy-prop/move_arg.f.CopyProp.panic-abort.diff b/tests/mir-opt/copy-prop/move_arg.f.CopyProp.panic-abort.diff deleted file mode 100644 index da9904bfa01f5..0000000000000 --- a/tests/mir-opt/copy-prop/move_arg.f.CopyProp.panic-abort.diff +++ /dev/null @@ -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::(move _4, move _5) -> [return: bb1, unwind unreachable]; -+ _3 = g::(copy _1, copy _1) -> [return: bb1, unwind unreachable]; - } - - bb1: { -- StorageDead(_5); -- StorageDead(_4); - StorageDead(_3); - _0 = const (); -- StorageDead(_2); - return; - } - } - diff --git a/tests/mir-opt/copy-prop/move_arg.f.CopyProp.panic-unwind.diff b/tests/mir-opt/copy-prop/move_arg.f.CopyProp.panic-unwind.diff deleted file mode 100644 index 9b0de655bd262..0000000000000 --- a/tests/mir-opt/copy-prop/move_arg.f.CopyProp.panic-unwind.diff +++ /dev/null @@ -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::(move _4, move _5) -> [return: bb1, unwind continue]; -+ _3 = g::(copy _1, copy _1) -> [return: bb1, unwind continue]; - } - - bb1: { -- StorageDead(_5); -- StorageDead(_4); - StorageDead(_3); - _0 = const (); -- StorageDead(_2); - return; - } - } - diff --git a/tests/mir-opt/copy-prop/move_arg.rs b/tests/mir-opt/copy-prop/move_arg.rs index 498340534324b..23e20021f5884 100644 --- a/tests/mir-opt/copy-prop/move_arg.rs +++ b/tests/mir-opt/copy-prop/move_arg.rs @@ -1,17 +1,46 @@ -// skip-filecheck -// 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(a: T) { - let b = a; - g(a, b); +#[custom_mir(dialect = "runtime", phase = "initial")] +pub fn moved_and_copied(_1: T) { + // CHECK-LABEL: fn moved_and_copied( + // CHECK: _0 = f::(copy _1, copy _1) + mir! { + { + let _2 = _1; + Call(RET = f(Move(_1), Move(_2)), ReturnTo(bb1), UnwindUnreachable()) + } + bb1 = { + Return() + } + } } -#[inline(never)] -pub fn g(_: T, _: T) {} - -fn main() { - f(5) +#[custom_mir(dialect = "runtime", phase = "initial")] +pub fn moved_twice(_1: MaybeUninit) { + // 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::>(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 f(_: T, _: T) {} diff --git a/tests/mir-opt/copy-prop/reborrow.remut.CopyProp.panic-abort.diff b/tests/mir-opt/copy-prop/reborrow.remut.CopyProp.panic-abort.diff index 2026c1982f299..8ef61b5667dd5 100644 --- a/tests/mir-opt/copy-prop/reborrow.remut.CopyProp.panic-abort.diff +++ b/tests/mir-opt/copy-prop/reborrow.remut.CopyProp.panic-abort.diff @@ -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: { diff --git a/tests/mir-opt/copy-prop/reborrow.remut.CopyProp.panic-unwind.diff b/tests/mir-opt/copy-prop/reborrow.remut.CopyProp.panic-unwind.diff index 67763fdce6676..2a7182af984d6 100644 --- a/tests/mir-opt/copy-prop/reborrow.remut.CopyProp.panic-unwind.diff +++ b/tests/mir-opt/copy-prop/reborrow.remut.CopyProp.panic-unwind.diff @@ -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: { diff --git a/tests/mir-opt/copy-prop/reborrow.reraw.CopyProp.panic-abort.diff b/tests/mir-opt/copy-prop/reborrow.reraw.CopyProp.panic-abort.diff index dfc8dd0975638..8a2cdd8e5728e 100644 --- a/tests/mir-opt/copy-prop/reborrow.reraw.CopyProp.panic-abort.diff +++ b/tests/mir-opt/copy-prop/reborrow.reraw.CopyProp.panic-abort.diff @@ -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: { diff --git a/tests/mir-opt/copy-prop/reborrow.reraw.CopyProp.panic-unwind.diff b/tests/mir-opt/copy-prop/reborrow.reraw.CopyProp.panic-unwind.diff index becc425632104..614d23cf62457 100644 --- a/tests/mir-opt/copy-prop/reborrow.reraw.CopyProp.panic-unwind.diff +++ b/tests/mir-opt/copy-prop/reborrow.reraw.CopyProp.panic-unwind.diff @@ -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: { diff --git a/tests/ui/lint/large_assignments/inline_mir.rs b/tests/ui/lint/large_assignments/inline_mir.rs index fc27b8ff244d1..d16aae6ab145b 100644 --- a/tests/ui/lint/large_assignments/inline_mir.rs +++ b/tests/ui/lint/large_assignments/inline_mir.rs @@ -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 } diff --git a/tests/ui/lint/large_assignments/inline_mir.stderr b/tests/ui/lint/large_assignments/inline_mir.stderr index d9010e24d03be..1a5fcb6c8fc18 100644 --- a/tests/ui/lint/large_assignments/inline_mir.stderr +++ b/tests/ui/lint/large_assignments/inline_mir.stderr @@ -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 diff --git a/tests/ui/lint/large_assignments/move_into_fn.rs b/tests/ui/lint/large_assignments/move_into_fn.rs index 73ec08fa23a74..b3b2160ca36e1 100644 --- a/tests/ui/lint/large_assignments/move_into_fn.rs +++ b/tests/ui/lint/large_assignments/move_into_fn.rs @@ -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) {} diff --git a/tests/ui/lint/large_assignments/move_into_fn.stderr b/tests/ui/lint/large_assignments/move_into_fn.stderr index 92a0489e472f9..19ec6a51d2e74 100644 --- a/tests/ui/lint/large_assignments/move_into_fn.stderr +++ b/tests/ui/lint/large_assignments/move_into_fn.stderr @@ -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