Skip to content

Commit

Permalink
Auto merge of #113758 - cjgillot:move-dse, r=JakobDegen,oli-obk
Browse files Browse the repository at this point in the history
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 #75993
Fixes #108068

r? `@RalfJung`
cc `@JakobDegen`
  • Loading branch information
bors committed Jul 20, 2023
2 parents 6b53175 + 254bf60 commit 06a53dd
Show file tree
Hide file tree
Showing 32 changed files with 130 additions and 31 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_mir_dataflow/src/impls/liveness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_mir_dataflow/src/impls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
40 changes: 38 additions & 2 deletions compiler/rustc_mir_transform/src/dead_store_elimination.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion tests/codegen/iter-repeat-n-trivial-drop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
3 changes: 2 additions & 1 deletion tests/codegen/move-operands.rs
Original file line number Diff line number Diff line change
@@ -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"]

Expand Down
Original file line number Diff line number Diff line change
@@ -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;
}
}

Original file line number Diff line number Diff line change
@@ -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;
}
}

15 changes: 15 additions & 0 deletions tests/mir-opt/dead-store-elimination/call_arg_copy.rs
Original file line number Diff line number Diff line change
@@ -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);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
_2 = move _3 as &dyn Cache<V = <C as Cache>::V> (PointerCoercion(Unsize));
StorageDead(_3);
- _0 = mk_cycle::<<C as Cache>::V>(move _2) -> [return: bb1, unwind unreachable];
+ _0 = <dyn Cache<V = <C as Cache>::V> as Cache>::store_nocache(_2) -> [return: bb1, unwind unreachable];
+ _0 = <dyn Cache<V = <C as Cache>::V> as Cache>::store_nocache(move _2) -> [return: bb1, unwind unreachable];
}

bb1: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
_2 = move _3 as &dyn Cache<V = <C as Cache>::V> (PointerCoercion(Unsize));
StorageDead(_3);
- _0 = mk_cycle::<<C as Cache>::V>(move _2) -> [return: bb1, unwind continue];
+ _0 = <dyn Cache<V = <C as Cache>::V> as Cache>::store_nocache(_2) -> [return: bb1, unwind continue];
+ _0 = <dyn Cache<V = <C as Cache>::V> as Cache>::store_nocache(move _2) -> [return: bb1, unwind continue];
}

bb1: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@
+ StorageDead(_14);
+ StorageLive(_9);
+ _13 = const _;
+ _9 = std::alloc::Global::alloc_impl(_13, _8, const false) -> [return: bb5, unwind unreachable];
+ _9 = std::alloc::Global::alloc_impl(move _13, _8, const false) -> [return: bb5, unwind unreachable];
}

bb1: {
Expand All @@ -144,7 +144,7 @@
}

bb2: {
+ _12 = handle_alloc_error(_8) -> unwind unreachable;
+ _12 = handle_alloc_error(move _8) -> unwind unreachable;
+ }
+
+ bb3: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@
+ StorageDead(_14);
+ StorageLive(_9);
+ _13 = const _;
+ _9 = std::alloc::Global::alloc_impl(_13, _8, const false) -> [return: bb7, unwind: bb3];
+ _9 = std::alloc::Global::alloc_impl(move _13, _8, const false) -> [return: bb7, unwind: bb3];
}

bb1: {
Expand All @@ -161,7 +161,7 @@
- bb4 (cleanup): {
- resume;
+ bb4: {
+ _12 = handle_alloc_error(_8) -> bb3;
+ _12 = handle_alloc_error(move _8) -> bb3;
+ }
+
+ bb5: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ fn test2(_1: &dyn X) -> bool {
_3 = &(*_1);
_2 = move _3 as &dyn X (PointerCoercion(Unsize));
StorageDead(_3);
_0 = <dyn X as X>::y(_2) -> [return: bb1, unwind unreachable];
_0 = <dyn X as X>::y(move _2) -> [return: bb1, unwind unreachable];
}

bb1: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ fn test2(_1: &dyn X) -> bool {
_3 = &(*_1);
_2 = move _3 as &dyn X (PointerCoercion(Unsize));
StorageDead(_3);
_0 = <dyn X as X>::y(_2) -> [return: bb1, unwind continue];
_0 = <dyn X as X>::y(move _2) -> [return: bb1, unwind continue];
}

bb1: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
bb2: {
_6 = Shl(move _7, const 1_i32);
StorageDead(_7);
_3 = rotate_right::<u32>(_4, _6) -> [return: bb3, unwind unreachable];
_3 = rotate_right::<u32>(move _4, move _6) -> [return: bb3, unwind unreachable];
}

bb3: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@
bb2: {
_6 = Shl(move _7, const 1_i32);
StorageDead(_7);
_3 = rotate_right::<u32>(_4, _6) -> [return: bb3, unwind unreachable];
_3 = rotate_right::<u32>(move _4, move _6) -> [return: bb3, unwind unreachable];
}

bb3: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ fn num_to_digit(_1: char) -> u32 {

bb2: {
StorageLive(_4);
_4 = char::methods::<impl char>::to_digit(_1, const 8_u32) -> [return: bb3, unwind unreachable];
_4 = char::methods::<impl char>::to_digit(move _1, const 8_u32) -> [return: bb3, unwind unreachable];
}

bb3: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ fn num_to_digit(_1: char) -> u32 {

bb2: {
StorageLive(_4);
_4 = char::methods::<impl char>::to_digit(_1, const 8_u32) -> [return: bb3, unwind continue];
_4 = char::methods::<impl char>::to_digit(move _1, const 8_u32) -> [return: bb3, unwind continue];
}

bb3: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ fn checked_shl(_1: u32, _2: u32) -> Option<u32> {
StorageDead(_4);
_6 = Ge(_2, const _);
StorageLive(_7);
_7 = unlikely(_6) -> [return: bb1, unwind unreachable];
_7 = unlikely(move _6) -> [return: bb1, unwind unreachable];
}

bb1: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ fn int_range(_1: usize, _2: usize) -> () {

bb7: {
_10 = ((_6 as Some).0: usize);
_11 = opaque::<usize>(_10) -> [return: bb8, unwind continue];
_11 = opaque::<usize>(move _10) -> [return: bb8, unwind continue];
}

bb8: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ fn inclusive_loop(_1: u32, _2: u32, _3: impl Fn(u32)) -> () {
StorageLive(_7);
StorageLive(_6);
_6 = &mut _5;
_7 = <RangeInclusive<u32> as iter::range::RangeInclusiveIteratorImpl>::spec_next(_6) -> [return: bb2, unwind unreachable];
_7 = <RangeInclusive<u32> as iter::range::RangeInclusiveIteratorImpl>::spec_next(move _6) -> [return: bb2, unwind unreachable];
}

bb2: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ fn inclusive_loop(_1: u32, _2: u32, _3: impl Fn(u32)) -> () {
StorageLive(_7);
StorageLive(_6);
_6 = &mut _5;
_7 = <RangeInclusive<u32> as iter::range::RangeInclusiveIteratorImpl>::spec_next(_6) -> [return: bb2, unwind: bb8];
_7 = <RangeInclusive<u32> as iter::range::RangeInclusiveIteratorImpl>::spec_next(move _6) -> [return: bb2, unwind: bb8];
}

bb2: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ fn range_inclusive_iter_next(_1: &mut RangeInclusive<u32>) -> Option<u32> {
}

bb0: {
_0 = <RangeInclusive<u32> as iter::range::RangeInclusiveIteratorImpl>::spec_next(_1) -> [return: bb1, unwind unreachable];
_0 = <RangeInclusive<u32> as iter::range::RangeInclusiveIteratorImpl>::spec_next(move _1) -> [return: bb1, unwind unreachable];
}

bb1: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ fn range_inclusive_iter_next(_1: &mut RangeInclusive<u32>) -> Option<u32> {
}

bb0: {
_0 = <RangeInclusive<u32> as iter::range::RangeInclusiveIteratorImpl>::spec_next(_1) -> [return: bb1, unwind continue];
_0 = <RangeInclusive<u32> as iter::range::RangeInclusiveIteratorImpl>::spec_next(move _1) -> [return: bb1, unwind continue];
}

bb1: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ fn slice_index_range(_1: &[u32], _2: std::ops::Range<usize>) -> &[u32] {

bb0: {
StorageLive(_3);
_3 = <std::ops::Range<usize> as SliceIndex<[u32]>>::index(move _2, _1) -> [return: bb1, unwind unreachable];
_3 = <std::ops::Range<usize> as SliceIndex<[u32]>>::index(move _2, move _1) -> [return: bb1, unwind unreachable];
}

bb1: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ fn slice_index_range(_1: &[u32], _2: std::ops::Range<usize>) -> &[u32] {

bb0: {
StorageLive(_3);
_3 = <std::ops::Range<usize> as SliceIndex<[u32]>>::index(move _2, _1) -> [return: bb1, unwind continue];
_3 = <std::ops::Range<usize> as SliceIndex<[u32]>>::index(move _2, move _1) -> [return: bb1, unwind continue];
}

bb1: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ fn slice_iter_mut_next_back(_1: &mut std::slice::IterMut<'_, T>) -> Option<&mut
let mut _0: std::option::Option<&mut T>;

bb0: {
_0 = <std::slice::IterMut<'_, T> as DoubleEndedIterator>::next_back(_1) -> [return: bb1, unwind unreachable];
_0 = <std::slice::IterMut<'_, T> as DoubleEndedIterator>::next_back(move _1) -> [return: bb1, unwind unreachable];
}

bb1: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ fn slice_iter_mut_next_back(_1: &mut std::slice::IterMut<'_, T>) -> Option<&mut
let mut _0: std::option::Option<&mut T>;

bb0: {
_0 = <std::slice::IterMut<'_, T> as DoubleEndedIterator>::next_back(_1) -> [return: bb1, unwind continue];
_0 = <std::slice::IterMut<'_, T> as DoubleEndedIterator>::next_back(move _1) -> [return: bb1, unwind continue];
}

bb1: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ fn slice_iter_next(_1: &mut std::slice::Iter<'_, T>) -> Option<&T> {
let mut _0: std::option::Option<&T>;

bb0: {
_0 = <std::slice::Iter<'_, T> as Iterator>::next(_1) -> [return: bb1, unwind unreachable];
_0 = <std::slice::Iter<'_, T> as Iterator>::next(move _1) -> [return: bb1, unwind unreachable];
}

bb1: {
Expand Down
Loading

0 comments on commit 06a53dd

Please sign in to comment.