Skip to content

Commit

Permalink
Auto merge of rust-lang#128299 - DianQK:clone-copy, r=cjgillot
Browse files Browse the repository at this point in the history
Simplify the canonical clone method and the copy-like forms to copy

Fixes rust-lang#128081.

The optimized clone method ends up as the following MIR:

```
_2 = copy ((*_1).0: i32);
_3 = copy ((*_1).1: u64);
_4 = copy ((*_1).2: [i8; 3]);
_0 = Foo { a: move _2, b: move _3, c: move _4 };
```

We can transform this to:

```
_0 = copy (*_1);
```

r? `@cjgillot`
  • Loading branch information
bors committed Sep 14, 2024
2 parents 02b1be1 + 25d434b commit e7386b3
Show file tree
Hide file tree
Showing 28 changed files with 1,648 additions and 53 deletions.
98 changes: 97 additions & 1 deletion compiler/rustc_mir_transform/src/gvn.rs
Original file line number Diff line number Diff line change
Expand Up @@ -875,6 +875,95 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
None
}

fn try_as_place_elem(
&mut self,
proj: ProjectionElem<VnIndex, Ty<'tcx>>,
loc: Location,
) -> Option<PlaceElem<'tcx>> {
Some(match proj {
ProjectionElem::Deref => ProjectionElem::Deref,
ProjectionElem::Field(idx, ty) => ProjectionElem::Field(idx, ty),
ProjectionElem::Index(idx) => {
let Some(local) = self.try_as_local(idx, loc) else {
return None;
};
self.reused_locals.insert(local);
ProjectionElem::Index(local)
}
ProjectionElem::ConstantIndex { offset, min_length, from_end } => {
ProjectionElem::ConstantIndex { offset, min_length, from_end }
}
ProjectionElem::Subslice { from, to, from_end } => {
ProjectionElem::Subslice { from, to, from_end }
}
ProjectionElem::Downcast(symbol, idx) => ProjectionElem::Downcast(symbol, idx),
ProjectionElem::OpaqueCast(idx) => ProjectionElem::OpaqueCast(idx),
ProjectionElem::Subtype(idx) => ProjectionElem::Subtype(idx),
})
}

fn simplify_aggregate_to_copy(
&mut self,
rvalue: &mut Rvalue<'tcx>,
location: Location,
fields: &[VnIndex],
variant_index: VariantIdx,
) -> Option<VnIndex> {
let Some(&first_field) = fields.first() else {
return None;
};
let Value::Projection(copy_from_value, _) = *self.get(first_field) else {
return None;
};
// All fields must correspond one-to-one and come from the same aggregate value.
if fields.iter().enumerate().any(|(index, &v)| {
if let Value::Projection(pointer, ProjectionElem::Field(from_index, _)) = *self.get(v)
&& copy_from_value == pointer
&& from_index.index() == index
{
return false;
}
true
}) {
return None;
}

let mut copy_from_local_value = copy_from_value;
if let Value::Projection(pointer, proj) = *self.get(copy_from_value)
&& let ProjectionElem::Downcast(_, read_variant) = proj
{
if variant_index == read_variant {
// When copying a variant, there is no need to downcast.
copy_from_local_value = pointer;
} else {
// The copied variant must be identical.
return None;
}
}

let tcx = self.tcx;
let mut projection = SmallVec::<[PlaceElem<'tcx>; 1]>::new();
loop {
if let Some(local) = self.try_as_local(copy_from_local_value, location) {
projection.reverse();
let place = Place { local, projection: tcx.mk_place_elems(projection.as_slice()) };
if rvalue.ty(self.local_decls, tcx) == place.ty(self.local_decls, tcx).ty {
self.reused_locals.insert(local);
*rvalue = Rvalue::Use(Operand::Copy(place));
return Some(copy_from_value);
}
return None;
} else if let Value::Projection(pointer, proj) = *self.get(copy_from_local_value)
&& let Some(proj) = self.try_as_place_elem(proj, location)
{
projection.push(proj);
copy_from_local_value = pointer;
} else {
return None;
}
}
}

fn simplify_aggregate(
&mut self,
rvalue: &mut Rvalue<'tcx>,
Expand Down Expand Up @@ -971,6 +1060,13 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
}
}

if let AggregateTy::Def(_, _) = ty
&& let Some(value) =
self.simplify_aggregate_to_copy(rvalue, location, &fields, variant_index)
{
return Some(value);
}

Some(self.insert(Value::Aggregate(ty, variant_index, fields)))
}

Expand Down Expand Up @@ -1485,7 +1581,7 @@ impl<'tcx> VnState<'_, 'tcx> {
}

/// If there is a local which is assigned `index`, and its assignment strictly dominates `loc`,
/// return it.
/// return it. If you used this local, add it to `reused_locals` to remove storage statements.
fn try_as_local(&mut self, index: VnIndex, loc: Location) -> Option<Local> {
let other = self.rev_locals.get(index)?;
other
Expand Down
40 changes: 40 additions & 0 deletions tests/codegen/clone_as_copy.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
//@ revisions: DEBUGINFO NODEBUGINFO
//@ compile-flags: -O -Cno-prepopulate-passes
//@ [DEBUGINFO] compile-flags: -Cdebuginfo=full

// From https://github.com/rust-lang/rust/issues/128081.
// Ensure that we only generate a memcpy instruction.

#![crate_type = "lib"]

#[derive(Clone)]
struct SubCloneAndCopy {
v1: u32,
v2: u32,
}

#[derive(Clone)]
struct CloneOnly {
v1: u8,
v2: u8,
v3: u8,
v4: u8,
v5: u8,
v6: u8,
v7: u8,
v8: u8,
v9: u8,
v_sub: SubCloneAndCopy,
v_large: [u8; 256],
}

// CHECK-LABEL: define {{.*}}@clone_only(
#[no_mangle]
pub fn clone_only(v: &CloneOnly) -> CloneOnly {
// CHECK-NOT: call {{.*}}clone
// CHECK-NOT: store i8
// CHECK-NOT: store i32
// CHECK: call void @llvm.memcpy
// CHECK-NEXT: ret void
v.clone()
}
6 changes: 4 additions & 2 deletions tests/codegen/enum/unreachable_enum_default_branch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,13 @@ pub fn implicit_match(x: Int) -> bool {
// The code is from https://github.com/rust-lang/rust/issues/110097.
// We expect it to generate the same optimized code as a full match.
// CHECK-LABEL: @if_let(
// CHECK-NEXT: start:
// CHECK: start:
// CHECK-NOT: zext
// CHECK: select
// CHECK-NEXT: insertvalue
// CHECK-NEXT: insertvalue
// CHECK-NEXT: ret
#[no_mangle]
pub fn if_let(val: Result<i32, ()>) -> Result<i32, ()> {
if let Ok(x) = val { Ok(x) } else { Err(()) }
if let Ok(x) = val { Ok(x * 2) } else { Err(()) }
}
7 changes: 6 additions & 1 deletion tests/codegen/try_question_mark_nop.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,22 @@
//@ compile-flags: -O -Z merge-functions=disabled --edition=2021
//@ only-x86_64
// FIXME: Remove the `min-llvm-version`.
//@ min-llvm-version: 19

#![crate_type = "lib"]
#![feature(try_blocks)]

use std::ops::ControlFlow::{self, Break, Continue};
use std::ptr::NonNull;

// FIXME: The `trunc` and `select` instructions can be eliminated.
// CHECK-LABEL: @option_nop_match_32
#[no_mangle]
pub fn option_nop_match_32(x: Option<u32>) -> Option<u32> {
// CHECK: start:
// CHECK-NEXT: insertvalue { i32, i32 }
// CHECK-NEXT: [[TRUNC:%.*]] = trunc nuw i32 %0 to i1
// CHECK-NEXT: [[FIRST:%.*]] = select i1 [[TRUNC]], i32 %0
// CHECK-NEXT: insertvalue { i32, i32 } poison, i32 [[FIRST]]
// CHECK-NEXT: insertvalue { i32, i32 }
// CHECK-NEXT: ret { i32, i32 }
match x {
Expand Down
17 changes: 17 additions & 0 deletions tests/mir-opt/gvn_clone.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
//@ test-mir-pass: GVN
//@ compile-flags: -Zmir-enable-passes=+InstSimplify-before-inline

// Check if we have transformed the default clone to copy in the specific pipeline.

// EMIT_MIR gvn_clone.{impl#0}-clone.GVN.diff

// CHECK-LABEL: ::clone(
// CHECK-NOT: = AllCopy { {{.*}} };
// CHECK: _0 = copy (*_1);
// CHECK: return;
#[derive(Clone)]
struct AllCopy {
a: i32,
b: u64,
c: [i8; 3],
}
71 changes: 71 additions & 0 deletions tests/mir-opt/gvn_clone.{impl#0}-clone.GVN.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
- // MIR for `<impl at $DIR/gvn_clone.rs:12:10: 12:15>::clone` before GVN
+ // MIR for `<impl at $DIR/gvn_clone.rs:12:10: 12:15>::clone` after GVN

fn <impl at $DIR/gvn_clone.rs:12:10: 12:15>::clone(_1: &AllCopy) -> AllCopy {
debug self => _1;
let mut _0: AllCopy;
let mut _2: i32;
let mut _3: &i32;
let _4: &i32;
let mut _5: u64;
let mut _6: &u64;
let _7: &u64;
let mut _8: [i8; 3];
let mut _9: &[i8; 3];
let _10: &[i8; 3];

bb0: {
StorageLive(_2);
StorageLive(_3);
- StorageLive(_4);
+ nop;
_4 = &((*_1).0: i32);
_3 = copy _4;
- _2 = copy (*_3);
+ _2 = copy ((*_1).0: i32);
goto -> bb1;
}

bb1: {
StorageDead(_3);
StorageLive(_5);
StorageLive(_6);
- StorageLive(_7);
+ nop;
_7 = &((*_1).1: u64);
_6 = copy _7;
- _5 = copy (*_6);
+ _5 = copy ((*_1).1: u64);
goto -> bb2;
}

bb2: {
StorageDead(_6);
StorageLive(_8);
StorageLive(_9);
- StorageLive(_10);
+ nop;
_10 = &((*_1).2: [i8; 3]);
_9 = copy _10;
- _8 = copy (*_9);
+ _8 = copy ((*_1).2: [i8; 3]);
goto -> bb3;
}

bb3: {
StorageDead(_9);
- _0 = AllCopy { a: move _2, b: move _5, c: move _8 };
+ _0 = copy (*_1);
StorageDead(_8);
StorageDead(_5);
StorageDead(_2);
- StorageDead(_10);
- StorageDead(_7);
- StorageDead(_4);
+ nop;
+ nop;
+ nop;
return;
}
}

53 changes: 53 additions & 0 deletions tests/mir-opt/gvn_copy_aggregate.all_copy.GVN.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
- // MIR for `all_copy` before GVN
+ // MIR for `all_copy` after GVN

fn all_copy(_1: &AllCopy) -> AllCopy {
debug v => _1;
let mut _0: AllCopy;
let _2: i32;
let mut _5: i32;
let mut _6: u64;
let mut _7: [i8; 3];
scope 1 {
debug a => _2;
let _3: u64;
scope 2 {
debug b => _3;
let _4: [i8; 3];
scope 3 {
debug c => _4;
}
}
}

bb0: {
- StorageLive(_2);
+ nop;
_2 = copy ((*_1).0: i32);
- StorageLive(_3);
+ nop;
_3 = copy ((*_1).1: u64);
- StorageLive(_4);
+ nop;
_4 = copy ((*_1).2: [i8; 3]);
StorageLive(_5);
_5 = copy _2;
StorageLive(_6);
_6 = copy _3;
StorageLive(_7);
_7 = copy _4;
- _0 = AllCopy { a: move _5, b: move _6, c: move _7 };
+ _0 = copy (*_1);
StorageDead(_7);
StorageDead(_6);
StorageDead(_5);
- StorageDead(_4);
- StorageDead(_3);
- StorageDead(_2);
+ nop;
+ nop;
+ nop;
return;
}
}

Loading

0 comments on commit e7386b3

Please sign in to comment.