Skip to content

Commit 5c3a336

Browse files
borsbors
committed
Auto merge of #111556 - cjgillot:copy-prop-nrvo, r=oli-obk
Merge return place with other locals in CopyProp. This reintroduces a limited form of NRVO. r? wg-mir-opt
2 parents e77366b + adfffc7 commit 5c3a336

19 files changed

+617
-240
lines changed

‎compiler/rustc_mir_transform/src/copy_prop.rs

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -162,20 +162,22 @@ impl<'tcx> MutVisitor<'tcx> for Replacer<'_, 'tcx> {
162162
}
163163

164164
fn visit_statement(&mut self, stmt: &mut Statement<'tcx>, loc: Location) {
165-
match stmt.kind {
166-
// When removing storage statements, we need to remove both (#107511).
167-
StatementKind::StorageLive(l) | StatementKind::StorageDead(l)
168-
if self.storage_to_remove.contains(l) =>
169-
{
170-
stmt.make_nop()
171-
}
172-
StatementKind::Assign(box (ref place, ref mut rvalue))
173-
if place.as_local().is_some() =>
174-
{
175-
// Do not replace assignments.
176-
self.visit_rvalue(rvalue, loc)
177-
}
178-
_ => self.super_statement(stmt, loc),
165+
// When removing storage statements, we need to remove both (#107511).
166+
if let StatementKind::StorageLive(l) | StatementKind::StorageDead(l) = stmt.kind
167+
&& self.storage_to_remove.contains(l)
168+
{
169+
stmt.make_nop();
170+
return
171+
}
172+
173+
self.super_statement(stmt, loc);
174+
175+
// Do not leave tautological assignments around.
176+
if let StatementKind::Assign(box (lhs, ref rhs)) = stmt.kind
177+
&& let Rvalue::Use(Operand::Copy(rhs) | Operand::Move(rhs)) | Rvalue::CopyForDeref(rhs) = *rhs
178+
&& lhs == rhs
179+
{
180+
stmt.make_nop();
179181
}
180182
}
181183
}

‎compiler/rustc_mir_transform/src/ssa.rs

Lines changed: 34 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -101,14 +101,15 @@ impl SsaLocals {
101101
.retain(|&local| matches!(visitor.assignments[local], Set1::One(_)));
102102
debug!(?visitor.assignment_order);
103103

104-
let copy_classes = compute_copy_classes(&mut visitor, body);
105-
106-
SsaLocals {
104+
let mut ssa = SsaLocals {
107105
assignments: visitor.assignments,
108106
assignment_order: visitor.assignment_order,
109107
direct_uses: visitor.direct_uses,
110-
copy_classes,
111-
}
108+
// This is filled by `compute_copy_classes`.
109+
copy_classes: IndexVec::default(),
110+
};
111+
compute_copy_classes(&mut ssa, body);
112+
ssa
112113
}
113114

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

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

267-
for &local in &ssa.assignment_order {
268-
debug!(?local);
269-
270-
if local == RETURN_PLACE {
271-
// `_0` is special, we cannot rename it.
272-
continue;
273-
}
274-
275-
// This is not SSA: mark that we don't know the value.
276-
debug!(assignments = ?ssa.assignments[local]);
277-
let Set1::One(LocationExtended::Plain(loc)) = ssa.assignments[local] else { continue };
278-
279-
// `loc` must point to a direct assignment to `local`.
280-
let Either::Left(stmt) = body.stmt_at(loc) else { bug!() };
281-
let Some((_target, rvalue)) = stmt.kind.as_assign() else { bug!() };
282-
assert_eq!(_target.as_local(), Some(local));
283-
269+
for (local, rvalue, _) in ssa.assignments(body) {
284270
let (Rvalue::Use(Operand::Copy(place) | Operand::Move(place)) | Rvalue::CopyForDeref(place))
285271
= rvalue
286272
else { continue };
287273

288274
let Some(rhs) = place.as_local() else { continue };
289-
let Set1::One(_) = ssa.assignments[rhs] else { continue };
275+
if !ssa.is_ssa(rhs) {
276+
continue;
277+
}
290278

291279
// We visit in `assignment_order`, ie. reverse post-order, so `rhs` has been
292280
// visited before `local`, and we just have to copy the representing local.
293-
copies[local] = copies[rhs];
294-
ssa.direct_uses[rhs] -= 1;
281+
let head = copies[rhs];
282+
283+
if local == RETURN_PLACE {
284+
// `_0` is special, we cannot rename it. Instead, rename the class of `rhs` to
285+
// `RETURN_PLACE`. This is only possible if the class head is a temporary, not an
286+
// argument.
287+
if body.local_kind(head) != LocalKind::Temp {
288+
continue;
289+
}
290+
for h in copies.iter_mut() {
291+
if *h == head {
292+
*h = RETURN_PLACE;
293+
}
294+
}
295+
} else {
296+
copies[local] = head;
297+
}
298+
direct_uses[rhs] -= 1;
295299
}
296300

297301
debug!(?copies);
298-
debug!(?ssa.direct_uses);
302+
debug!(?direct_uses);
299303

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

306-
copies
311+
ssa.direct_uses = direct_uses;
312+
ssa.copy_classes = copies;
307313
}
308314

309315
#[derive(Debug)]

‎tests/codegen/fewer-names.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ pub fn sum(x: u32, y: u32) -> u32 {
1313

1414
// NO-LABEL: define{{.*}}i32 @sum(i32 noundef %x, i32 noundef %y)
1515
// NO-NEXT: start:
16-
// NO-NEXT: %z = add i32 %y, %x
17-
// NO-NEXT: ret i32 %z
16+
// NO-NEXT: %0 = add i32 %y, %x
17+
// NO-NEXT: ret i32 %0
1818
let z = x + y;
1919
z
2020
}

‎tests/codegen/mem-replace-big-type.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,7 @@ pub struct Big([u64; 7]);
1313
pub fn replace_big(dst: &mut Big, src: Big) -> Big {
1414
// Back in 1.68, this emitted six `memcpy`s.
1515
// `read_via_copy` in 1.69 got that down to three.
16-
// `write_via_move` it was originally down to the essential two, however
17-
// with nrvo disabled it is back at 3
16+
// `write_via_move` and nvro get this down to the essential two.
1817
std::mem::replace(dst, src)
1918
}
2019

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

3633
// CHECK-NOT: call void @llvm.memcpy

‎tests/codegen/var-names.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ pub fn test(a: u32, b: u32) -> u32 {
99
// CHECK: %c = add i32 %a, %b
1010
let d = c;
1111
let e = d * a;
12-
// CHECK-NEXT: %e = mul i32 %c, %a
12+
// CHECK-NEXT: %0 = mul i32 %c, %a
1313
e
14-
// CHECK-NEXT: ret i32 %e
14+
// CHECK-NEXT: ret i32 %0
1515
}

‎tests/mir-opt/copy-prop/copy_propagation_arg.arg_src.CopyProp.diff

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,17 @@
66
let mut _0: i32; // return place in scope 0 at $DIR/copy_propagation_arg.rs:+0:27: +0:30
77
let _2: i32; // in scope 0 at $DIR/copy_propagation_arg.rs:+1:9: +1:10
88
scope 1 {
9-
debug y => _2; // in scope 1 at $DIR/copy_propagation_arg.rs:+1:9: +1:10
9+
- debug y => _2; // in scope 1 at $DIR/copy_propagation_arg.rs:+1:9: +1:10
10+
+ debug y => _0; // in scope 1 at $DIR/copy_propagation_arg.rs:+1:9: +1:10
1011
}
1112

1213
bb0: {
13-
StorageLive(_2); // scope 0 at $DIR/copy_propagation_arg.rs:+1:9: +1:10
14-
_2 = _1; // scope 0 at $DIR/copy_propagation_arg.rs:+1:13: +1:14
14+
- StorageLive(_2); // scope 0 at $DIR/copy_propagation_arg.rs:+1:9: +1:10
15+
- _2 = _1; // scope 0 at $DIR/copy_propagation_arg.rs:+1:13: +1:14
16+
+ _0 = _1; // scope 0 at $DIR/copy_propagation_arg.rs:+1:13: +1:14
1517
_1 = const 123_i32; // scope 1 at $DIR/copy_propagation_arg.rs:+2:5: +2:12
16-
_0 = _2; // scope 1 at $DIR/copy_propagation_arg.rs:+3:5: +3:6
17-
StorageDead(_2); // scope 0 at $DIR/copy_propagation_arg.rs:+4:1: +4:2
18+
- _0 = _2; // scope 1 at $DIR/copy_propagation_arg.rs:+3:5: +3:6
19+
- StorageDead(_2); // scope 0 at $DIR/copy_propagation_arg.rs:+4:1: +4:2
1820
return; // scope 0 at $DIR/copy_propagation_arg.rs:+4:2: +4:2
1921
}
2022
}

‎tests/mir-opt/inline/inline_into_box_place.main.Inline.diff

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@
1616
+ let mut _4: usize; // in scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
1717
+ let mut _5: usize; // in scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
1818
+ let mut _6: *mut u8; // in scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
19-
+ let mut _7: std::boxed::Box<std::vec::Vec<u32>>; // in scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
20-
+ let mut _8: *const std::vec::Vec<u32>; // in scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
19+
+ let mut _7: *const std::vec::Vec<u32>; // in scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
2120
+ scope 4 {
2221
+ }
2322
+ }
@@ -66,12 +65,9 @@
6665
bb3: {
6766
- StorageDead(_1); // scope 0 at $DIR/inline_into_box_place.rs:+2:1: +2:2
6867
- return; // scope 0 at $DIR/inline_into_box_place.rs:+2:2: +2:2
69-
+ StorageLive(_7); // scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
70-
+ _7 = ShallowInitBox(move _6, std::vec::Vec<u32>); // scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
71-
+ _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
72-
+ (*_8) = move _2; // scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
73-
+ _1 = move _7; // scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
74-
+ StorageDead(_7); // scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
68+
+ _1 = ShallowInitBox(move _6, std::vec::Vec<u32>); // scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
69+
+ _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
70+
+ (*_7) = move _2; // scope 3 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
7571
+ StorageDead(_2); // scope 0 at $DIR/inline_into_box_place.rs:+1:48: +1:49
7672
+ _0 = const (); // scope 0 at $DIR/inline_into_box_place.rs:+0:11: +2:2
7773
+ drop(_1) -> [return: bb1, unwind: bb2]; // scope 0 at $DIR/inline_into_box_place.rs:+2:1: +2:2

‎tests/mir-opt/inline/issue_58867_inline_as_ref_as_mut.b.Inline.after.mir

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,18 @@ fn b(_1: &mut Box<T>) -> &mut T {
88
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
99
scope 1 (inlined <Box<T> as AsMut<T>>::as_mut) { // at $DIR/issue_58867_inline_as_ref_as_mut.rs:8:7: 8:15
1010
debug self => _4; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
11-
let mut _5: &mut T; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
12-
let mut _6: std::boxed::Box<T>; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
13-
let mut _7: *const T; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
11+
let mut _5: std::boxed::Box<T>; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
12+
let mut _6: *const T; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
1413
}
1514

1615
bb0: {
1716
StorageLive(_2); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
1817
StorageLive(_3); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
1918
StorageLive(_4); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
2019
_4 = &mut (*_1); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
21-
StorageLive(_5); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:7: +1:15
22-
_6 = deref_copy (*_4); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
23-
_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
24-
_5 = &mut (*_7); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
25-
_3 = _5; // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
26-
StorageDead(_5); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:7: +1:15
20+
_5 = deref_copy (*_4); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
21+
_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
22+
_3 = &mut (*_6); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
2723
_2 = &mut (*_3); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
2824
StorageDead(_4); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:14: +1:15
2925
_0 = &mut (*_2); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15

‎tests/mir-opt/inline/issue_58867_inline_as_ref_as_mut.d.Inline.after.mir

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,21 +7,17 @@ fn d(_1: &Box<T>) -> &T {
77
let mut _3: &std::boxed::Box<T>; // in scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
88
scope 1 (inlined <Box<T> as AsRef<T>>::as_ref) { // at $DIR/issue_58867_inline_as_ref_as_mut.rs:18:7: 18:15
99
debug self => _3; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
10-
let _4: &T; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
11-
let mut _5: std::boxed::Box<T>; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
12-
let mut _6: *const T; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
10+
let mut _4: std::boxed::Box<T>; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
11+
let mut _5: *const T; // in scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
1312
}
1413

1514
bb0: {
1615
StorageLive(_2); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
1716
StorageLive(_3); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
1817
_3 = &(*_1); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
19-
StorageLive(_4); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
20-
_5 = deref_copy (*_3); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
21-
_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
22-
_4 = &(*_6); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
23-
_2 = _4; // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
24-
StorageDead(_4); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
18+
_4 = deref_copy (*_3); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
19+
_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
20+
_2 = &(*_5); // scope 1 at $SRC_DIR/alloc/src/boxed.rs:LL:COL
2521
_0 = &(*_2); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:5: +1:15
2622
StorageDead(_3); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+1:14: +1:15
2723
StorageDead(_2); // scope 0 at $DIR/issue_58867_inline_as_ref_as_mut.rs:+2:1: +2:2

0 commit comments

Comments
 (0)