Skip to content

Merge return place with other locals in CopyProp. #111556

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.

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

Merged
merged 2 commits into from
May 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
30 changes: 16 additions & 14 deletions compiler/rustc_mir_transform/src/copy_prop.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,20 +162,22 @@ impl<'tcx> MutVisitor<'tcx> for Replacer<'_, 'tcx> {
}

fn visit_statement(&mut self, stmt: &mut Statement<'tcx>, loc: Location) {
match stmt.kind {
// When removing storage statements, we need to remove both (#107511).
StatementKind::StorageLive(l) | StatementKind::StorageDead(l)
if self.storage_to_remove.contains(l) =>
{
stmt.make_nop()
}
StatementKind::Assign(box (ref place, ref mut rvalue))
if place.as_local().is_some() =>
{
// Do not replace assignments.
self.visit_rvalue(rvalue, loc)
}
_ => self.super_statement(stmt, loc),
// 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)
{
stmt.make_nop();
return
}

self.super_statement(stmt, loc);

// Do not leave tautological assignments around.
if let StatementKind::Assign(box (lhs, ref rhs)) = stmt.kind
&& let Rvalue::Use(Operand::Copy(rhs) | Operand::Move(rhs)) | Rvalue::CopyForDeref(rhs) = *rhs
&& lhs == rhs
{
stmt.make_nop();
}
}
}
62 changes: 34 additions & 28 deletions compiler/rustc_mir_transform/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,15 @@ impl SsaLocals {
.retain(|&local| matches!(visitor.assignments[local], Set1::One(_)));
debug!(?visitor.assignment_order);

let copy_classes = compute_copy_classes(&mut visitor, body);

SsaLocals {
let mut ssa = SsaLocals {
assignments: visitor.assignments,
assignment_order: visitor.assignment_order,
direct_uses: visitor.direct_uses,
copy_classes,
}
// This is filled by `compute_copy_classes`.
copy_classes: IndexVec::default(),
};
compute_copy_classes(&mut ssa, body);
ssa
}

pub fn num_locals(&self) -> usize {
Expand Down Expand Up @@ -261,49 +262,54 @@ impl<'tcx> Visitor<'tcx> for SsaVisitor {
}

#[instrument(level = "trace", skip(ssa, body))]
fn compute_copy_classes(ssa: &mut SsaVisitor, body: &Body<'_>) -> IndexVec<Local, Local> {
fn compute_copy_classes(ssa: &mut SsaLocals, body: &Body<'_>) {
let mut direct_uses = std::mem::take(&mut ssa.direct_uses);
let mut copies = IndexVec::from_fn_n(|l| l, body.local_decls.len());

for &local in &ssa.assignment_order {
debug!(?local);

if local == RETURN_PLACE {
// `_0` is special, we cannot rename it.
continue;
}

// This is not SSA: mark that we don't know the value.
debug!(assignments = ?ssa.assignments[local]);
let Set1::One(LocationExtended::Plain(loc)) = ssa.assignments[local] else { continue };

// `loc` must point to a direct assignment to `local`.
let Either::Left(stmt) = body.stmt_at(loc) else { bug!() };
let Some((_target, rvalue)) = stmt.kind.as_assign() else { bug!() };
assert_eq!(_target.as_local(), Some(local));

for (local, rvalue, _) in ssa.assignments(body) {
let (Rvalue::Use(Operand::Copy(place) | Operand::Move(place)) | Rvalue::CopyForDeref(place))
= rvalue
else { continue };

let Some(rhs) = place.as_local() else { continue };
let Set1::One(_) = ssa.assignments[rhs] else { continue };
if !ssa.is_ssa(rhs) {
continue;
}

// We visit in `assignment_order`, ie. reverse post-order, so `rhs` has been
// visited before `local`, and we just have to copy the representing local.
copies[local] = copies[rhs];
ssa.direct_uses[rhs] -= 1;
let head = copies[rhs];

if local == RETURN_PLACE {
// `_0` is special, we cannot rename it. Instead, rename the class of `rhs` to
// `RETURN_PLACE`. This is only possible if the class head is a temporary, not an
// argument.
if body.local_kind(head) != LocalKind::Temp {
continue;
}
for h in copies.iter_mut() {
if *h == head {
*h = RETURN_PLACE;
}
}
} else {
copies[local] = head;
}
direct_uses[rhs] -= 1;
}

debug!(?copies);
debug!(?ssa.direct_uses);
debug!(?direct_uses);

// Invariant: `copies` must point to the head of an equivalence class.
#[cfg(debug_assertions)]
for &head in copies.iter() {
assert_eq!(copies[head], head);
}
debug_assert_eq!(copies[RETURN_PLACE], RETURN_PLACE);

copies
ssa.direct_uses = direct_uses;
ssa.copy_classes = copies;
}

#[derive(Debug)]
Expand Down
4 changes: 2 additions & 2 deletions tests/codegen/fewer-names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ pub fn sum(x: u32, y: u32) -> u32 {

// NO-LABEL: define{{.*}}i32 @sum(i32 noundef %x, i32 noundef %y)
// NO-NEXT: start:
// NO-NEXT: %z = add i32 %y, %x
// NO-NEXT: ret i32 %z
// NO-NEXT: %0 = add i32 %y, %x
// NO-NEXT: ret i32 %0
let z = x + y;
z
}
7 changes: 2 additions & 5 deletions tests/codegen/mem-replace-big-type.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ pub struct Big([u64; 7]);
pub fn replace_big(dst: &mut Big, src: Big) -> Big {
// Back in 1.68, this emitted six `memcpy`s.
// `read_via_copy` in 1.69 got that down to three.
// `write_via_move` it was originally down to the essential two, however
// with nrvo disabled it is back at 3
// `write_via_move` and nvro get this down to the essential two.
std::mem::replace(dst, src)
}

Expand All @@ -26,11 +25,9 @@ pub fn replace_big(dst: &mut Big, src: Big) -> Big {
// For a large type, we expect exactly three `memcpy`s
// CHECK-LABEL: define internal void @{{.+}}mem{{.+}}replace{{.+}}sret(%Big)
// CHECK-NOT: call void @llvm.memcpy
// CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 8 %result, {{i8\*|ptr}} align 8 %dest, i{{.*}} 56, i1 false)
// CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 8 %0, {{i8\*|ptr}} align 8 %dest, i{{.*}} 56, i1 false)
// CHECK-NOT: call void @llvm.memcpy
// CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 8 %dest, {{i8\*|ptr}} align 8 %src, i{{.*}} 56, i1 false)
// CHECK-NOT: call void @llvm.memcpy
// CHECK: call void @llvm.memcpy.{{.+}}({{i8\*|ptr}} align 8 %0, {{i8\*|ptr}} align 8 %result, i{{.*}} 56, i1 false)
// CHECK-NOT: call void @llvm.memcpy

// CHECK-NOT: call void @llvm.memcpy
4 changes: 2 additions & 2 deletions tests/codegen/var-names.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ pub fn test(a: u32, b: u32) -> u32 {
// CHECK: %c = add i32 %a, %b
let d = c;
let e = d * a;
// CHECK-NEXT: %e = mul i32 %c, %a
// CHECK-NEXT: %0 = mul i32 %c, %a
e
// CHECK-NEXT: ret i32 %e
// CHECK-NEXT: ret i32 %0
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,17 @@
let mut _0: i32; // return place in scope 0 at $DIR/copy_propagation_arg.rs:+0:27: +0:30
let _2: i32; // in scope 0 at $DIR/copy_propagation_arg.rs:+1:9: +1:10
scope 1 {
debug y => _2; // in scope 1 at $DIR/copy_propagation_arg.rs:+1:9: +1:10
- debug y => _2; // in scope 1 at $DIR/copy_propagation_arg.rs:+1:9: +1:10
+ debug y => _0; // in scope 1 at $DIR/copy_propagation_arg.rs:+1:9: +1:10
}

bb0: {
StorageLive(_2); // scope 0 at $DIR/copy_propagation_arg.rs:+1:9: +1:10
_2 = _1; // scope 0 at $DIR/copy_propagation_arg.rs:+1:13: +1:14
- StorageLive(_2); // scope 0 at $DIR/copy_propagation_arg.rs:+1:9: +1:10
- _2 = _1; // scope 0 at $DIR/copy_propagation_arg.rs:+1:13: +1:14
+ _0 = _1; // scope 0 at $DIR/copy_propagation_arg.rs:+1:13: +1:14
_1 = const 123_i32; // scope 1 at $DIR/copy_propagation_arg.rs:+2:5: +2:12
_0 = _2; // scope 1 at $DIR/copy_propagation_arg.rs:+3:5: +3:6
StorageDead(_2); // scope 0 at $DIR/copy_propagation_arg.rs:+4:1: +4:2
- _0 = _2; // scope 1 at $DIR/copy_propagation_arg.rs:+3:5: +3:6
- StorageDead(_2); // scope 0 at $DIR/copy_propagation_arg.rs:+4:1: +4:2
return; // scope 0 at $DIR/copy_propagation_arg.rs:+4:2: +4:2
}
}
Expand Down
12 changes: 4 additions & 8 deletions tests/mir-opt/inline/inline_into_box_place.main.Inline.diff
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,7 @@
+ let mut _4: usize; // in scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
+ let mut _5: usize; // in scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
+ let mut _6: *mut u8; // in scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
+ let mut _7: std::boxed::Box<std::vec::Vec<u32>>; // in scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
+ let mut _8: *const std::vec::Vec<u32>; // in scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
+ let mut _7: *const std::vec::Vec<u32>; // in scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
+ scope 4 {
+ }
+ }
Expand Down Expand Up @@ -66,12 +65,9 @@
bb3: {
- StorageDead(_1); // scope 0 at $DIR/inline_into_box_place.rs:+2:1: +2:2
- return; // scope 0 at $DIR/inline_into_box_place.rs:+2:2: +2:2
+ StorageLive(_7); // scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
+ _7 = ShallowInitBox(move _6, std::vec::Vec<u32>); // scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
+ _8 = (((_7.0: std::ptr::Unique<std::vec::Vec<u32>>).0: std::ptr::NonNull<std::vec::Vec<u32>>).0: *const std::vec::Vec<u32>); // scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
+ (*_8) = move _2; // scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
+ _1 = move _7; // scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
+ StorageDead(_7); // scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
+ _1 = ShallowInitBox(move _6, std::vec::Vec<u32>); // scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
+ _7 = (((_1.0: std::ptr::Unique<std::vec::Vec<u32>>).0: std::ptr::NonNull<std::vec::Vec<u32>>).0: *const std::vec::Vec<u32>); // scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
+ (*_7) = move _2; // scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
+ StorageDead(_2); // scope 0 at $DIR/inline_into_box_place.rs:+1:48: +1:49
+ _0 = const (); // scope 0 at $DIR/inline_into_box_place.rs:+0:11: +2:2
+ drop(_1) -> [return: bb1, unwind: bb2]; // scope 0 at $DIR/inline_into_box_place.rs:+2:1: +2:2
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,22 +8,18 @@ fn b(_1: &mut Box<T>) -> &mut T {
let mut _4: &mut std::boxed::Box<T>; // in scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
scope 1 (inlined <Box<T> as AsMut<T>>::as_mut) { // at $DIR/issue_58867_inline_as_ref_as_mut.rs:8:7: 8:15
debug self => _4; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
let mut _5: &mut T; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
let mut _6: std::boxed::Box<T>; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
let mut _7: *const T; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
let mut _5: std::boxed::Box<T>; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
let mut _6: *const T; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
}

bb0: {
StorageLive(_2); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
StorageLive(_3); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
StorageLive(_4); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
_4 = &mut (*_1); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
StorageLive(_5); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:7: +1:15
_6 = deref_copy (*_4); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_7 = (((_6.0: std::ptr::Unique<T>).0: std::ptr::NonNull<T>).0: *const T); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_5 = &mut (*_7); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_3 = _5; // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
StorageDead(_5); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:7: +1:15
_5 = deref_copy (*_4); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_6 = (((_5.0: std::ptr::Unique<T>).0: std::ptr::NonNull<T>).0: *const T); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_3 = &mut (*_6); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_2 = &mut (*_3); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
StorageDead(_4); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:14: +1:15
_0 = &mut (*_2); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,21 +7,17 @@ fn d(_1: &Box<T>) -> &T {
let mut _3: &std::boxed::Box<T>; // in scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
scope 1 (inlined <Box<T> as AsRef<T>>::as_ref) { // at $DIR/issue_58867_inline_as_ref_as_mut.rs:18:7: 18:15
debug self => _3; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
let _4: &T; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
let mut _5: std::boxed::Box<T>; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
let mut _6: *const T; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
let mut _4: std::boxed::Box<T>; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
let mut _5: *const T; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
}

bb0: {
StorageLive(_2); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
StorageLive(_3); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
_3 = &(*_1); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
StorageLive(_4); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_5 = deref_copy (*_3); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_6 = (((_5.0: std::ptr::Unique<T>).0: std::ptr::NonNull<T>).0: *const T); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_4 = &(*_6); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_2 = _4; // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
StorageDead(_4); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_4 = deref_copy (*_3); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_5 = (((_4.0: std::ptr::Unique<T>).0: std::ptr::NonNull<T>).0: *const T); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_2 = &(*_5); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
_0 = &(*_2); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
StorageDead(_3); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:14: +1:15
StorageDead(_2); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+2:1: +2:2
Expand Down
Loading