Skip to content

Commit 418c278

Browse files
author
Michael Benfield
committed
Use a Flag to enable more niche optimizations.
Also: Try to fit other variants by moving fields around the niche. Keep multiple niches, not just the largest one. Look for multiple largest variants. Introduce repr(flag). Fixes #101567
1 parent 0ca3565 commit 418c278

File tree

27 files changed

+1301
-631
lines changed

27 files changed

+1301
-631
lines changed

‎Cargo.lock

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4249,6 +4249,7 @@ dependencies = [
42494249
"rustc_target",
42504250
"rustc_trait_selection",
42514251
"rustc_type_ir",
4252+
"smallvec",
42524253
"tracing",
42534254
]
42544255

‎compiler/rustc_ast/src/ast.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3065,6 +3065,8 @@ mod size_asserts {
30653065
static_assert_size!(PathSegment, 24);
30663066
static_assert_size!(Stmt, 32);
30673067
static_assert_size!(StmtKind, 16);
3068-
static_assert_size!(Ty, 96);
3069-
static_assert_size!(TyKind, 72);
3068+
#[cfg(not(bootstrap))]
3069+
static_assert_size!(Ty, 88);
3070+
#[cfg(not(bootstrap))]
3071+
static_assert_size!(TyKind, 64);
30703072
}

‎compiler/rustc_attr/src/builtin.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -944,6 +944,7 @@ pub enum ReprAttr {
944944
ReprSimd,
945945
ReprTransparent,
946946
ReprAlign(u32),
947+
ReprFlag,
947948
}
948949

949950
#[derive(Eq, PartialEq, Debug, Copy, Clone)]
@@ -998,6 +999,7 @@ pub fn parse_repr_attr(sess: &Session, attr: &Attribute) -> Vec<ReprAttr> {
998999
recognised = true;
9991000
None
10001001
}
1002+
sym::flag => Some(ReprFlag),
10011003
name => int_type_of_word(name).map(ReprInt),
10021004
};
10031005

‎compiler/rustc_codegen_cranelift/src/abi/comments.rs

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -82,14 +82,8 @@ pub(super) fn add_local_place_comments<'tcx>(
8282
return;
8383
}
8484
let TyAndLayout { ty, layout } = place.layout();
85-
let rustc_target::abi::LayoutS {
86-
size,
87-
align,
88-
abi: _,
89-
variants: _,
90-
fields: _,
91-
largest_niche: _,
92-
} = layout.0.0;
85+
let rustc_target::abi::LayoutS { size, align, abi: _, variants: _, fields: _, niches: _ } =
86+
layout.0.0;
9387

9488
let (kind, extra) = match *place.inner() {
9589
CPlaceInner::Var(place_local, var) => {

‎compiler/rustc_codegen_cranelift/src/discriminant.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ pub(crate) fn codegen_set_discriminant<'tcx>(
4242
Variants::Multiple {
4343
tag: _,
4444
tag_field,
45-
tag_encoding: TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start },
45+
tag_encoding:
46+
TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, .. },
4647
variants: _,
4748
} => {
4849
if variant_index != untagged_variant {
@@ -113,7 +114,7 @@ pub(crate) fn codegen_get_discriminant<'tcx>(
113114
let res = CValue::by_val(val, dest_layout);
114115
dest.write_cvalue(fx, res);
115116
}
116-
TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start } => {
117+
TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, .. } => {
117118
// Rebase from niche values to discriminants, and check
118119
// whether the result is in range for the niche variants.
119120

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

Lines changed: 26 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -430,32 +430,34 @@ fn compute_discriminant_value<'ll, 'tcx>(
430430
enum_type_and_layout.ty.discriminant_for_variant(cx.tcx, variant_index).unwrap().val,
431431
),
432432
&Variants::Multiple {
433-
tag_encoding: TagEncoding::Niche { ref niche_variants, niche_start, untagged_variant },
434-
tag,
433+
//tag_encoding: TagEncoding::Niche { ref niche_variants, niche_start, untagged_variant, .. },
434+
//tag,
435435
..
436436
} => {
437-
if variant_index == untagged_variant {
438-
let valid_range = enum_type_and_layout
439-
.for_variant(cx, variant_index)
440-
.largest_niche
441-
.as_ref()
442-
.unwrap()
443-
.valid_range;
444-
445-
let min = valid_range.start.min(valid_range.end);
446-
let min = tag.size(cx).truncate(min);
447-
448-
let max = valid_range.start.max(valid_range.end);
449-
let max = tag.size(cx).truncate(max);
450-
451-
DiscrResult::Range(min, max)
452-
} else {
453-
let value = (variant_index.as_u32() as u128)
454-
.wrapping_sub(niche_variants.start().as_u32() as u128)
455-
.wrapping_add(niche_start);
456-
let value = tag.size(cx).truncate(value);
457-
DiscrResult::Value(value)
458-
}
437+
// YYY
438+
DiscrResult::Range(0, 1)
439+
//if variant_index == untagged_variant {
440+
// let valid_range = enum_type_and_layout
441+
// .for_variant(cx, variant_index)
442+
// .largest_niche
443+
// .as_ref()
444+
// .unwrap()
445+
// .valid_range;
446+
447+
// let min = valid_range.start.min(valid_range.end);
448+
// let min = tag.size(cx).truncate(min);
449+
450+
// let max = valid_range.start.max(valid_range.end);
451+
// let max = tag.size(cx).truncate(max);
452+
453+
// DiscrResult::Range(min, max)
454+
//} else {
455+
// let value = (variant_index.as_u32() as u128)
456+
// .wrapping_sub(niche_variants.start().as_u32() as u128)
457+
// .wrapping_add(niche_start);
458+
// let value = tag.size(cx).truncate(value);
459+
// DiscrResult::Value(value)
460+
//}
459461
}
460462
}
461463
}

‎compiler/rustc_codegen_ssa/src/mir/place.rs

Lines changed: 70 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use super::operand::OperandValue;
22
use super::{FunctionCx, LocalRef};
33

4-
use crate::common::IntPredicate;
4+
use crate::common::{IntPredicate, TypeKind};
55
use crate::glue;
66
use crate::traits::*;
77

@@ -227,13 +227,13 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
227227
}
228228
};
229229

230-
// Read the tag/niche-encoded discriminant from memory.
231-
let tag = self.project_field(bx, tag_field);
232-
let tag = bx.load_operand(tag);
230+
let tag_place = self.project_field(bx, tag_field);
233231

234232
// Decode the discriminant (specifically if it's niche-encoded).
235233
match *tag_encoding {
236234
TagEncoding::Direct => {
235+
// Read the tag from memory.
236+
let tag = bx.load_operand(tag_place);
237237
let signed = match tag_scalar.primitive() {
238238
// We use `i1` for bytes that are always `0` or `1`,
239239
// e.g., `#[repr(i8)] enum E { A, B }`, but we can't
@@ -244,11 +244,30 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
244244
};
245245
bx.intcast(tag.immediate(), cast_to, signed)
246246
}
247-
TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start } => {
248-
// Rebase from niche values to discriminants, and check
249-
// whether the result is in range for the niche variants.
250-
let niche_llty = bx.cx().immediate_backend_type(tag.layout);
251-
let tag = tag.immediate();
247+
TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, ref flag, } => {
248+
let read = |bx: &mut Bx, place: Self| -> (V, <Bx as BackendTypes>::Type) {
249+
let ty = bx.cx().immediate_backend_type(place.layout);
250+
let op = bx.load_operand(place);
251+
let val = op.immediate();
252+
if bx.cx().type_kind(ty) == TypeKind::Pointer {
253+
let new_ty = bx.cx().type_isize();
254+
let new_val = bx.ptrtoint(val, new_ty);
255+
(new_val, new_ty)
256+
} else {
257+
(val, ty)
258+
}
259+
};
260+
261+
let (tag, niche_llty) = read(bx, tag_place);
262+
263+
let (untagged_in_niche, flag_eq_magic_value_opt) = if let Some(flag) = flag {
264+
let flag_place = self.project_field(bx, flag.field);
265+
let (flag_imm, flag_llty) = read(bx, flag_place);
266+
let magic_value = bx.cx().const_uint_big(flag_llty, flag.magic_value);
267+
(flag.untagged_in_niche, Some(bx.icmp(IntPredicate::IntEQ, flag_imm, magic_value)))
268+
} else {
269+
(true, None)
270+
};
252271

253272
// We first compute the "relative discriminant" (wrt `niche_variants`),
254273
// that is, if `n = niche_variants.end() - niche_variants.start()`,
@@ -259,23 +278,8 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
259278
// and check that it is in the range `niche_variants`, because
260279
// that might not fit in the same type, on top of needing an extra
261280
// comparison (see also the comment on `let niche_discr`).
262-
let relative_discr = if niche_start == 0 {
263-
// Avoid subtracting `0`, which wouldn't work for pointers.
264-
// FIXME(eddyb) check the actual primitive type here.
265-
tag
266-
} else {
267-
bx.sub(tag, bx.cx().const_uint_big(niche_llty, niche_start))
268-
};
281+
let relative_discr = bx.sub(tag, bx.cx().const_uint_big(niche_llty, niche_start));
269282
let relative_max = niche_variants.end().as_u32() - niche_variants.start().as_u32();
270-
let is_niche = if relative_max == 0 {
271-
// Avoid calling `const_uint`, which wouldn't work for pointers.
272-
// Also use canonical == 0 instead of non-canonical u<= 0.
273-
// FIXME(eddyb) check the actual primitive type here.
274-
bx.icmp(IntPredicate::IntEQ, relative_discr, bx.cx().const_null(niche_llty))
275-
} else {
276-
let relative_max = bx.cx().const_uint(niche_llty, relative_max as u64);
277-
bx.icmp(IntPredicate::IntULE, relative_discr, relative_max)
278-
};
279283

280284
// NOTE(eddyb) this addition needs to be performed on the final
281285
// type, in case the niche itself can't represent all variant
@@ -285,7 +289,7 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
285289
// In other words, `niche_variants.end - niche_variants.start`
286290
// is representable in the niche, but `niche_variants.end`
287291
// might not be, in extreme cases.
288-
let niche_discr = {
292+
let potential_niche_discr = {
289293
let relative_discr = if relative_max == 0 {
290294
// HACK(eddyb) since we have only one niche, we know which
291295
// one it is, and we can avoid having a dynamic value here.
@@ -299,11 +303,29 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
299303
)
300304
};
301305

302-
bx.select(
303-
is_niche,
304-
niche_discr,
305-
bx.cx().const_uint(cast_to, untagged_variant.as_u32() as u64),
306-
)
306+
let untagged_discr = bx.cx().const_uint(cast_to, untagged_variant.as_u32() as u64);
307+
308+
let niche_discr = if untagged_in_niche {
309+
let relative_max_const = bx.cx().const_uint(niche_llty, relative_max as u64);
310+
let is_niche = bx.icmp(IntPredicate::IntULE, relative_discr, relative_max_const);
311+
bx.select(
312+
is_niche,
313+
potential_niche_discr,
314+
untagged_discr,
315+
)
316+
} else {
317+
potential_niche_discr
318+
};
319+
320+
if let Some(flag_eq_magic_value) = flag_eq_magic_value_opt {
321+
bx.select(
322+
flag_eq_magic_value,
323+
niche_discr,
324+
untagged_discr,
325+
)
326+
} else {
327+
niche_discr
328+
}
307329
}
308330
}
309331
}
@@ -337,23 +359,30 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
337359
}
338360
Variants::Multiple {
339361
tag_encoding:
340-
TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start },
362+
TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, ref flag, },
341363
tag_field,
342364
..
343365
} => {
366+
let store = |bx: &mut Bx, value: u128, place: Self| {
367+
let ty = bx.cx().immediate_backend_type(place.layout);
368+
let val = if bx.cx().type_kind(ty) == TypeKind::Pointer {
369+
let ty_isize = bx.cx().type_isize();
370+
let llvalue = bx.cx().const_uint_big(ty_isize, value);
371+
bx.inttoptr(llvalue, ty)
372+
} else {
373+
bx.cx().const_uint_big(ty, value)
374+
};
375+
OperandValue::Immediate(val).store(bx, place);
376+
};
344377
if variant_index != untagged_variant {
378+
if let Some(flag) = flag {
379+
let place = self.project_field(bx, flag.field);
380+
store(bx, flag.magic_value, place);
381+
}
345382
let niche = self.project_field(bx, tag_field);
346-
let niche_llty = bx.cx().immediate_backend_type(niche.layout);
347383
let niche_value = variant_index.as_u32() - niche_variants.start().as_u32();
348384
let niche_value = (niche_value as u128).wrapping_add(niche_start);
349-
// FIXME(eddyb): check the actual primitive type here.
350-
let niche_llval = if niche_value == 0 {
351-
// HACK(eddyb): using `c_null` as it works on all types.
352-
bx.cx().const_null(niche_llty)
353-
} else {
354-
bx.cx().const_uint_big(niche_llty, niche_value)
355-
};
356-
OperandValue::Immediate(niche_llval).store(bx, niche);
385+
store(bx, niche_value, niche);
357386
}
358387
}
359388
}

‎compiler/rustc_const_eval/src/interpret/operand.rs

Lines changed: 38 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -722,27 +722,55 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
722722
// Return the cast value, and the index.
723723
(discr_val, index.0)
724724
}
725-
TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start } => {
725+
TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, ref flag, } => {
726+
let is_magic_val = if let Some(flag) = flag {
727+
let flag_val = self.read_immediate(&self.operand_field(op, flag.field)?)?;
728+
let flag_val = flag_val.to_scalar();
729+
match flag_val.try_to_int() {
730+
Err(dbg_val) => {
731+
// So this is a pointer then, and casting to an int
732+
// failed. Can only happen during CTFE. If the magic
733+
// value is 0 and the scalar is not null, we know
734+
// the pointer cannot be the magic value. Anything
735+
// else we conservatively reject.
736+
let ptr_definitely_not_magic_value =
737+
flag.magic_value == 0 && !self.scalar_may_be_null(flag_val)?;
738+
if !ptr_definitely_not_magic_value {
739+
throw_ub!(InvalidTag(dbg_val))
740+
}
741+
false
742+
}
743+
Ok(flag_bits) => {
744+
let flag_layout =
745+
self.layout_of(flag.scalar.primitive().to_int_ty(*self.tcx))?;
746+
let flag_bits = flag_bits.assert_bits(flag_layout.size);
747+
flag_bits == flag.magic_value
748+
}
749+
}
750+
} else {
751+
true
752+
};
726753
let tag_val = tag_val.to_scalar();
727754
// Compute the variant this niche value/"tag" corresponds to. With niche layout,
728755
// discriminant (encoded in niche/tag) and variant index are the same.
729756
let variants_start = niche_variants.start().as_u32();
730757
let variants_end = niche_variants.end().as_u32();
731-
let variant = match tag_val.try_to_int() {
732-
Err(dbg_val) => {
758+
let variant = match (is_magic_val, tag_val.try_to_int()) {
759+
(false, _) => untagged_variant,
760+
(true, Err(dbg_val)) => {
733761
// So this is a pointer then, and casting to an int failed.
734762
// Can only happen during CTFE.
735763
// The niche must be just 0, and the ptr not null, then we know this is
736764
// okay. Everything else, we conservatively reject.
737-
let ptr_valid = niche_start == 0
765+
let ptr_definitely_not_in_niche_variants = niche_start == 0
738766
&& variants_start == variants_end
739767
&& !self.scalar_may_be_null(tag_val)?;
740-
if !ptr_valid {
768+
if !ptr_definitely_not_in_niche_variants {
741769
throw_ub!(InvalidTag(dbg_val))
742770
}
743771
untagged_variant
744772
}
745-
Ok(tag_bits) => {
773+
(true, Ok(tag_bits)) => {
746774
let tag_bits = tag_bits.assert_bits(tag_layout.size);
747775
// We need to use machine arithmetic to get the relative variant idx:
748776
// variant_index_relative = tag_val - niche_start_val
@@ -791,6 +819,8 @@ mod size_asserts {
791819
// These are in alphabetical order, which is easy to maintain.
792820
static_assert_size!(Immediate, 48);
793821
static_assert_size!(ImmTy<'_>, 64);
794-
static_assert_size!(Operand, 56);
795-
static_assert_size!(OpTy<'_>, 80);
822+
#[cfg(not(bootstrap))]
823+
static_assert_size!(Operand, 48);
824+
#[cfg(not(bootstrap))]
825+
static_assert_size!(OpTy<'_>, 72);
796826
}

‎compiler/rustc_const_eval/src/interpret/place.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -823,15 +823,22 @@ where
823823
}
824824
abi::Variants::Multiple {
825825
tag_encoding:
826-
TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start },
826+
TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start, ref flag, },
827827
tag: tag_layout,
828828
tag_field,
829829
..
830830
} => {
831-
// No need to validate that the discriminant here because the
831+
// No need to validate the discriminant here because the
832832
// `TyAndLayout::for_variant()` call earlier already checks the variant is valid.
833833

834834
if variant_index != untagged_variant {
835+
if let Some(flag) = flag {
836+
let flag_layout = self.layout_of(flag.scalar.primitive().to_int_ty(*self.tcx))?;
837+
let val = ImmTy::from_uint(flag.magic_value, flag_layout);
838+
let flag_dest = self.place_field(dest, flag.field)?;
839+
self.write_immediate(*val, &flag_dest)?;
840+
}
841+
835842
let variants_start = niche_variants.start().as_u32();
836843
let variant_index_relative = variant_index
837844
.as_u32()

0 commit comments

Comments
 (0)