Skip to content

Commit 7af5de3

Browse files
committed
make enum size not depend on the order of variants
1 parent c0838c8 commit 7af5de3

File tree

7 files changed

+323
-140
lines changed

7 files changed

+323
-140
lines changed

compiler/rustc_abi/src/layout.rs

+12-4
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
192192
pub fn layout_of_struct_or_enum<
193193
'a,
194194
FieldIdx: Idx,
195-
VariantIdx: Idx,
195+
VariantIdx: Idx + PartialOrd,
196196
F: Deref<Target = &'a LayoutS<FieldIdx, VariantIdx>> + fmt::Debug + Copy,
197197
>(
198198
&self,
@@ -464,7 +464,7 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
464464
fn layout_of_enum<
465465
'a,
466466
FieldIdx: Idx,
467-
VariantIdx: Idx,
467+
VariantIdx: Idx + PartialOrd,
468468
F: Deref<Target = &'a LayoutS<FieldIdx, VariantIdx>> + fmt::Debug + Copy,
469469
>(
470470
&self,
@@ -524,8 +524,16 @@ impl<Cx: HasDataLayout> LayoutCalculator<Cx> {
524524
let niche_variants = all_indices.clone().find(|v| needs_disc(*v)).unwrap()
525525
..=all_indices.rev().find(|v| needs_disc(*v)).unwrap();
526526

527-
let count =
528-
(niche_variants.end().index() as u128 - niche_variants.start().index() as u128) + 1;
527+
let count = {
528+
let niche_variants_len = (niche_variants.end().index() as u128
529+
- niche_variants.start().index() as u128)
530+
+ 1;
531+
if niche_variants.contains(&largest_variant_index) {
532+
niche_variants_len - 1
533+
} else {
534+
niche_variants_len
535+
}
536+
};
529537

530538
// Use the largest niche in the largest variant.
531539
let niche = variant_layouts[largest_variant_index].largest_niche?;

compiler/rustc_abi/src/lib.rs

+50-6
Original file line numberDiff line numberDiff line change
@@ -1475,15 +1475,59 @@ pub enum TagEncoding<VariantIdx: Idx> {
14751475
Direct,
14761476

14771477
/// Niche (values invalid for a type) encoding the discriminant:
1478-
/// Discriminant and variant index coincide.
1478+
/// Discriminant and variant index doesn't always coincide.
1479+
///
14791480
/// The variant `untagged_variant` contains a niche at an arbitrary
14801481
/// offset (field `tag_field` of the enum), which for a variant with
1481-
/// discriminant `d` is set to
1482-
/// `(d - niche_variants.start).wrapping_add(niche_start)`.
1482+
/// discriminant `d` is set to `d.wrapping_add(niche_start)`.
14831483
///
1484-
/// For example, `Option<(usize, &T)>` is represented such that
1485-
/// `None` has a null pointer for the second tuple field, and
1486-
/// `Some` is the identity function (with a non-null reference).
1484+
/// As for how to compute the discriminant, we have an optimization here that we allocate discriminant
1485+
/// value starting from the variant after the `untagged_variant` when the `untagged_variant` is
1486+
/// contained in `niche_variants`' range. Thus the `untagged_variant` won't be allocated with a
1487+
/// unneeded discriminant. Motivation for this is issue #117238.
1488+
/// For example,
1489+
/// ```rust
1490+
/// enum {
1491+
/// A, // 1
1492+
/// B, // 2
1493+
/// C(bool), // untagged_variant, no discriminant
1494+
/// D, // has a discriminant of 0
1495+
/// }
1496+
/// ```
1497+
/// The algorithm is as follows:
1498+
/// ```rust
1499+
/// // We ignore leading and trailing variants that don't need discriminants.
1500+
/// adjusted_len = niche_variants.end - niche_variants.start + 1
1501+
/// adjusted_index = variant_index - niche_variants.start
1502+
/// d = if niche_variants.contains(untagged_variant) {
1503+
/// adjusted_untagged_index = untagged_variant - niche_variants.start
1504+
/// (adjusted_index + adjusted_len - adjusted_untagged_index) % adjusted_len - 1
1505+
/// } else {
1506+
/// adjusted_index
1507+
/// }
1508+
/// tag_value = d.wrapping_add(niche_start)
1509+
/// ```
1510+
/// To load variant index from tag value:
1511+
/// ```rust
1512+
/// adjusted_len = niche_variants.end - niche_variants.start + 1
1513+
/// d = tag_value.wrapping_sub(niche_start)
1514+
/// variant_index = if niche_variants.contains(untagged_variant) {
1515+
/// if d < adjusted_len - 1 {
1516+
/// adjusted_untagged_index = untagged_variant - niche_variants.start
1517+
/// (d + 1 + adjusted_untagged_index) % adjusted_len + niche_variants.start
1518+
/// } else {
1519+
/// // When the discriminant is larger than the number of variants having
1520+
/// // discriminant, we know it represents the untagged_variant.
1521+
/// untagged_variant
1522+
/// }
1523+
/// } else {
1524+
/// if d < adjusted_len {
1525+
/// d + niche_variants.start
1526+
/// } else {
1527+
/// untagged_variant
1528+
/// }
1529+
/// }
1530+
/// ```
14871531
Niche {
14881532
untagged_variant: VariantIdx,
14891533
niche_variants: RangeInclusive<VariantIdx>,

compiler/rustc_codegen_cranelift/src/discriminant.rs

+105-64
Original file line numberDiff line numberDiff line change
@@ -51,10 +51,20 @@ pub(crate) fn codegen_set_discriminant<'tcx>(
5151
variants: _,
5252
} => {
5353
if variant_index != untagged_variant {
54+
let discr_len = niche_variants.end().index() - niche_variants.start().index() + 1;
55+
let adj_idx = variant_index.index() - niche_variants.start().index();
56+
5457
let niche = place.place_field(fx, FieldIdx::new(tag_field));
5558
let niche_type = fx.clif_type(niche.layout().ty).unwrap();
56-
let niche_value = variant_index.as_u32() - niche_variants.start().as_u32();
57-
let niche_value = (niche_value as u128).wrapping_add(niche_start);
59+
60+
let discr = if niche_variants.contains(&untagged_variant) {
61+
let adj_untagged_idx =
62+
untagged_variant.index() - niche_variants.start().index();
63+
(adj_idx + discr_len - adj_untagged_idx) % discr_len - 1;
64+
} else {
65+
adj_idx
66+
};
67+
let niche_value = (discr as u128).wrapping_add(niche_start);
5868
let niche_value = match niche_type {
5969
types::I128 => {
6070
let lsb = fx.bcx.ins().iconst(types::I64, niche_value as u64 as i64);
@@ -130,72 +140,103 @@ pub(crate) fn codegen_get_discriminant<'tcx>(
130140
dest.write_cvalue(fx, res);
131141
}
132142
TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start } => {
133-
let relative_max = niche_variants.end().as_u32() - niche_variants.start().as_u32();
134-
135-
// We have a subrange `niche_start..=niche_end` inside `range`.
136-
// If the value of the tag is inside this subrange, it's a
137-
// "niche value", an increment of the discriminant. Otherwise it
138-
// indicates the untagged variant.
139-
// A general algorithm to extract the discriminant from the tag
140-
// is:
141-
// relative_tag = tag - niche_start
142-
// is_niche = relative_tag <= (ule) relative_max
143-
// discr = if is_niche {
144-
// cast(relative_tag) + niche_variants.start()
145-
// } else {
146-
// untagged_variant
147-
// }
148-
// However, we will likely be able to emit simpler code.
149-
150-
let (is_niche, tagged_discr, delta) = if relative_max == 0 {
151-
// Best case scenario: only one tagged variant. This will
152-
// likely become just a comparison and a jump.
153-
// The algorithm is:
154-
// is_niche = tag == niche_start
155-
// discr = if is_niche {
156-
// niche_start
157-
// } else {
158-
// untagged_variant
159-
// }
160-
let is_niche = codegen_icmp_imm(fx, IntCC::Equal, tag, niche_start as i128);
143+
// See the algorithm explanation in the definition of `TagEncoding::Niche`.
144+
let discr_len = niche_variants.end().index() - niche_variants.start().index() + 1;
145+
146+
let niche_start = match fx.bcx.func.dfg.value_type(tag) {
147+
types::I128 => {
148+
let lsb = fx.bcx.ins().iconst(types::I64, niche_start as u64 as i64);
149+
let msb = fx.bcx.ins().iconst(types::I64, (niche_start >> 64) as u64 as i64);
150+
fx.bcx.ins().iconcat(lsb, msb)
151+
}
152+
ty => fx.bcx.ins().iconst(ty, niche_start as i64),
153+
};
154+
155+
let (is_niche, tagged_discr) = if discr_len == 1 {
156+
// Special case where we only have a single tagged variant.
157+
// The untagged variant can't be contained in niche_variant's range in this case.
158+
// Thus the discriminant of the only tagged variant is 0 and its variant index
159+
// is the start of niche_variants.
160+
let is_niche = codegen_icmp_imm(fx, IntCC::Equal, tag, niche_start);
161161
let tagged_discr =
162162
fx.bcx.ins().iconst(cast_to, niche_variants.start().as_u32() as i64);
163-
(is_niche, tagged_discr, 0)
163+
(is_niche, tagged_discr)
164164
} else {
165-
// The special cases don't apply, so we'll have to go with
166-
// the general algorithm.
167-
let niche_start = match fx.bcx.func.dfg.value_type(tag) {
168-
types::I128 => {
169-
let lsb = fx.bcx.ins().iconst(types::I64, niche_start as u64 as i64);
170-
let msb =
171-
fx.bcx.ins().iconst(types::I64, (niche_start >> 64) as u64 as i64);
172-
fx.bcx.ins().iconcat(lsb, msb)
173-
}
174-
ty => fx.bcx.ins().iconst(ty, niche_start as i64),
175-
};
176-
let relative_discr = fx.bcx.ins().isub(tag, niche_start);
177-
let cast_tag = clif_intcast(fx, relative_discr, cast_to, false);
178-
let is_niche = crate::common::codegen_icmp_imm(
179-
fx,
180-
IntCC::UnsignedLessThanOrEqual,
181-
relative_discr,
182-
i128::from(relative_max),
183-
);
184-
(is_niche, cast_tag, niche_variants.start().as_u32() as u128)
185-
};
165+
// General case.
166+
let discr = fx.bcx.ins().isub(tag, niche_start);
167+
let tagged_discr = clif_intcast(fx, relative_discr, cast_to, false);
168+
if niche_variants.contains(&untagged_variant) {
169+
let is_niche = crate::common::codegen_icmp_imm(
170+
fx,
171+
IntCC::UnsignedLessThan,
172+
discr,
173+
i128::from(discr_len - 1),
174+
);
175+
let adj_untagged_idx =
176+
untagged_variant.index() - niche_variants.start().index();
177+
let untagged_delta = 1 + adj_untagged_idx;
178+
let untagged_delta = match cast_to {
179+
types::I128 => {
180+
let lsb = fx.bcx.ins().iconst(types::I64, untagged_delta as u64 as i64);
181+
let msb = fx
182+
.bcx
183+
.ins()
184+
.iconst(types::I64, (untagged_delta >> 64) as u64 as i64);
185+
fx.bcx.ins().iconcat(lsb, msb)
186+
}
187+
ty => fx.bcx.ins().iconst(ty, untagged_delta as i64),
188+
};
189+
let tagged_discr = fx.bcx.ins().iadd(tagged_discr, untagged_delta);
186190

187-
let tagged_discr = if delta == 0 {
188-
tagged_discr
189-
} else {
190-
let delta = match cast_to {
191-
types::I128 => {
192-
let lsb = fx.bcx.ins().iconst(types::I64, delta as u64 as i64);
193-
let msb = fx.bcx.ins().iconst(types::I64, (delta >> 64) as u64 as i64);
194-
fx.bcx.ins().iconcat(lsb, msb)
195-
}
196-
ty => fx.bcx.ins().iconst(ty, delta as i64),
197-
};
198-
fx.bcx.ins().iadd(tagged_discr, delta)
191+
let discr_len = match cast_to {
192+
types::I128 => {
193+
let lsb = fx.bcx.ins().iconst(types::I64, discr_len as u64 as i64);
194+
let msb =
195+
fx.bcx.ins().iconst(types::I64, (discr_len >> 64) as u64 as i64);
196+
fx.bcx.ins().iconcat(lsb, msb)
197+
}
198+
ty => fx.bcx.ins().iconst(ty, discr_len as i64),
199+
};
200+
let tagged_discr = fx.bcx.ins().urem(tagged_discr, discr_len);
201+
202+
let niche_variants_start = niche_variants.start().index();
203+
let niche_variants_start = match cast_to {
204+
types::I128 => {
205+
let lsb =
206+
fx.bcx.ins().iconst(types::I64, niche_variants_start as u64 as i64);
207+
let msb = fx
208+
.bcx
209+
.ins()
210+
.iconst(types::I64, (niche_variants_start >> 64) as u64 as i64);
211+
fx.bcx.ins().iconcat(lsb, msb)
212+
}
213+
ty => fx.bcx.ins().iconst(ty, niche_variants_start as i64),
214+
};
215+
let tagged_discr = fx.bcx.ins().iadd(tagged_discr, niche_variants_start);
216+
(is_niche, tagged_discr)
217+
} else {
218+
let is_niche = crate::common::codegen_icmp_imm(
219+
fx,
220+
IntCC::UnsignedLessThan,
221+
discr,
222+
i128::from(discr_len - 1),
223+
);
224+
let niche_variants_start = niche_variants.start().index();
225+
let niche_variants_start = match cast_to {
226+
types::I128 => {
227+
let lsb =
228+
fx.bcx.ins().iconst(types::I64, niche_variants_start as u64 as i64);
229+
let msb = fx
230+
.bcx
231+
.ins()
232+
.iconst(types::I64, (niche_variants_start >> 64) as u64 as i64);
233+
fx.bcx.ins().iconcat(lsb, msb)
234+
}
235+
ty => fx.bcx.ins().iconst(ty, niche_variants_start as i64),
236+
};
237+
let tagged_discr = fx.bcx.ins().iadd(tagged_discr, niche_variants_start);
238+
(is_niche, tagged_discr)
239+
}
199240
};
200241

201242
let untagged_variant = if cast_to == types::I128 {

compiler/rustc_codegen_llvm/src/debuginfo/metadata/enums/mod.rs

+11-3
Original file line numberDiff line numberDiff line change
@@ -391,9 +391,17 @@ fn compute_discriminant_value<'ll, 'tcx>(
391391

392392
DiscrResult::Range(min, max)
393393
} else {
394-
let value = (variant_index.as_u32() as u128)
395-
.wrapping_sub(niche_variants.start().as_u32() as u128)
396-
.wrapping_add(niche_start);
394+
let discr_len = niche_variants.end().index() - niche_variants.start().index() + 1;
395+
let adj_idx = variant_index.index() - niche_variants.start().index();
396+
397+
let discr = if niche_variants.contains(&untagged_variant) {
398+
let adj_untagged_idx =
399+
untagged_variant.index() - niche_variants.start().index();
400+
(adj_idx + discr_len - adj_untagged_idx) % discr_len - 1
401+
} else {
402+
adj_idx
403+
};
404+
let value = (discr as u128).wrapping_add(niche_start);
397405
let value = tag.size(cx).truncate(value);
398406
DiscrResult::Value(value)
399407
}

0 commit comments

Comments
 (0)