Skip to content

Commit 277319f

Browse files
borsbors
committed
Auto merge of rust-lang#139327 - cjgillot:gvn-place, r=<try>
Allow GVN to produce places and not just locals. That may be too big of a hammer, as we may introduce new deref projections (possible UB footgun + probably not good for perf). r? `@ghost` for perf Fixes rust-lang#138936 cc `@scottmcm` `@dianqk`
2 parents 946aea0 + 731a3e6 commit 277319f

20 files changed

+143
-67
lines changed

‎compiler/rustc_mir_transform/src/gvn.rs

+37-25
Original file line numberDiff line numberDiff line change
@@ -989,26 +989,15 @@ impl<'body, 'tcx> VnState<'body, 'tcx> {
989989
}
990990

991991
let tcx = self.tcx;
992-
let mut projection = SmallVec::<[PlaceElem<'tcx>; 1]>::new();
993-
loop {
994-
if let Some(local) = self.try_as_local(copy_from_local_value, location) {
995-
projection.reverse();
996-
let place = Place { local, projection: tcx.mk_place_elems(projection.as_slice()) };
997-
if rvalue.ty(self.local_decls, tcx) == place.ty(self.local_decls, tcx).ty {
998-
self.reused_locals.insert(local);
999-
*rvalue = Rvalue::Use(Operand::Copy(place));
1000-
return Some(copy_from_value);
1001-
}
1002-
return None;
1003-
} else if let Value::Projection(pointer, proj) = *self.get(copy_from_local_value)
1004-
&& let Some(proj) = self.try_as_place_elem(proj, location)
1005-
{
1006-
projection.push(proj);
1007-
copy_from_local_value = pointer;
1008-
} else {
1009-
return None;
992+
if let Some(place) = self.try_as_place(copy_from_local_value, location) {
993+
if rvalue.ty(self.local_decls, tcx) == place.ty(self.local_decls, tcx).ty {
994+
self.reused_locals.insert(place.local);
995+
*rvalue = Rvalue::Use(Operand::Copy(place));
996+
return Some(copy_from_local_value);
1010997
}
1011998
}
999+
1000+
None
10121001
}
10131002

10141003
fn simplify_aggregate(
@@ -1688,9 +1677,9 @@ impl<'tcx> VnState<'_, 'tcx> {
16881677
fn try_as_operand(&mut self, index: VnIndex, location: Location) -> Option<Operand<'tcx>> {
16891678
if let Some(const_) = self.try_as_constant(index) {
16901679
Some(Operand::Constant(Box::new(const_)))
1691-
} else if let Some(local) = self.try_as_local(index, location) {
1692-
self.reused_locals.insert(local);
1693-
Some(Operand::Copy(local.into()))
1680+
} else if let Some(place) = self.try_as_place(index, location) {
1681+
self.reused_locals.insert(place.local);
1682+
Some(Operand::Copy(place))
16941683
} else {
16951684
None
16961685
}
@@ -1723,6 +1712,29 @@ impl<'tcx> VnState<'_, 'tcx> {
17231712
Some(ConstOperand { span: DUMMY_SP, user_ty: None, const_ })
17241713
}
17251714

1715+
/// Construct a place which holds the same value as `index` and for which all locals strictly
1716+
/// dominate `loc`. If you used this place, add its base local to `reused_locals` to remove
1717+
/// storage statements.
1718+
#[instrument(level = "trace", skip(self), ret)]
1719+
fn try_as_place(&mut self, mut index: VnIndex, loc: Location) -> Option<Place<'tcx>> {
1720+
let mut projection = SmallVec::<[PlaceElem<'tcx>; 1]>::new();
1721+
loop {
1722+
if let Some(local) = self.try_as_local(index, loc) {
1723+
projection.reverse();
1724+
let place =
1725+
Place { local, projection: self.tcx.mk_place_elems(projection.as_slice()) };
1726+
return Some(place);
1727+
} else if let Value::Projection(pointer, proj) = *self.get(index)
1728+
&& let Some(proj) = self.try_as_place_elem(proj, loc)
1729+
{
1730+
projection.push(proj);
1731+
index = pointer;
1732+
} else {
1733+
return None;
1734+
}
1735+
}
1736+
}
1737+
17261738
/// If there is a local which is assigned `index`, and its assignment strictly dominates `loc`,
17271739
/// return it. If you used this local, add it to `reused_locals` to remove storage statements.
17281740
fn try_as_local(&mut self, index: VnIndex, loc: Location) -> Option<Local> {
@@ -1764,11 +1776,11 @@ impl<'tcx> MutVisitor<'tcx> for VnState<'_, 'tcx> {
17641776

17651777
if let Some(const_) = self.try_as_constant(value) {
17661778
*rvalue = Rvalue::Use(Operand::Constant(Box::new(const_)));
1767-
} else if let Some(local) = self.try_as_local(value, location)
1768-
&& *rvalue != Rvalue::Use(Operand::Move(local.into()))
1779+
} else if let Some(place) = self.try_as_place(value, location)
1780+
&& *rvalue != Rvalue::Use(Operand::Move(place))
17691781
{
1770-
*rvalue = Rvalue::Use(Operand::Copy(local.into()));
1771-
self.reused_locals.insert(local);
1782+
*rvalue = Rvalue::Use(Operand::Copy(place));
1783+
self.reused_locals.insert(place.local);
17721784
}
17731785

17741786
return;

‎tests/mir-opt/const_prop/invalid_constant.main.GVN.diff

+8-4
Original file line numberDiff line numberDiff line change
@@ -27,18 +27,22 @@
2727

2828
bb0: {
2929
StorageLive(_1);
30-
StorageLive(_2);
30+
- StorageLive(_2);
31+
+ nop;
3132
_2 = InvalidChar { int: const 1114113_u32 };
3233
_1 = copy (_2.1: char);
33-
StorageDead(_2);
34+
- StorageDead(_2);
35+
+ nop;
3436
StorageLive(_3);
3537
StorageLive(_4);
36-
StorageLive(_5);
38+
- StorageLive(_5);
39+
+ nop;
3740
_5 = InvalidTag { int: const 4_u32 };
3841
_4 = copy (_5.1: E);
3942
_3 = [move _4];
4043
StorageDead(_4);
41-
StorageDead(_5);
44+
- StorageDead(_5);
45+
+ nop;
4246
nop;
4347
nop;
4448
StorageLive(_8);

‎tests/mir-opt/dest-prop/union.main.DestinationPropagation.panic-abort.diff

-2
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,13 @@
1616
}
1717

1818
bb0: {
19-
StorageLive(_1);
2019
StorageLive(_2);
2120
nop;
2221
_1 = Un { us: const 1_u32 };
2322
StorageDead(_2);
2423
StorageLive(_3);
2524
_3 = copy (_1.0: u32);
2625
StorageDead(_3);
27-
StorageDead(_1);
2826
return;
2927
}
3028
}

‎tests/mir-opt/dest-prop/union.main.DestinationPropagation.panic-unwind.diff

-2
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,13 @@
1616
}
1717

1818
bb0: {
19-
StorageLive(_1);
2019
StorageLive(_2);
2120
nop;
2221
_1 = Un { us: const 1_u32 };
2322
StorageDead(_2);
2423
StorageLive(_3);
2524
_3 = copy (_1.0: u32);
2625
StorageDead(_3);
27-
StorageDead(_1);
2826
return;
2927
}
3028
}

‎tests/mir-opt/dont_reset_cast_kind_without_updating_operand.test.GVN.32bit.panic-abort.diff

+5-3
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@
8080
+ _11 = const 0_usize;
8181
+ _12 = const 1_usize;
8282
StorageLive(_14);
83+
- StorageLive(_15);
84+
+ nop;
8385
StorageLive(_16);
8486
StorageLive(_17);
8587
StorageLive(_19);
@@ -109,9 +111,10 @@
109111
_22 = copy _17 as *mut [u8] (Transmute);
110112
_13 = copy _22 as *mut u8 (PtrToPtr);
111113
StorageDead(_22);
112-
StorageDead(_15);
113114
StorageDead(_17);
114115
StorageDead(_16);
116+
- StorageDead(_15);
117+
+ nop;
115118
StorageDead(_14);
116119
_3 = ShallowInitBox(move _13, ());
117120
StorageDead(_13);
@@ -141,7 +144,7 @@
141144
StorageLive(_8);
142145
_8 = copy _5;
143146
- _7 = copy _8 as *mut () (PtrToPtr);
144-
+ _7 = copy _5 as *mut () (PtrToPtr);
147+
+ _7 = copy ((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>) as *mut () (Transmute);
145148
StorageDead(_8);
146149
StorageDead(_7);
147150
- StorageDead(_5);
@@ -163,7 +166,6 @@
163166
+ _21 = const std::ptr::Alignment(std::ptr::alignment::AlignmentEnum::_Align1Shl0);
164167
+ _14 = const Layout {{ size: 0_usize, align: std::ptr::Alignment(std::ptr::alignment::AlignmentEnum::_Align1Shl0) }};
165168
StorageDead(_21);
166-
StorageLive(_15);
167169
- _15 = std::alloc::Global::alloc_impl(const alloc::alloc::exchange_malloc::promoted[0], copy _14, const false) -> [return: bb7, unwind unreachable];
168170
+ _15 = std::alloc::Global::alloc_impl(const alloc::alloc::exchange_malloc::promoted[0], const Layout {{ size: 0_usize, align: std::ptr::Alignment(std::ptr::alignment::AlignmentEnum::_Align1Shl0) }}, const false) -> [return: bb7, unwind unreachable];
169171
}

‎tests/mir-opt/dont_reset_cast_kind_without_updating_operand.test.GVN.32bit.panic-unwind.diff

+1-1
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@
6666
StorageLive(_8);
6767
_8 = copy _5;
6868
- _7 = copy _8 as *mut () (PtrToPtr);
69-
+ _7 = copy _5 as *mut () (PtrToPtr);
69+
+ _7 = copy ((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>) as *mut () (Transmute);
7070
StorageDead(_8);
7171
StorageDead(_7);
7272
- StorageDead(_5);

‎tests/mir-opt/dont_reset_cast_kind_without_updating_operand.test.GVN.64bit.panic-abort.diff

+5-3
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@
8080
+ _11 = const 0_usize;
8181
+ _12 = const 1_usize;
8282
StorageLive(_14);
83+
- StorageLive(_15);
84+
+ nop;
8385
StorageLive(_16);
8486
StorageLive(_17);
8587
StorageLive(_19);
@@ -109,9 +111,10 @@
109111
_22 = copy _17 as *mut [u8] (Transmute);
110112
_13 = copy _22 as *mut u8 (PtrToPtr);
111113
StorageDead(_22);
112-
StorageDead(_15);
113114
StorageDead(_17);
114115
StorageDead(_16);
116+
- StorageDead(_15);
117+
+ nop;
115118
StorageDead(_14);
116119
_3 = ShallowInitBox(move _13, ());
117120
StorageDead(_13);
@@ -141,7 +144,7 @@
141144
StorageLive(_8);
142145
_8 = copy _5;
143146
- _7 = copy _8 as *mut () (PtrToPtr);
144-
+ _7 = copy _5 as *mut () (PtrToPtr);
147+
+ _7 = copy ((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>) as *mut () (Transmute);
145148
StorageDead(_8);
146149
StorageDead(_7);
147150
- StorageDead(_5);
@@ -163,7 +166,6 @@
163166
+ _21 = const std::ptr::Alignment(std::ptr::alignment::AlignmentEnum::_Align1Shl0);
164167
+ _14 = const Layout {{ size: 0_usize, align: std::ptr::Alignment(std::ptr::alignment::AlignmentEnum::_Align1Shl0) }};
165168
StorageDead(_21);
166-
StorageLive(_15);
167169
- _15 = std::alloc::Global::alloc_impl(const alloc::alloc::exchange_malloc::promoted[0], copy _14, const false) -> [return: bb7, unwind unreachable];
168170
+ _15 = std::alloc::Global::alloc_impl(const alloc::alloc::exchange_malloc::promoted[0], const Layout {{ size: 0_usize, align: std::ptr::Alignment(std::ptr::alignment::AlignmentEnum::_Align1Shl0) }}, const false) -> [return: bb7, unwind unreachable];
169171
}

‎tests/mir-opt/dont_reset_cast_kind_without_updating_operand.test.GVN.64bit.panic-unwind.diff

+1-1
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@
6666
StorageLive(_8);
6767
_8 = copy _5;
6868
- _7 = copy _8 as *mut () (PtrToPtr);
69-
+ _7 = copy _5 as *mut () (PtrToPtr);
69+
+ _7 = copy ((_9.0: std::ptr::Unique<()>).0: std::ptr::NonNull<()>) as *mut () (Transmute);
7070
StorageDead(_8);
7171
StorageDead(_7);
7272
- StorageDead(_5);

‎tests/mir-opt/early_otherwise_branch_unwind.poll.EarlyOtherwiseBranch.diff

+1-1
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@
6262

6363
bb7: {
6464
StorageLive(_5);
65-
_5 = move ((((((_1 as Ready).0: std::result::Result<std::option::Option<std::vec::Vec<u8>>, u8>) as Ok).0: std::option::Option<std::vec::Vec<u8>>) as Some).0: std::vec::Vec<u8>);
65+
_5 = copy ((((((_1 as Ready).0: std::result::Result<std::option::Option<std::vec::Vec<u8>>, u8>) as Ok).0: std::option::Option<std::vec::Vec<u8>>) as Some).0: std::vec::Vec<u8>);
6666
_0 = const ();
6767
drop(_5) -> [return: bb8, unwind: bb20];
6868
}

‎tests/mir-opt/gvn_copy_constant_projection.compare_constant_index.GVN.diff

+58
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
- // MIR for `compare_constant_index` before GVN
2+
+ // MIR for `compare_constant_index` after GVN
3+
4+
fn compare_constant_index(_1: [i32; 1], _2: [i32; 1]) -> std::cmp::Ordering {
5+
debug x => _1;
6+
debug y => _2;
7+
let mut _0: std::cmp::Ordering;
8+
let _3: &i32;
9+
let _4: usize;
10+
let mut _5: bool;
11+
let _6: &i32;
12+
let _7: usize;
13+
let mut _8: bool;
14+
scope 1 (inlined std::cmp::impls::<impl Ord for i32>::cmp) {
15+
let mut _9: i32;
16+
let mut _10: i32;
17+
}
18+
19+
bb0: {
20+
- StorageLive(_4);
21+
+ nop;
22+
_4 = const 0_usize;
23+
- _5 = Lt(copy _4, const 1_usize);
24+
- assert(move _5, "index out of bounds: the length is {} but the index is {}", const 1_usize, copy _4) -> [success: bb1, unwind unreachable];
25+
+ _5 = const true;
26+
+ assert(const true, "index out of bounds: the length is {} but the index is {}", const 1_usize, const 0_usize) -> [success: bb1, unwind unreachable];
27+
}
28+
29+
bb1: {
30+
- _3 = &_1[_4];
31+
+ _3 = &_1[0 of 1];
32+
StorageLive(_7);
33+
_7 = const 0_usize;
34+
- _8 = Lt(copy _7, const 1_usize);
35+
- assert(move _8, "index out of bounds: the length is {} but the index is {}", const 1_usize, copy _7) -> [success: bb2, unwind unreachable];
36+
+ _8 = const true;
37+
+ assert(const true, "index out of bounds: the length is {} but the index is {}", const 1_usize, const 0_usize) -> [success: bb2, unwind unreachable];
38+
}
39+
40+
bb2: {
41+
- _6 = &_2[_7];
42+
+ _6 = &_2[0 of 1];
43+
StorageLive(_9);
44+
- _9 = copy (*_3);
45+
+ _9 = copy _1[0 of 1];
46+
StorageLive(_10);
47+
- _10 = copy (*_6);
48+
+ _10 = copy _2[0 of 1];
49+
_0 = Cmp(move _9, move _10);
50+
StorageDead(_10);
51+
StorageDead(_9);
52+
StorageDead(_7);
53+
- StorageDead(_4);
54+
+ nop;
55+
return;
56+
}
57+
}
58+

‎tests/mir-opt/gvn_copy_constant_projection.rs

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
//@ compile-flags: -Zunsound-mir-opts
2+
// FIXME: see <https://github.com/rust-lang/rust/issues/132353>
3+
4+
use std::cmp::Ordering;
5+
fn compare_constant_index(x: [i32; 1], y: [i32; 1]) -> Ordering {
6+
// CHECK-LABEL: fn compare_constant_index(
7+
// CHECK-NOT: (*{{_.*}});
8+
// CHECK: [[lhs:_.*]] = copy _1[0 of 1];
9+
// CHECK-NOT: (*{{_.*}});
10+
// CHECK: [[rhs:_.*]] = copy _2[0 of 1];
11+
// CHECK: _0 = Cmp(move [[lhs]], move [[rhs]]);
12+
Ord::cmp(&x[0], &y[0])
13+
}
14+
15+
fn main() {
16+
compare_constant_index([1], [2]);
17+
}
18+
19+
// EMIT_MIR gvn_copy_constant_projection.compare_constant_index.GVN.diff

‎tests/mir-opt/gvn_uninhabited.f.GVN.panic-abort.diff

+4-2
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@
1313
}
1414

1515
bb0: {
16-
StorageLive(_2);
16+
- StorageLive(_2);
17+
+ nop;
1718
StorageLive(_3);
1819
_5 = const f::promoted[0];
1920
_3 = &(*_5);
@@ -22,7 +23,8 @@
2223
+ nop;
2324
_1 = copy ((_2 as A).1: u32);
2425
StorageDead(_3);
25-
StorageDead(_2);
26+
- StorageDead(_2);
27+
+ nop;
2628
_0 = copy _1;
2729
- StorageDead(_1);
2830
+ nop;

‎tests/mir-opt/gvn_uninhabited.f.GVN.panic-unwind.diff

+4-2
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,8 @@
1313
}
1414

1515
bb0: {
16-
StorageLive(_2);
16+
- StorageLive(_2);
17+
+ nop;
1718
StorageLive(_3);
1819
_5 = const f::promoted[0];
1920
_3 = &(*_5);
@@ -22,7 +23,8 @@
2223
+ nop;
2324
_1 = copy ((_2 as A).1: u32);
2425
StorageDead(_3);
25-
StorageDead(_2);
26+
- StorageDead(_2);
27+
+ nop;
2628
_0 = copy _1;
2729
- StorageDead(_1);
2830
+ nop;

‎tests/mir-opt/pre-codegen/checked_ops.step_forward.PreCodegen.after.panic-abort.mir

-3
Original file line numberDiff line numberDiff line change
@@ -42,14 +42,12 @@ fn step_forward(_1: u16, _2: usize) -> u16 {
4242
_4 = copy _2 as u16 (IntToInt);
4343
StorageDead(_3);
4444
StorageLive(_6);
45-
StorageLive(_5);
4645
_5 = AddWithOverflow(copy _1, copy _4);
4746
_6 = copy (_5.1: bool);
4847
switchInt(copy _6) -> [0: bb2, otherwise: bb3];
4948
}
5049

5150
bb2: {
52-
StorageDead(_5);
5351
StorageDead(_6);
5452
goto -> bb7;
5553
}
@@ -59,7 +57,6 @@ fn step_forward(_1: u16, _2: usize) -> u16 {
5957
}
6058

6159
bb4: {
62-
StorageDead(_5);
6360
StorageDead(_6);
6461
goto -> bb6;
6562
}

0 commit comments

Comments
 (0)