Skip to content

Commit

Permalink
Auto merge of rust-lang#94075 - mikebenfield:wip-enum, r=oli-obk
Browse files Browse the repository at this point in the history
Use niche-filling optimization even when multiple variants have data.

Fixes rust-lang#46213
  • Loading branch information
bors committed Sep 7, 2022
2 parents c2804e6 + d7a750b commit 512bd84
Show file tree
Hide file tree
Showing 22 changed files with 377 additions and 196 deletions.
3 changes: 2 additions & 1 deletion compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3075,7 +3075,8 @@ mod size_asserts {
static_assert_size!(Block, 48);
static_assert_size!(Expr, 104);
static_assert_size!(ExprKind, 72);
static_assert_size!(Fn, 192);
#[cfg(not(bootstrap))]
static_assert_size!(Fn, 184);
static_assert_size!(ForeignItem, 96);
static_assert_size!(ForeignItemKind, 24);
static_assert_size!(GenericArg, 24);
Expand Down
11 changes: 6 additions & 5 deletions compiler/rustc_codegen_cranelift/src/discriminant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,10 @@ pub(crate) fn codegen_set_discriminant<'tcx>(
Variants::Multiple {
tag: _,
tag_field,
tag_encoding: TagEncoding::Niche { dataful_variant, ref niche_variants, niche_start },
tag_encoding: TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start },
variants: _,
} => {
if variant_index != dataful_variant {
if variant_index != untagged_variant {
let niche = place.place_field(fx, mir::Field::new(tag_field));
let niche_value = variant_index.as_u32() - niche_variants.start().as_u32();
let niche_value = ty::ScalarInt::try_from_uint(
Expand Down Expand Up @@ -113,7 +113,7 @@ pub(crate) fn codegen_get_discriminant<'tcx>(
let res = CValue::by_val(val, dest_layout);
dest.write_cvalue(fx, res);
}
TagEncoding::Niche { dataful_variant, ref niche_variants, niche_start } => {
TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start } => {
// Rebase from niche values to discriminants, and check
// whether the result is in range for the niche variants.

Expand Down Expand Up @@ -169,8 +169,9 @@ pub(crate) fn codegen_get_discriminant<'tcx>(
fx.bcx.ins().iadd_imm(relative_discr, i64::from(niche_variants.start().as_u32()))
};

let dataful_variant = fx.bcx.ins().iconst(cast_to, i64::from(dataful_variant.as_u32()));
let discr = fx.bcx.ins().select(is_niche, niche_discr, dataful_variant);
let untagged_variant =
fx.bcx.ins().iconst(cast_to, i64::from(untagged_variant.as_u32()));
let discr = fx.bcx.ins().select(is_niche, niche_discr, untagged_variant);
let res = CValue::by_val(discr, dest_layout);
dest.write_cvalue(fx, res);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ const SINGLE_VARIANT_VIRTUAL_DISR: u64 = 0;
/// compiler versions.
///
/// Niche-tag enums have one special variant, usually called the
/// "dataful variant". This variant has a field that
/// "untagged variant". This variant has a field that
/// doubles as the tag of the enum. The variant is active when the value of
/// that field is within a pre-defined range. Therefore the variant struct
/// has a `DISCR_BEGIN` and `DISCR_END` field instead of `DISCR_EXACT` in
Expand Down Expand Up @@ -249,7 +249,7 @@ pub(super) fn build_enum_type_di_node<'ll, 'tcx>(
None,
),
Variants::Multiple {
tag_encoding: TagEncoding::Niche { dataful_variant, .. },
tag_encoding: TagEncoding::Niche { untagged_variant, .. },
ref variants,
tag_field,
..
Expand All @@ -260,7 +260,7 @@ pub(super) fn build_enum_type_di_node<'ll, 'tcx>(
enum_type_di_node,
variants.indices(),
tag_field,
Some(dataful_variant),
Some(untagged_variant),
),
}
},
Expand Down Expand Up @@ -391,7 +391,7 @@ fn build_union_fields_for_enum<'ll, 'tcx>(
enum_type_di_node: &'ll DIType,
variant_indices: impl Iterator<Item = VariantIdx> + Clone,
tag_field: usize,
dataful_variant_index: Option<VariantIdx>,
untagged_variant_index: Option<VariantIdx>,
) -> SmallVec<&'ll DIType> {
let tag_base_type = super::tag_base_type(cx, enum_type_and_layout);

Expand Down Expand Up @@ -436,7 +436,7 @@ fn build_union_fields_for_enum<'ll, 'tcx>(
variant_names_type_di_node,
tag_base_type,
tag_field,
dataful_variant_index,
untagged_variant_index,
)
}

Expand Down Expand Up @@ -472,7 +472,7 @@ fn build_variant_struct_wrapper_type_di_node<'ll, 'tcx>(
enum_or_generator_type_and_layout: TyAndLayout<'tcx>,
enum_or_generator_type_di_node: &'ll DIType,
variant_index: VariantIdx,
dataful_variant_index: Option<VariantIdx>,
untagged_variant_index: Option<VariantIdx>,
variant_struct_type_di_node: &'ll DIType,
variant_names_type_di_node: &'ll DIType,
tag_base_type_di_node: &'ll DIType,
Expand Down Expand Up @@ -517,7 +517,7 @@ fn build_variant_struct_wrapper_type_di_node<'ll, 'tcx>(
}
}
DiscrResult::Range(min, max) => {
assert_eq!(Some(variant_index), dataful_variant_index);
assert_eq!(Some(variant_index), untagged_variant_index);
if is_128_bits {
DiscrKind::Range128(min, max)
} else {
Expand Down Expand Up @@ -757,7 +757,7 @@ fn build_union_fields_for_direct_tag_enum_or_generator<'ll, 'tcx>(
discr_type_di_node: &'ll DIType,
tag_base_type: Ty<'tcx>,
tag_field: usize,
dataful_variant_index: Option<VariantIdx>,
untagged_variant_index: Option<VariantIdx>,
) -> SmallVec<&'ll DIType> {
let tag_base_type_di_node = type_di_node(cx, tag_base_type);
let mut unions_fields = SmallVec::with_capacity(variant_field_infos.len() + 1);
Expand All @@ -776,7 +776,7 @@ fn build_union_fields_for_direct_tag_enum_or_generator<'ll, 'tcx>(
enum_type_and_layout,
enum_type_di_node,
variant_member_info.variant_index,
dataful_variant_index,
untagged_variant_index,
variant_member_info.variant_struct_type_di_node,
discr_type_di_node,
tag_base_type_di_node,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,7 @@ impl DiscrResult {
/// Returns the discriminant value corresponding to the variant index.
///
/// Will return `None` if there is less than two variants (because then the enum won't have)
/// a tag, and if this is the dataful variant of a niche-layout enum (because then there is no
/// a tag, and if this is the untagged variant of a niche-layout enum (because then there is no
/// single discriminant value).
fn compute_discriminant_value<'ll, 'tcx>(
cx: &CodegenCx<'ll, 'tcx>,
Expand All @@ -430,11 +430,11 @@ fn compute_discriminant_value<'ll, 'tcx>(
enum_type_and_layout.ty.discriminant_for_variant(cx.tcx, variant_index).unwrap().val,
),
&Variants::Multiple {
tag_encoding: TagEncoding::Niche { ref niche_variants, niche_start, dataful_variant },
tag_encoding: TagEncoding::Niche { ref niche_variants, niche_start, untagged_variant },
tag,
..
} => {
if variant_index == dataful_variant {
if variant_index == untagged_variant {
let valid_range = enum_type_and_layout
.for_variant(cx, variant_index)
.largest_niche
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -378,7 +378,7 @@ fn build_discr_member_di_node<'ll, 'tcx>(
///
/// The DW_AT_discr_value is optional, and is omitted if
/// - This is the only variant of a univariant enum (i.e. their is no discriminant)
/// - This is the "dataful" variant of a niche-layout enum
/// - This is the "untagged" variant of a niche-layout enum
/// (where only the other variants are identified by a single value)
///
/// There is only ever a single member, the type of which is a struct that describes the
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_codegen_ssa/src/mir/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
};
bx.intcast(tag.immediate(), cast_to, signed)
}
TagEncoding::Niche { dataful_variant, ref niche_variants, niche_start } => {
TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start } => {
// Rebase from niche values to discriminants, and check
// whether the result is in range for the niche variants.
let niche_llty = bx.cx().immediate_backend_type(tag.layout);
Expand Down Expand Up @@ -302,7 +302,7 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
bx.select(
is_niche,
niche_discr,
bx.cx().const_uint(cast_to, dataful_variant.as_u32() as u64),
bx.cx().const_uint(cast_to, untagged_variant.as_u32() as u64),
)
}
}
Expand Down Expand Up @@ -337,11 +337,11 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
}
Variants::Multiple {
tag_encoding:
TagEncoding::Niche { dataful_variant, ref niche_variants, niche_start },
TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start },
tag_field,
..
} => {
if variant_index != dataful_variant {
if variant_index != untagged_variant {
let niche = self.project_field(bx, tag_field);
let niche_llty = bx.cx().immediate_backend_type(niche.layout);
let niche_value = variant_index.as_u32() - niche_variants.start().as_u32();
Expand Down
16 changes: 8 additions & 8 deletions compiler/rustc_const_eval/src/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -718,7 +718,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// Return the cast value, and the index.
(discr_val, index.0)
}
TagEncoding::Niche { dataful_variant, ref niche_variants, niche_start } => {
TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start } => {
let tag_val = tag_val.to_scalar();
// Compute the variant this niche value/"tag" corresponds to. With niche layout,
// discriminant (encoded in niche/tag) and variant index are the same.
Expand All @@ -736,7 +736,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
if !ptr_valid {
throw_ub!(InvalidTag(dbg_val))
}
dataful_variant
untagged_variant
}
Ok(tag_bits) => {
let tag_bits = tag_bits.assert_bits(tag_layout.size);
Expand Down Expand Up @@ -766,7 +766,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
assert!(usize::try_from(variant_index).unwrap() < variants_len);
VariantIdx::from_u32(variant_index)
} else {
dataful_variant
untagged_variant
}
}
};
Expand All @@ -780,13 +780,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
}

// Some nodes are used a lot. Make sure they don't unintentionally get bigger.
#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
#[cfg(all(target_arch = "x86_64", target_pointer_width = "64", not(bootstrap)))]
mod size_asserts {
use super::*;
use rustc_data_structures::static_assert_size;
// These are in alphabetical order, which is easy to maintain.
static_assert_size!(Immediate, 56);
static_assert_size!(ImmTy<'_>, 72);
static_assert_size!(Operand, 64);
static_assert_size!(OpTy<'_>, 88);
static_assert_size!(Immediate, 48);
static_assert_size!(ImmTy<'_>, 64);
static_assert_size!(Operand, 56);
static_assert_size!(OpTy<'_>, 80);
}
10 changes: 6 additions & 4 deletions compiler/rustc_const_eval/src/interpret/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -817,15 +817,15 @@ where
}
abi::Variants::Multiple {
tag_encoding:
TagEncoding::Niche { dataful_variant, ref niche_variants, niche_start },
TagEncoding::Niche { untagged_variant, ref niche_variants, niche_start },
tag: tag_layout,
tag_field,
..
} => {
// No need to validate that the discriminant here because the
// `TyAndLayout::for_variant()` call earlier already checks the variant is valid.

if variant_index != dataful_variant {
if variant_index != untagged_variant {
let variants_start = niche_variants.start().as_u32();
let variant_index_relative = variant_index
.as_u32()
Expand Down Expand Up @@ -890,6 +890,8 @@ mod size_asserts {
static_assert_size!(MemPlaceMeta, 24);
static_assert_size!(MemPlace, 40);
static_assert_size!(MPlaceTy<'_>, 64);
static_assert_size!(Place, 48);
static_assert_size!(PlaceTy<'_>, 72);
#[cfg(not(bootstrap))]
static_assert_size!(Place, 40);
#[cfg(not(bootstrap))]
static_assert_size!(PlaceTy<'_>, 64);
}
4 changes: 4 additions & 0 deletions compiler/rustc_data_structures/src/obligation_forest/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,10 @@ pub trait ObligationProcessor {
}

/// The result type used by `process_obligation`.
// `repr(C)` to inhibit the niche filling optimization. Otherwise, the `match` appearing
// in `process_obligations` is significantly slower, which can substantially affect
// benchmarks like `rustc-perf`'s inflate and keccak.
#[repr(C)]
#[derive(Debug)]
pub enum ProcessResult<O, E> {
Unchanged,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ pub type PResult<'a, T> = Result<T, DiagnosticBuilder<'a, ErrorGuaranteed>>;
// (See also the comment on `DiagnosticBuilder`'s `diagnostic` field.)
#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
rustc_data_structures::static_assert_size!(PResult<'_, ()>, 16);
#[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
rustc_data_structures::static_assert_size!(PResult<'_, bool>, 24);
#[cfg(all(target_arch = "x86_64", target_pointer_width = "64", not(bootstrap)))]
rustc_data_structures::static_assert_size!(PResult<'_, bool>, 16);

#[derive(Debug, PartialEq, Eq, Clone, Copy, Hash, Encodable, Decodable)]
pub enum SuggestionStyle {
Expand Down
15 changes: 10 additions & 5 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3473,12 +3473,15 @@ mod size_asserts {
static_assert_size!(FnDecl<'_>, 40);
static_assert_size!(ForeignItem<'_>, 72);
static_assert_size!(ForeignItemKind<'_>, 40);
static_assert_size!(GenericArg<'_>, 40);
#[cfg(not(bootstrap))]
static_assert_size!(GenericArg<'_>, 32);
static_assert_size!(GenericBound<'_>, 48);
static_assert_size!(Generics<'_>, 56);
static_assert_size!(Impl<'_>, 80);
static_assert_size!(ImplItem<'_>, 88);
static_assert_size!(ImplItemKind<'_>, 40);
#[cfg(not(bootstrap))]
static_assert_size!(ImplItem<'_>, 80);
#[cfg(not(bootstrap))]
static_assert_size!(ImplItemKind<'_>, 32);
static_assert_size!(Item<'_>, 80);
static_assert_size!(ItemKind<'_>, 48);
static_assert_size!(Local<'_>, 64);
Expand All @@ -3490,8 +3493,10 @@ mod size_asserts {
static_assert_size!(QPath<'_>, 24);
static_assert_size!(Stmt<'_>, 32);
static_assert_size!(StmtKind<'_>, 16);
static_assert_size!(TraitItem<'_>, 96);
static_assert_size!(TraitItemKind<'_>, 56);
#[cfg(not(bootstrap))]
static_assert_size!(TraitItem<'static>, 88);
#[cfg(not(bootstrap))]
static_assert_size!(TraitItemKind<'_>, 48);
static_assert_size!(Ty<'_>, 72);
static_assert_size!(TyKind<'_>, 56);
}
3 changes: 2 additions & 1 deletion compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1231,7 +1231,8 @@ pub enum BinOp {
mod size_asserts {
use super::*;
// These are in alphabetical order, which is easy to maintain.
static_assert_size!(AggregateKind<'_>, 48);
#[cfg(not(bootstrap))]
static_assert_size!(AggregateKind<'_>, 40);
static_assert_size!(Operand<'_>, 24);
static_assert_size!(Place<'_>, 16);
static_assert_size!(PlaceElem<'_>, 24);
Expand Down
12 changes: 8 additions & 4 deletions compiler/rustc_middle/src/thir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -825,8 +825,12 @@ mod size_asserts {
static_assert_size!(Block, 56);
static_assert_size!(Expr<'_>, 64);
static_assert_size!(ExprKind<'_>, 40);
static_assert_size!(Pat<'_>, 72);
static_assert_size!(PatKind<'_>, 56);
static_assert_size!(Stmt<'_>, 56);
static_assert_size!(StmtKind<'_>, 48);
#[cfg(not(bootstrap))]
static_assert_size!(Pat<'_>, 64);
#[cfg(not(bootstrap))]
static_assert_size!(PatKind<'_>, 48);
#[cfg(not(bootstrap))]
static_assert_size!(Stmt<'_>, 48);
#[cfg(not(bootstrap))]
static_assert_size!(StmtKind<'_>, 40);
}
Loading

0 comments on commit 512bd84

Please sign in to comment.