Auto merge of rust-lang#113758 - cjgillot:move-dse, r=JakobDegen,oli-obk

Turn copy into moves during DSE.

Dead store elimination computes whether removing a direct store to an unborrowed place is allowed.
Where removing a store is allowed, writing `uninit` is too.

This means that we can use this pass to transform `copy` operands into `move` operands. This is only interesting in call terminators, so we only handle those.

Special care is taken for the `use_both(_1, _1)` case:
- moving the second argument is ok, as `_1` is not live after the call;
- moving the first argument is not, as the second argument reads `_1`.

Fixes rust-lang#75993
Fixes rust-lang#108068

r? `@RalfJung`
cc `@JakobDegen`

Author: bors

compiler/rustc_mir_dataflow/src/impls/liveness.rs

@@ -88,7 +88,7 @@ impl<'tcx> GenKillAnalysis<'tcx> for MaybeLiveLocals {
     }
 }
 
-struct TransferFunction<'a, T>(&'a mut T);
+pub struct TransferFunction<'a, T>(pub &'a mut T);
 
 impl<'tcx, T> Visitor<'tcx> for TransferFunction<'_, T>
 where

compiler/rustc_mir_dataflow/src/impls/mod.rs

@@ -26,6 +26,7 @@ pub use self::borrowed_locals::borrowed_locals;
 pub use self::borrowed_locals::MaybeBorrowedLocals;
 pub use self::liveness::MaybeLiveLocals;
 pub use self::liveness::MaybeTransitiveLiveLocals;
+pub use self::liveness::TransferFunction as LivenessTransferFunction;
 pub use self::storage_liveness::{MaybeRequiresStorage, MaybeStorageDead, MaybeStorageLive};
 
 /// `MaybeInitializedPlaces` tracks all places that might be

compiler/rustc_mir_transform/src/dead_store_elimination.rs

@@ -13,9 +13,12 @@
 //!
 
 use rustc_index::bit_set::BitSet;
+use rustc_middle::mir::visit::Visitor;
 use rustc_middle::mir::*;
 use rustc_middle::ty::TyCtxt;
-use rustc_mir_dataflow::impls::{borrowed_locals, MaybeTransitiveLiveLocals};
+use rustc_mir_dataflow::impls::{
+    borrowed_locals, LivenessTransferFunction, MaybeTransitiveLiveLocals,
+};
 use rustc_mir_dataflow::Analysis;
 
 /// Performs the optimization on the body
@@ -28,8 +31,33 @@ pub fn eliminate<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>, borrowed: &BitS
         .iterate_to_fixpoint()
         .into_results_cursor(body);
 
+    // For blocks with a call terminator, if an argument copy can be turned into a move,
+    // record it as (block, argument index).
+    let mut call_operands_to_move = Vec::new();
     let mut patch = Vec::new();
+
     for (bb, bb_data) in traversal::preorder(body) {
+        if let TerminatorKind::Call { ref args, .. } = bb_data.terminator().kind {
+            let loc = Location { block: bb, statement_index: bb_data.statements.len() };
+
+            // Position ourselves between the evaluation of `args` and the write to `destination`.
+            live.seek_to_block_end(bb);
+            let mut state = live.get().clone();
+
+            for (index, arg) in args.iter().enumerate().rev() {
+                if let Operand::Copy(place) = *arg
+                    && !place.is_indirect()
+                    && !borrowed.contains(place.local)
+                    && !state.contains(place.local)
+                {
+                    call_operands_to_move.push((bb, index));
+                }
+
+                // Account that `arg` is read from, so we don't promote another argument to a move.
+                LivenessTransferFunction(&mut state).visit_operand(arg, loc);
+            }
+        }
+
         for (statement_index, statement) in bb_data.statements.iter().enumerate().rev() {
             let loc = Location { block: bb, statement_index };
             if let StatementKind::Assign(assign) = &statement.kind {
@@ -64,14 +92,22 @@ pub fn eliminate<'tcx>(tcx: TyCtxt<'tcx>, body: &mut Body<'tcx>, borrowed: &BitS
         }
     }
 
-    if patch.is_empty() {
+    if patch.is_empty() && call_operands_to_move.is_empty() {
         return;
     }
 
     let bbs = body.basic_blocks.as_mut_preserves_cfg();
     for Location { block, statement_index } in patch {
         bbs[block].statements[statement_index].make_nop();
     }
+    for (block, argument_index) in call_operands_to_move {
+        let TerminatorKind::Call { ref mut args, .. } = bbs[block].terminator_mut().kind else {
+            bug!()
+        };
+        let arg = &mut args[argument_index];
+        let Operand::Copy(place) = *arg else { bug!() };
+        *arg = Operand::Move(place);
+    }
 
     crate::simplify::simplify_locals(body, tcx)
 }

tests/codegen/iter-repeat-n-trivial-drop.rs

@@ -33,7 +33,7 @@ pub fn iter_repeat_n_next(it: &mut std::iter::RepeatN<NotCopy>) -> Option<NotCop
 
     // CHECK: [[EMPTY]]:
     // CHECK-NOT: br
-    // CHECK: phi i16 [ %[[VAL]], %[[NOT_EMPTY]] ], [ undef, %start ]
+    // CHECK: phi i16
     // CHECK-NOT: br
     // CHECK: ret
 

tests/codegen/move-operands.rs

@@ -1,4 +1,5 @@
-// compile-flags: -C no-prepopulate-passes -Zmir-enable-passes=+DestinationPropagation,-CopyProp
+// Verify that optimized MIR only copies `a` once.
+// compile-flags: -O -C no-prepopulate-passes
 
 #![crate_type = "lib"]
 

tests/mir-opt/dead-store-elimination/call_arg_copy.move_simple.DeadStoreElimination.panic-abort.diff

@@ -0,0 +1,23 @@
+- // MIR for `move_simple` before DeadStoreElimination
++ // MIR for `move_simple` after DeadStoreElimination
+  
+  fn move_simple(_1: i32) -> () {
+      debug x => _1;
+      let mut _0: ();
+      let _2: ();
+-     let mut _3: i32;
+-     let mut _4: i32;
+  
+      bb0: {
+          StorageLive(_2);
+-         _2 = use_both(_1, _1) -> [return: bb1, unwind unreachable];
++         _2 = use_both(_1, move _1) -> [return: bb1, unwind unreachable];
+      }
+  
+      bb1: {
+          StorageDead(_2);
+          _0 = const ();
+          return;
+      }
+  }
+  

tests/mir-opt/dead-store-elimination/call_arg_copy.move_simple.DeadStoreElimination.panic-unwind.diff

@@ -0,0 +1,23 @@
+- // MIR for `move_simple` before DeadStoreElimination
++ // MIR for `move_simple` after DeadStoreElimination
+  
+  fn move_simple(_1: i32) -> () {
+      debug x => _1;
+      let mut _0: ();
+      let _2: ();
+-     let mut _3: i32;
+-     let mut _4: i32;
+  
+      bb0: {
+          StorageLive(_2);
+-         _2 = use_both(_1, _1) -> [return: bb1, unwind continue];
++         _2 = use_both(_1, move _1) -> [return: bb1, unwind continue];
+      }
+  
+      bb1: {
+          StorageDead(_2);
+          _0 = const ();
+          return;
+      }
+  }
+  

tests/mir-opt/dead-store-elimination/call_arg_copy.rs

@@ -0,0 +1,15 @@
+// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
+// unit-test: DeadStoreElimination
+// compile-flags: -Zmir-enable-passes=+CopyProp
+
+#[inline(never)]
+fn use_both(_: i32, _: i32) {}
+
+// EMIT_MIR call_arg_copy.move_simple.DeadStoreElimination.diff
+fn move_simple(x: i32) {
+    use_both(x, x);
+}
+
+fn main() {
+    move_simple(1);
+}

tests/mir-opt/inline/dyn_trait.get_query.Inline.panic-abort.diff

@@ -32,7 +32,7 @@
 -         _0 = try_execute_query::<<Q as Query>::C>(move _4) -> [return: bb2, unwind unreachable];
 +         StorageLive(_5);
 +         _5 = _4 as &dyn Cache<V = <Q as Query>::V> (PointerCoercion(Unsize));
-+         _0 = <dyn Cache<V = <Q as Query>::V> as Cache>::store_nocache(_5) -> [return: bb2, unwind unreachable];
++         _0 = <dyn Cache<V = <Q as Query>::V> as Cache>::store_nocache(move _5) -> [return: bb2, unwind unreachable];
       }
 
       bb2: {

tests/mir-opt/inline/dyn_trait.get_query.Inline.panic-unwind.diff

@@ -32,7 +32,7 @@
 -         _0 = try_execute_query::<<Q as Query>::C>(move _4) -> [return: bb2, unwind continue];
 +         StorageLive(_5);
 +         _5 = _4 as &dyn Cache<V = <Q as Query>::V> (PointerCoercion(Unsize));
-+         _0 = <dyn Cache<V = <Q as Query>::V> as Cache>::store_nocache(_5) -> [return: bb2, unwind continue];
++         _0 = <dyn Cache<V = <Q as Query>::V> as Cache>::store_nocache(move _5) -> [return: bb2, unwind continue];
       }
 
       bb2: {