Skip to content

Commit ff5be13

Browse files
committed
Auto merge of #146355 - cjgillot:gvn-union, r=jackh726
GVN: Support unions. Supporting union values is not very hard, and allows to const-prop them. r? `@ghost` for perf
2 parents 68ad253 + 96c3978 commit ff5be13

9 files changed

+118
-79
lines changed

compiler/rustc_mir_transform/src/gvn.rs

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -213,6 +213,8 @@ enum Value<'a, 'tcx> {
213213
/// An aggregate value, either tuple/closure/struct/enum.
214214
/// This does not contain unions, as we cannot reason with the value.
215215
Aggregate(VariantIdx, &'a [VnIndex]),
216+
/// A union aggregate value.
217+
Union(FieldIdx, VnIndex),
216218
/// A raw pointer aggregate built from a thin pointer and metadata.
217219
RawPtr {
218220
/// Thin pointer component. This is field 0 in MIR.
@@ -600,6 +602,21 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
600602
return None;
601603
}
602604
}
605+
Union(active_field, field) => {
606+
let field = self.evaluated[field].as_ref()?;
607+
if matches!(ty.backend_repr, BackendRepr::Scalar(..) | BackendRepr::ScalarPair(..))
608+
{
609+
let dest = self.ecx.allocate(ty, MemoryKind::Stack).discard_err()?;
610+
let field_dest = self.ecx.project_field(&dest, active_field).discard_err()?;
611+
self.ecx.copy_op(field, &field_dest).discard_err()?;
612+
self.ecx
613+
.alloc_mark_immutable(dest.ptr().provenance.unwrap().alloc_id())
614+
.discard_err()?;
615+
dest.into()
616+
} else {
617+
return None;
618+
}
619+
}
603620
RawPtr { pointer, metadata } => {
604621
let pointer = self.evaluated[pointer].as_ref()?;
605622
let metadata = self.evaluated[metadata].as_ref()?;
@@ -802,11 +819,11 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
802819
}
803820
}
804821
ProjectionElem::Downcast(name, index) => ProjectionElem::Downcast(name, index),
805-
ProjectionElem::Field(f, _) => {
806-
if let Value::Aggregate(_, fields) = self.get(value) {
807-
return Some((projection_ty, fields[f.as_usize()]));
808-
} else if let Value::Projection(outer_value, ProjectionElem::Downcast(_, read_variant)) = self.get(value)
809-
&& let Value::Aggregate(written_variant, fields) = self.get(outer_value)
822+
ProjectionElem::Field(f, _) => match self.get(value) {
823+
Value::Aggregate(_, fields) => return Some((projection_ty, fields[f.as_usize()])),
824+
Value::Union(active, field) if active == f => return Some((projection_ty, field)),
825+
Value::Projection(outer_value, ProjectionElem::Downcast(_, read_variant))
826+
if let Value::Aggregate(written_variant, fields) = self.get(outer_value)
810827
// This pass is not aware of control-flow, so we do not know whether the
811828
// replacement we are doing is actually reachable. We could be in any arm of
812829
// ```
@@ -822,12 +839,12 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
822839
// accessing the wrong variant is not UB if the enum has repr.
823840
// So it's not impossible for a series of MIR opts to generate
824841
// a downcast to an inactive variant.
825-
&& written_variant == read_variant
842+
&& written_variant == read_variant =>
826843
{
827844
return Some((projection_ty, fields[f.as_usize()]));
828845
}
829-
ProjectionElem::Field(f, ())
830-
}
846+
_ => ProjectionElem::Field(f, ()),
847+
},
831848
ProjectionElem::Index(idx) => {
832849
if let Value::Repeat(inner, _) = self.get(value) {
833850
return Some((projection_ty, inner));
@@ -1167,7 +1184,10 @@ impl<'body, 'a, 'tcx> VnState<'body, 'a, 'tcx> {
11671184
| AggregateKind::Coroutine(..) => FIRST_VARIANT,
11681185
AggregateKind::Adt(_, variant_index, _, _, None) => variant_index,
11691186
// Do not track unions.
1170-
AggregateKind::Adt(_, _, _, _, Some(_)) => return None,
1187+
AggregateKind::Adt(_, _, _, _, Some(active_field)) => {
1188+
let field = *fields.first()?;
1189+
return Some(self.insert(ty, Value::Union(active_field, field)));
1190+
}
11711191
AggregateKind::RawPtr(..) => {
11721192
assert_eq!(field_ops.len(), 2);
11731193
let [mut pointer, metadata] = fields.try_into().unwrap();

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

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,21 +28,27 @@
2828
bb0: {
2929
StorageLive(_1);
3030
StorageLive(_2);
31-
_2 = InvalidChar { int: const 1114113_u32 };
32-
_1 = copy (_2.1: char);
31+
- _2 = InvalidChar { int: const 1114113_u32 };
32+
- _1 = copy (_2.1: char);
33+
+ _2 = const InvalidChar {{ int: 1114113_u32, chr: {transmute(0x00110001): char} }};
34+
+ _1 = const {transmute(0x00110001): char};
3335
StorageDead(_2);
3436
StorageLive(_3);
3537
StorageLive(_4);
3638
StorageLive(_5);
37-
_5 = InvalidTag { int: const 4_u32 };
38-
_4 = copy (_5.1: E);
39-
_3 = [move _4];
39+
- _5 = InvalidTag { int: const 4_u32 };
40+
- _4 = copy (_5.1: E);
41+
- _3 = [move _4];
42+
+ _5 = const InvalidTag {{ int: 4_u32, e: Scalar(0x00000004): E }};
43+
+ _4 = const Scalar(0x00000004): E;
44+
+ _3 = [const Scalar(0x00000004): E];
4045
StorageDead(_4);
4146
StorageDead(_5);
4247
nop;
4348
nop;
4449
StorageLive(_8);
45-
_8 = NoVariants { int: const 0_u32 };
50+
- _8 = NoVariants { int: const 0_u32 };
51+
+ _8 = const NoVariants {{ int: 0_u32, empty: ZeroSized: Empty }};
4652
nop;
4753
nop;
4854
nop;
@@ -55,5 +61,17 @@
5561
StorageDead(_1);
5662
return;
5763
}
64+
+ }
65+
+
66+
+ ALLOC0 (size: 4, align: 4) {
67+
+ 00 00 00 00 │ ....
68+
+ }
69+
+
70+
+ ALLOC1 (size: 4, align: 4) {
71+
+ 04 00 00 00 │ ....
72+
+ }
73+
+
74+
+ ALLOC2 (size: 4, align: 4) {
75+
+ 01 00 11 00 │ ....
5876
}
5977

tests/mir-opt/const_prop/transmute.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,8 @@ pub unsafe fn invalid_bool() -> bool {
4343
// EMIT_MIR transmute.undef_union_as_integer.GVN.diff
4444
pub unsafe fn undef_union_as_integer() -> u32 {
4545
// CHECK-LABEL: fn undef_union_as_integer(
46-
// CHECK: _1 = Union32 {
47-
// CHECK: _0 = move _1 as u32 (Transmute);
46+
// CHECK: _1 = const Union32
47+
// CHECK: _0 = const {{.*}}: u32;
4848
union Union32 {
4949
value: u32,
5050
unit: (),

tests/mir-opt/const_prop/transmute.undef_union_as_integer.GVN.32bit.diff

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,16 @@
1212
- _2 = ();
1313
- _1 = Union32 { value: move _2 };
1414
+ _2 = const ();
15-
+ _1 = Union32 { value: const () };
15+
+ _1 = const Union32 {{ value: Indirect { alloc_id: ALLOC0, offset: Size(0 bytes) }: u32, unit: () }};
1616
StorageDead(_2);
17-
_0 = move _1 as u32 (Transmute);
17+
- _0 = move _1 as u32 (Transmute);
18+
+ _0 = const Indirect { alloc_id: ALLOC0, offset: Size(0 bytes) }: u32;
1819
StorageDead(_1);
1920
return;
2021
}
22+
+ }
23+
+
24+
+ ALLOC0 (size: 4, align: 4) {
25+
+ __ __ __ __ │ ░░░░
2126
}
2227

tests/mir-opt/const_prop/transmute.undef_union_as_integer.GVN.64bit.diff

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,16 @@
1212
- _2 = ();
1313
- _1 = Union32 { value: move _2 };
1414
+ _2 = const ();
15-
+ _1 = Union32 { value: const () };
15+
+ _1 = const Union32 {{ value: Indirect { alloc_id: ALLOC0, offset: Size(0 bytes) }: u32, unit: () }};
1616
StorageDead(_2);
17-
_0 = move _1 as u32 (Transmute);
17+
- _0 = move _1 as u32 (Transmute);
18+
+ _0 = const Indirect { alloc_id: ALLOC0, offset: Size(0 bytes) }: u32;
1819
StorageDead(_1);
1920
return;
2021
}
22+
+ }
23+
+
24+
+ ALLOC0 (size: 4, align: 4) {
25+
+ __ __ __ __ │ ░░░░
2126
}
2227

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
- // MIR for `main` before GVN
2+
+ // MIR for `main` after GVN
3+
4+
fn main() -> () {
5+
let mut _0: ();
6+
let _1: main::Un;
7+
let mut _2: u32;
8+
let _3: ();
9+
let mut _4: u32;
10+
scope 1 {
11+
debug un => _1;
12+
scope 3 (inlined std::mem::drop::<u32>) {
13+
}
14+
}
15+
scope 2 (inlined val) {
16+
}
17+
18+
bb0: {
19+
StorageLive(_1);
20+
- StorageLive(_2);
21+
+ nop;
22+
_2 = const 1_u32;
23+
- _1 = Un { us: move _2 };
24+
- StorageDead(_2);
25+
+ _1 = const Un {{ us: 1_u32 }};
26+
+ nop;
27+
StorageLive(_3);
28+
StorageLive(_4);
29+
- _4 = copy (_1.0: u32);
30+
+ _4 = const 1_u32;
31+
StorageDead(_4);
32+
StorageDead(_3);
33+
_0 = const ();
34+
StorageDead(_1);
35+
return;
36+
}
37+
+ }
38+
+
39+
+ ALLOC0 (size: 4, align: 4) {
40+
+ 01 00 00 00 │ ....
41+
}
42+

tests/mir-opt/dest-prop/union.rs renamed to tests/mir-opt/const_prop/union.rs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,17 @@
1-
// EMIT_MIR_FOR_EACH_PANIC_STRATEGY
21
//! Tests that we can propagate into places that are projections into unions
3-
//@ compile-flags: -Zunsound-mir-opts -C debuginfo=full
2+
//@ test-mir-pass: GVN
3+
//@ compile-flags: -Zinline-mir
4+
45
fn val() -> u32 {
56
1
67
}
78

8-
// EMIT_MIR union.main.DestinationPropagation.diff
9+
// EMIT_MIR union.main.GVN.diff
910
fn main() {
1011
// CHECK-LABEL: fn main(
11-
// CHECK: {{_.*}} = Un { us: const 1_u32 };
12+
// CHECK: debug un => [[un:_.*]];
13+
// CHECK: bb0: {
14+
// CHECK: [[un]] = const Un {{{{ us: 1_u32 }}}};
1215
union Un {
1316
us: u32,
1417
}

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

Lines changed: 0 additions & 27 deletions
This file was deleted.

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

Lines changed: 0 additions & 27 deletions
This file was deleted.

0 commit comments

Comments
 (0)