Skip to content

Commit 0b658af

Browse files
zachs18zachs18
committed
Do not store tag for single-inhabited-variant enums (if this results in a better layout)
1 parent 2955d8f commit 0b658af

File tree

4 files changed

+158
-21
lines changed

4 files changed

+158
-21
lines changed

‎compiler/rustc_abi/src/layout.rs‎

Lines changed: 148 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,54 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
197197
})
198198
}
199199

200+
pub fn simd_type_for_scalar<FieldIdx: Idx, VariantIdx: Idx, F>(
201+
&self,
202+
element: Scalar,
203+
count: u64,
204+
repr_packed: bool,
205+
) -> LayoutCalculatorResult<FieldIdx, VariantIdx, F> {
206+
let elt = element;
207+
if count == 0 {
208+
return Err(LayoutCalculatorError::ZeroLengthSimdType);
209+
} else if count > crate::MAX_SIMD_LANES {
210+
return Err(LayoutCalculatorError::OversizedSimdType {
211+
max_lanes: crate::MAX_SIMD_LANES,
212+
});
213+
}
214+
215+
// Compute the size and alignment of the vector
216+
let dl = self.cx.data_layout();
217+
let size = elt
218+
.size(&self.cx)
219+
.checked_mul(count, dl)
220+
.ok_or_else(|| LayoutCalculatorError::SizeOverflow)?;
221+
let (repr, align) = if repr_packed && !count.is_power_of_two() {
222+
// Non-power-of-two vectors have padding up to the next power-of-two.
223+
// If we're a packed repr, remove the padding while keeping the alignment as close
224+
// to a vector as possible.
225+
(BackendRepr::Memory { sized: true }, Align::max_aligned_factor(size))
226+
} else {
227+
(BackendRepr::SimdVector { element, count }, dl.llvmlike_vector_align(size))
228+
};
229+
let size = size.align_to(align);
230+
231+
Ok(LayoutData {
232+
variants: Variants::Single { index: VariantIdx::new(0), variants: None },
233+
fields: FieldsShape::Arbitrary {
234+
offsets: [Size::ZERO].into(),
235+
memory_index: [0].into(),
236+
},
237+
backend_repr: repr,
238+
largest_niche: None,
239+
uninhabited: false,
240+
size,
241+
align: AbiAlign::new(align),
242+
max_repr_align: None,
243+
unadjusted_abi_align: elt.align(&self.cx).abi,
244+
randomization_seed: (Hash64::new(count)),
245+
})
246+
}
247+
200248
/// Compute the layout for a coroutine.
201249
///
202250
/// This uses dedicated code instead of [`Self::layout_of_struct_or_enum`], as coroutine
@@ -807,6 +855,9 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
807855
});
808856
trace!(?largest_niche);
809857

858+
let single_variant_layout_eligible =
859+
!repr.inhibit_enum_layout_opt() && valid_discriminants.len() == 1;
860+
810861
// `max` is the last valid discriminant before the largest niche
811862
// `min` is the first valid discriminant after the largest niche
812863
let (max, min) = largest_niche
@@ -839,10 +890,15 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
839890
}
840891

841892
// Create the set of structs that represent each variant.
893+
let mut single_inhabited_variant_no_tag_layout = None;
842894
let mut layout_variants = variants
843895
.iter_enumerated()
844896
.map(|(i, field_layouts)| {
845897
let uninhabited = field_layouts.iter().any(|f| f.is_uninhabited());
898+
if !uninhabited && single_variant_layout_eligible {
899+
single_inhabited_variant_no_tag_layout =
900+
Some((i, self.univariant(field_layouts, repr, StructKind::AlwaysSized)));
901+
}
846902
// We don't need to encode the tag in uninhabited variants in repr(Rust) enums
847903
let struct_kind = if uninhabited && !repr.inhibit_enum_layout_opt() {
848904
StructKind::AlwaysSized
@@ -869,6 +925,73 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
869925
})
870926
.collect::<Result<IndexVec<VariantIdx, _>, _>>()?;
871927

928+
// If there is a single uninhabited variant, we can use it mostly unchanged as the layout,
929+
// without using a tag or niche.
930+
//
931+
// We do still need to modify it to make all the uninhabited variants fit so they
932+
// can be partially-initialized.
933+
//
934+
// We keep this as a prospective layout, and don't assume it's better than the tagged
935+
// layout and return it immediately; e.g. it's worse for `enum Foo { A, B(i32, !) }`
936+
// because it has no niche.
937+
let no_tag_layout = if single_variant_layout_eligible
938+
&& let Some((single_inhabited_variant_idx, Ok(mut st))) =
939+
single_inhabited_variant_no_tag_layout
940+
{
941+
// Keep track of original variant layouts (including the inhabited one)
942+
// for `offset_of!`.
943+
let mut variants = layout_variants.clone();
944+
variants[single_inhabited_variant_idx] = st.clone();
945+
946+
// We know that every other variant is uninhabited, and thus does not have a
947+
// prefix for the tag, so we can use them to find the necessary size.
948+
for (idx, layout) in layout_variants.iter_enumerated() {
949+
if idx != single_inhabited_variant_idx {
950+
st.size = cmp::max(st.size, layout.size);
951+
st.align = st.align.max(layout.align);
952+
st.max_repr_align = st.max_repr_align.max(layout.max_repr_align);
953+
st.unadjusted_abi_align =
954+
st.unadjusted_abi_align.max(layout.unadjusted_abi_align);
955+
}
956+
}
957+
958+
// Align the maximum variant size to the largest alignment.
959+
st.size = st.size.align_to(st.align.abi);
960+
961+
// If the inhabited variant's layout would use a non-Memory BackendRepr,
962+
// but we made it bigger or more-aligned due to uninhabited variants,
963+
// force it to be BackendRepr::Memory
964+
match st.backend_repr {
965+
BackendRepr::Scalar(..) | BackendRepr::ScalarPair(..) => {
966+
if st.backend_repr.scalar_size(&self.cx) != Some(st.size)
967+
|| st.backend_repr.scalar_align(&self.cx) != Some(st.align.abi)
968+
{
969+
st.backend_repr = BackendRepr::Memory { sized: true }
970+
}
971+
}
972+
BackendRepr::SimdVector { element, count } => {
973+
// FIXME: is there a better way to do this than making a copy of
974+
// `LayoutCalculator::simd_type` *just* for this?
975+
let vector_layout = self.simd_type_for_scalar::<FieldIdx, VariantIdx, _>(
976+
element,
977+
count,
978+
repr.packed(),
979+
)?;
980+
if vector_layout.size != st.size || vector_layout.align != st.align {
981+
st.backend_repr = BackendRepr::Memory { sized: true }
982+
}
983+
}
984+
BackendRepr::Memory { .. } => {}
985+
}
986+
987+
st.variants =
988+
Variants::Single { index: single_inhabited_variant_idx, variants: Some(variants) };
989+
990+
Some(st)
991+
} else {
992+
None
993+
};
994+
872995
// Align the maximum variant size to the largest alignment.
873996
size = size.align_to(align);
874997

@@ -1149,22 +1272,36 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
11491272
randomization_seed: combined_seed,
11501273
};
11511274

1152-
let best_layout = match (tagged_layout, niche_filling_layout) {
1153-
(tl, Some(nl)) => {
1154-
// Pick the smaller layout; otherwise,
1155-
// pick the layout with the larger niche; otherwise,
1156-
// pick tagged as it has simpler codegen.
1275+
// Pick the smallest layout; otherwise,
1276+
// pick the layout with the largest niche; otherwise,
1277+
// pick no_tag as it has simpler codegen than tagged and niched; otherwise,
1278+
// pick tagged as it has simpler codegen than niched.
1279+
1280+
let better_layout_or_first =
1281+
|l1: LayoutData<FieldIdx, VariantIdx>, l2: LayoutData<FieldIdx, VariantIdx>| {
11571282
use cmp::Ordering::*;
11581283
let niche_size = |l: &LayoutData<FieldIdx, VariantIdx>| {
11591284
l.largest_niche.map_or(0, |n| n.available(dl))
11601285
};
1161-
match (tl.size.cmp(&nl.size), niche_size(&tl).cmp(&niche_size(&nl))) {
1162-
(Greater, _) => nl,
1163-
(Equal, Less) => nl,
1164-
_ => tl,
1286+
match (l1.size.cmp(&l2.size), niche_size(&l1).cmp(&niche_size(&l2))) {
1287+
(Greater, _) => l2,
1288+
(Equal, Less) => l2,
1289+
_ => l1,
11651290
}
1166-
}
1167-
(tl, None) => tl,
1291+
};
1292+
1293+
let best_layout = match niche_filling_layout {
1294+
None => tagged_layout,
1295+
// Prefer tagged over niched if they have the same size and niche size,
1296+
// as the tagged layout has simpler codegen.
1297+
Some(niched_layout) => better_layout_or_first(tagged_layout, niched_layout),
1298+
};
1299+
1300+
let best_layout = match no_tag_layout {
1301+
None => best_layout,
1302+
// Prefer no-tag over tagged/niched if they have the same size and niche size,
1303+
// as the no-tag layout has simpler codegen.
1304+
Some(no_tag_layout) => better_layout_or_first(no_tag_layout, best_layout),
11681305
};
11691306

11701307
Ok(best_layout)

‎tests/ui/consts/const-eval/ub-enum.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,8 +102,8 @@ const BAD_UNINHABITED_WITH_DATA2: Result<(i32, !), (i32, Never)> = unsafe { mem:
102102

103103
// All variants have same-size data but only one inhabited.
104104
// Use `0` as constant to make behavior endianness-independent.
105-
const GOOD_SEMIINHABITED_WITH_DATA1: Result<i32, (i32, !)> = unsafe { mem::transmute(0u64) };
106-
const GOOD_SEMIINHABITED_WITH_DATA2: Result<(i32, !), i32> = unsafe { mem::transmute(1u64) };
105+
const GOOD_SEMIINHABITED_WITH_DATA1: Result<i32, (i32, !)> = unsafe { mem::transmute(0u32) };
106+
const GOOD_SEMIINHABITED_WITH_DATA2: Result<(i32, !), i32> = unsafe { mem::transmute(0u32) };
107107

108108
const TEST_ICE_89765: () = {
109109
// This is a regression test for https://github.com/rust-lang/rust/issues/89765.

‎tests/ui/layout/enum.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ enum AbsentVariantUntagged { //~ERROR: size: Size(4 bytes)
3535
// Even if uninhabited variants are not absent, the enum can still be laid out without
3636
// a tag.
3737
#[rustc_layout(size, abi)]
38-
enum UninhabitedVariantUntagged { //~ERROR: size: Size(8 bytes)
39-
//~^ ERROR: abi: ScalarPair(Initialized
38+
enum UninhabitedVariantUntagged { //~ERROR: size: Size(4 bytes)
39+
//~^ ERROR: abi: Scalar(Initialized
4040
A(i32),
4141
B(i32, !),
4242
}
@@ -53,8 +53,8 @@ enum UninhabitedVariantUntaggedBigger { //~ERROR: size: Size(8 bytes)
5353
}
5454

5555
#[rustc_layout(size, abi)]
56-
enum UninhabitedVariantWithNiche { //~ERROR: size: Size(3 bytes)
57-
//~^ERROR: abi: Memory
56+
enum UninhabitedVariantWithNiche { //~ERROR: size: Size(2 bytes)
57+
//~^ERROR: abi: ScalarPair(Initialized { value: Int(I8, false), valid_range: 0..=1 }, Initialized { value: Int(I8, true), valid_range: 0..=255 })
5858
A(i8, bool),
5959
B(u8, u8, !),
6060
}

‎tests/ui/layout/enum.stderr‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,13 @@ error: abi: Scalar(Initialized { value: Int(I32, true), valid_range: 0..=4294967
2828
LL | enum AbsentVariantUntagged {
2929
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
3030

31-
error: size: Size(8 bytes)
31+
error: size: Size(4 bytes)
3232
--> $DIR/enum.rs:38:1
3333
|
3434
LL | enum UninhabitedVariantUntagged {
3535
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
3636

37-
error: abi: ScalarPair(Initialized { value: Int(I32, false), valid_range: 0..=0 }, Initialized { value: Int(I32, true), valid_range: 0..=4294967295 })
37+
error: abi: Scalar(Initialized { value: Int(I32, true), valid_range: 0..=4294967295 })
3838
--> $DIR/enum.rs:38:1
3939
|
4040
LL | enum UninhabitedVariantUntagged {
@@ -52,13 +52,13 @@ error: abi: ScalarPair(Initialized { value: Int(I8, false), valid_range: 0..=0 }
5252
LL | enum UninhabitedVariantUntaggedBigger {
5353
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
5454

55-
error: size: Size(3 bytes)
55+
error: size: Size(2 bytes)
5656
--> $DIR/enum.rs:56:1
5757
|
5858
LL | enum UninhabitedVariantWithNiche {
5959
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
6060

61-
error: abi: Memory { sized: true }
61+
error: abi: ScalarPair(Initialized { value: Int(I8, false), valid_range: 0..=1 }, Initialized { value: Int(I8, true), valid_range: 0..=255 })
6262
--> $DIR/enum.rs:56:1
6363
|
6464
LL | enum UninhabitedVariantWithNiche {

0 commit comments

Comments
 (0)