Skip to content

rustc_target: use Integer instead of Primitive for discriminants and niches. #63450

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 55 additions & 24 deletions src/librustc/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
let largest_niche = Niche::from_scalar(dl, b_offset, b.clone())
.into_iter()
.chain(Niche::from_scalar(dl, Size::ZERO, a.clone()))
.max_by_key(|niche| niche.available(dl));
.max_by_key(|niche| niche.available());

LayoutDetails {
variants: Variants::Single { index: VariantIdx::new(0) },
Expand Down Expand Up @@ -367,7 +367,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
offsets[i as usize] = offset;

if let Some(mut niche) = field.largest_niche.clone() {
let available = niche.available(dl);
let available = niche.available();
if available > largest_niche_available {
largest_niche_available = available;
niche.offset += offset;
Expand Down Expand Up @@ -887,7 +887,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
Some(largest_niche) => {
// Replace the existing niche even if they're equal,
// because this one is at a lower offset.
if largest_niche.available(dl) <= niche.available(dl) {
if largest_niche.available() <= niche.available() {
st.largest_niche = Some(niche);
}
}
Expand Down Expand Up @@ -947,12 +947,12 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
// FIXME(#62691) use the largest niche across all fields,
// not just the first one.
for (field_index, &field) in variants[i].iter().enumerate() {
let niche = match &field.largest_niche {
let mut niche = match field.largest_niche.clone() {
Some(niche) => niche,
_ => continue,
None => continue,
};
let (niche_start, niche_scalar) = match niche.reserve(self, count) {
Some(pair) => pair,
let niche_start = match niche.reserve(count) {
Some(start) => start,
None => continue,
};

Expand All @@ -970,23 +970,30 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
let offset = st[i].fields.offset(field_index) + niche.offset;
let size = st[i].size;

let use_valid_range_from_niche = |scalar: &Scalar| {
assert_eq!(scalar.value.size(dl), niche.scalar.value.size());
Scalar {
value: scalar.value.clone(),
valid_range: niche.scalar.valid_range.clone(),
}
};
let mut abi = match st[i].abi {
Abi::Scalar(_) => Abi::Scalar(niche_scalar.clone()),
Abi::ScalarPair(ref first, ref second) => {
Abi::Scalar(ref x) => Abi::Scalar(use_valid_range_from_niche(x)),
Abi::ScalarPair(ref a, ref b) => {
// We need to use scalar_unit to reset the
// valid range to the maximal one for that
// primitive, because only the niche is
// guaranteed to be initialised, not the
// other primitive.
if offset.bytes() == 0 {
Abi::ScalarPair(
niche_scalar.clone(),
scalar_unit(second.value),
use_valid_range_from_niche(a),
scalar_unit(b.value),
)
} else {
Abi::ScalarPair(
scalar_unit(first.value),
niche_scalar.clone(),
scalar_unit(a.value),
use_valid_range_from_niche(b),
)
}
}
Expand All @@ -999,11 +1006,11 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {


let largest_niche =
Niche::from_scalar(dl, offset, niche_scalar.clone());
Niche::from_int(niche.offset, niche.scalar.clone());

return Ok(tcx.intern_layout(LayoutDetails {
variants: Variants::Multiple {
discr: niche_scalar,
discr: niche.scalar,
discr_kind: DiscriminantKind::Niche {
dataful_variant: i,
niche_variants,
Expand Down Expand Up @@ -1156,12 +1163,15 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {

let tag_mask = !0u128 >> (128 - ity.size().bits());
let tag = Scalar {
value: Int(ity, signed),
value: ity,
valid_range: (min as u128 & tag_mask)..=(max as u128 & tag_mask),
};
let mut abi = Abi::Aggregate { sized: true };
if tag.value.size(dl) == size {
abi = Abi::Scalar(tag.clone());
if tag.value.size() == size {
abi = Abi::Scalar(Scalar {
value: Int(tag.value, signed),
valid_range: tag.valid_range.clone(),
});
} else {
// Try to use a ScalarPair for all tagged enums.
let mut common_prim = None;
Expand Down Expand Up @@ -1203,7 +1213,10 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
}
}
if let Some((prim, offset)) = common_prim {
let pair = self.scalar_pair(tag.clone(), scalar_unit(prim));
let pair = self.scalar_pair(Scalar {
value: Int(tag.value, signed),
valid_range: tag.valid_range.clone(),
}, scalar_unit(prim));
let pair_offsets = match pair.fields {
FieldPlacement::Arbitrary {
ref offsets,
Expand All @@ -1229,7 +1242,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
abi = Abi::Uninhabited;
}

let largest_niche = Niche::from_scalar(dl, Size::ZERO, tag.clone());
let largest_niche = Niche::from_int(Size::ZERO, tag.clone());

tcx.intern_layout(LayoutDetails {
variants: Variants::Multiple {
Expand Down Expand Up @@ -1423,8 +1436,11 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
let discr_index = substs.prefix_tys(def_id, tcx).count();
// FIXME(eddyb) set the correct vaidity range for the discriminant.
let discr_layout = self.layout_of(substs.discr_ty(tcx))?;
let discr = match &discr_layout.abi {
Abi::Scalar(s) => s.clone(),
let discr = match discr_layout.abi.clone() {
Abi::Scalar(Scalar {
value: Int(value, false),
valid_range,
}) => Scalar { value, valid_range },
_ => bug!(),
};
let promoted_layouts = ineligible_locals.iter()
Expand Down Expand Up @@ -1716,7 +1732,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
})
.collect();
record(adt_kind.into(), adt_packed, match discr_kind {
DiscriminantKind::Tag => Some(discr.value.size(self)),
DiscriminantKind::Tag => Some(discr.value.size()),
_ => None
}, variant_infos);
}
Expand Down Expand Up @@ -2071,7 +2087,10 @@ where

fn field(this: TyLayout<'tcx>, cx: &C, i: usize) -> C::TyLayout {
let tcx = cx.tcx();
let discr_layout = |discr: &Scalar| -> C::TyLayout {
let discr_layout = |discr: &Scalar<Integer>| -> C::TyLayout {
let Scalar { value, valid_range } = discr.clone();
let discr = Scalar { value: Int(value, false), valid_range };

let layout = LayoutDetails::scalar(cx, discr.clone());
MaybeResult::from(Ok(TyLayout {
details: tcx.intern_layout(layout),
Expand Down Expand Up @@ -2443,6 +2462,18 @@ impl<'a> HashStable<StableHashingContext<'a>> for Scalar {
}
}

// FIXME(eddyb) derive this in `rustc_target` (can't be generic here).
impl<'a> HashStable<StableHashingContext<'a>> for Scalar<Integer> {
fn hash_stable<W: StableHasherResult>(&self,
hcx: &mut StableHashingContext<'a>,
hasher: &mut StableHasher<W>) {
let Scalar { value, ref valid_range } = *self;
value.hash_stable(hcx, hasher);
valid_range.start().hash_stable(hcx, hasher);
valid_range.end().hash_stable(hcx, hasher);
}
}

impl_stable_hash_for!(struct crate::ty::layout::Niche {
offset,
scalar
Expand Down
23 changes: 8 additions & 15 deletions src/librustc_codegen_llvm/debuginfo/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,12 @@ use rustc_data_structures::fingerprint::Fingerprint;
use rustc::ty::Instance;
use rustc::ty::{self, AdtKind, ParamEnv, Ty, TyCtxt};
use rustc::ty::layout::{self, Align, Integer, IntegerExt, LayoutOf,
PrimitiveExt, Size, TyLayout, VariantIdx};
Size, TyLayout, VariantIdx};
use rustc::ty::subst::UnpackedKind;
use rustc::session::config::{self, DebugInfo};
use rustc::util::nodemap::FxHashMap;
use rustc_fs_util::path_to_c_string;
use rustc_data_structures::small_c_str::SmallCStr;
use rustc_target::abi::HasDataLayout;

use libc::{c_uint, c_longlong};
use std::collections::hash_map::Entry;
Expand Down Expand Up @@ -1537,7 +1536,7 @@ impl EnumMemberDescriptionFactory<'ll, 'tcx> {
let value = (i.as_u32() as u128)
.wrapping_sub(niche_variants.start().as_u32() as u128)
.wrapping_add(niche_start);
let value = truncate(value, discr.value.size(cx));
let value = truncate(value, discr.value.size());
// NOTE(eddyb) do *NOT* remove this assert, until
// we pass the full 128-bit value to LLVM, otherwise
// truncation will be silent and remain undetected.
Expand Down Expand Up @@ -1742,7 +1741,7 @@ fn prepare_enum_metadata(
// <unknown>
let file_metadata = unknown_file_metadata(cx);

let discriminant_type_metadata = |discr: layout::Primitive| {
let discriminant_type_metadata = |discr: Integer| {
let enumerators_metadata: Vec<_> = match enum_type.sty {
ty::Adt(def, _) => def
.discriminants(cx.tcx)
Expand Down Expand Up @@ -1782,9 +1781,9 @@ fn prepare_enum_metadata(
Some(discriminant_type_metadata) => discriminant_type_metadata,
None => {
let (discriminant_size, discriminant_align) =
(discr.size(cx), discr.align(cx));
(discr.size(), discr.align(cx));
let discriminant_base_type_metadata =
type_metadata(cx, discr.to_ty(cx.tcx), syntax_pos::DUMMY_SP);
type_metadata(cx, discr.to_ty(cx.tcx, false), syntax_pos::DUMMY_SP);

let discriminant_name = match enum_type.sty {
ty::Adt(..) => SmallCStr::new(&cx.tcx.item_name(enum_def_id).as_str()),
Expand Down Expand Up @@ -1893,16 +1892,10 @@ fn prepare_enum_metadata(
..
} => {
// Find the integer type of the correct size.
let size = discr.value.size(cx);
let size = discr.value.size();
let align = discr.value.align(cx);

let discr_type = match discr.value {
layout::Int(t, _) => t,
layout::Float(layout::FloatTy::F32) => Integer::I32,
layout::Float(layout::FloatTy::F64) => Integer::I64,
layout::Pointer => cx.data_layout().ptr_sized_integer(),
}.to_ty(cx.tcx, false);

let discr_type = discr.value.to_ty(cx.tcx, false);
let discr_metadata = basic_type_metadata(cx, discr_type);
unsafe {
Some(llvm::LLVMRustDIBuilderCreateMemberType(
Expand All @@ -1925,7 +1918,7 @@ fn prepare_enum_metadata(
discr_index,
..
} => {
let discr_type = discr.value.to_ty(cx.tcx);
let discr_type = discr.value.to_ty(cx.tcx, false);
let (size, align) = cx.size_and_align_of(discr_type);

let discr_metadata = basic_type_metadata(cx, discr_type);
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_codegen_llvm/debuginfo/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ pub struct CrateDebugContext<'a, 'tcx> {
llmod: &'a llvm::Module,
builder: &'a mut DIBuilder<'a>,
created_files: RefCell<FxHashMap<(Option<String>, Option<String>), &'a DIFile>>,
created_enum_disr_types: RefCell<FxHashMap<(DefId, layout::Primitive), &'a DIType>>,
created_enum_disr_types: RefCell<FxHashMap<(DefId, layout::Integer), &'a DIType>>,

type_map: RefCell<TypeMap<'a, 'tcx>>,
namespace_map: RefCell<DefIdMap<&'a DIScope>>,
Expand Down
47 changes: 22 additions & 25 deletions src/librustc_codegen_ssa/mir/place.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,15 +213,16 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
bx: &mut Bx,
cast_to: Ty<'tcx>
) -> V {
let cast_to = bx.cx().immediate_backend_type(bx.cx().layout_of(cast_to));
let cast_to = bx.cx().layout_of(cast_to);
let cast_to_llty = bx.cx().immediate_backend_type(cast_to);
if self.layout.abi.is_uninhabited() {
return bx.cx().const_undef(cast_to);
return bx.cx().const_undef(cast_to_llty);
}
let (discr_scalar, discr_kind, discr_index) = match self.layout.variants {
layout::Variants::Single { index } => {
let discr_val = self.layout.ty.discriminant_for_variant(bx.cx().tcx(), index)
.map_or(index.as_u32() as u128, |discr| discr.val);
return bx.cx().const_uint_big(cast_to, discr_val);
return bx.cx().const_uint_big(cast_to_llty, discr_val);
}
layout::Variants::Multiple { ref discr, ref discr_kind, discr_index, .. } => {
(discr, discr_kind, discr_index)
Expand All @@ -235,15 +236,22 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
// Decode the discriminant (specifically if it's niche-encoded).
match *discr_kind {
layout::DiscriminantKind::Tag => {
let signed = match discr_scalar.value {
let signed_cast = if discr_scalar.is_bool() {
// We use `i1` for bytes that are always `0` or `1`,
// e.g., `#[repr(i8)] enum E { A, B }`, but we can't
// let LLVM interpret the `i1` as signed, because
// then `i1 1` (i.e., `E::B`) is effectively `i8 -1`.
layout::Int(_, signed) => !discr_scalar.is_bool() && signed,
_ => false
false
} else {
match cast_to.abi {
layout::Abi::Scalar(layout::Scalar {
value: layout::Int(_, signed),
..
}) => signed,
_ => false
}
};
bx.intcast(encoded_discr.immediate(), cast_to, signed)
bx.intcast(encoded_discr.immediate(), cast_to_llty, signed_cast)
}
layout::DiscriminantKind::Niche {
dataful_variant,
Expand All @@ -264,22 +272,11 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
// and check that it is in the range `niche_variants`, because
// that might not fit in the same type, on top of needing an extra
// comparison (see also the comment on `let niche_discr`).
let relative_discr = if niche_start == 0 {
// Avoid subtracting `0`, which wouldn't work for pointers.
// FIXME(eddyb) check the actual primitive type here.
encoded_discr
} else {
bx.sub(encoded_discr, bx.cx().const_uint_big(niche_llty, niche_start))
};
let relative_discr =
bx.sub(encoded_discr, bx.cx().const_uint_big(niche_llty, niche_start));
let relative_max = niche_variants.end().as_u32() - niche_variants.start().as_u32();
let is_niche = {
let relative_max = if relative_max == 0 {
// Avoid calling `const_uint`, which wouldn't work for pointers.
// FIXME(eddyb) check the actual primitive type here.
bx.cx().const_null(niche_llty)
} else {
bx.cx().const_uint(niche_llty, relative_max as u64)
};
let relative_max = bx.cx().const_uint(niche_llty, relative_max as u64);
bx.icmp(IntPredicate::IntULE, relative_discr, relative_max)
};

Expand All @@ -295,20 +292,20 @@ impl<'a, 'tcx, V: CodegenObject> PlaceRef<'tcx, V> {
let relative_discr = if relative_max == 0 {
// HACK(eddyb) since we have only one niche, we know which
// one it is, and we can avoid having a dynamic value here.
bx.cx().const_uint(cast_to, 0)
bx.cx().const_uint(cast_to_llty, 0)
} else {
bx.intcast(relative_discr, cast_to, false)
bx.intcast(relative_discr, cast_to_llty, false)
};
bx.add(
relative_discr,
bx.cx().const_uint(cast_to, niche_variants.start().as_u32() as u64),
bx.cx().const_uint(cast_to_llty, niche_variants.start().as_u32() as u64),
)
};

bx.select(
is_niche,
niche_discr,
bx.cx().const_uint(cast_to, dataful_variant.as_u32() as u64),
bx.cx().const_uint(cast_to_llty, dataful_variant.as_u32() as u64),
)
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_lint/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -963,7 +963,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for VariantSizeDifferences {
_ => return,
};

let discr_size = tag.value.size(&cx.tcx).bytes();
let discr_size = tag.value.size().bytes();

debug!("enum `{}` is {} bytes large with layout:\n{:#?}",
t, layout.size.bytes(), layout);
Expand Down
Loading