Skip to content
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

Refactor layout to store offsets of fields, not offsets after fields #36904

Merged
merged 1 commit into from Oct 3, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
59 changes: 26 additions & 33 deletions src/librustc/ty/layout.rs
Expand Up @@ -511,11 +511,11 @@ pub struct Struct {
/// If true, the size is exact, otherwise it's only a lower bound.
pub sized: bool,

/// Offsets for the first byte after each field.
/// That is, field_offset(i) = offset_after_field[i - 1] and the
/// whole structure's size is the last offset, excluding padding.
// FIXME(eddyb) use small vector optimization for the common case.
pub offset_after_field: Vec<Size>
/// Offsets for the first byte of each field.
/// FIXME(eddyb) use small vector optimization for the common case.
pub offsets: Vec<Size>,

pub min_size: Size,
}

impl<'a, 'gcx, 'tcx> Struct {
Expand All @@ -524,7 +524,8 @@ impl<'a, 'gcx, 'tcx> Struct {
align: if packed { dl.i8_align } else { dl.aggregate_align },
packed: packed,
sized: true,
offset_after_field: vec![]
offsets: vec![],
min_size: Size::from_bytes(0),
}
}

Expand All @@ -534,12 +535,14 @@ impl<'a, 'gcx, 'tcx> Struct {
scapegoat: Ty<'gcx>)
-> Result<(), LayoutError<'gcx>>
where I: Iterator<Item=Result<&'a Layout, LayoutError<'gcx>>> {
self.offset_after_field.reserve(fields.size_hint().0);
self.offsets.reserve(fields.size_hint().0);

let mut offset = self.min_size;

for field in fields {
if !self.sized {
bug!("Struct::extend: field #{} of `{}` comes after unsized field",
self.offset_after_field.len(), scapegoat);
self.offsets.len(), scapegoat);
}

let field = field?;
Expand All @@ -548,34 +551,29 @@ impl<'a, 'gcx, 'tcx> Struct {
}

// Invariant: offset < dl.obj_size_bound() <= 1<<61
let mut offset = if !self.packed {
if !self.packed {
let align = field.align(dl);
self.align = self.align.max(align);
self.offset_after_field.last_mut().map_or(Size::from_bytes(0), |last| {
*last = last.abi_align(align);
*last
})
} else {
self.offset_after_field.last().map_or(Size::from_bytes(0), |&last| last)
};
offset = offset.abi_align(align);
}

self.offsets.push(offset);


offset = offset.checked_add(field.size(dl), dl)
.map_or(Err(LayoutError::SizeOverflow(scapegoat)), Ok)?;

self.offset_after_field.push(offset);
}

self.min_size = offset;

Ok(())
}

/// Get the size without trailing alignment padding.
pub fn min_size(&self) -> Size {
self.offset_after_field.last().map_or(Size::from_bytes(0), |&last| last)
}

/// Get the size with trailing aligment padding.
pub fn stride(&self) -> Size {
self.min_size().abi_align(self.align)
self.min_size.abi_align(self.align)
}

/// Determine whether a structure would be zero-sized, given its fields.
Expand Down Expand Up @@ -671,15 +669,6 @@ impl<'a, 'gcx, 'tcx> Struct {
}
Ok(None)
}

pub fn offset_of_field(&self, index: usize) -> Size {
assert!(index < self.offset_after_field.len());
if index == 0 {
Size::from_bytes(0)
} else {
self.offset_after_field[index-1]
}
}
}

/// An untagged union.
Expand Down Expand Up @@ -1138,7 +1127,7 @@ impl<'a, 'gcx, 'tcx> Layout {
});
let mut st = Struct::new(dl, false);
st.extend(dl, discr.iter().map(Ok).chain(fields), ty)?;
size = cmp::max(size, st.min_size());
size = cmp::max(size, st.min_size);
align = align.max(st.align);
Ok(st)
}).collect::<Result<Vec<_>, _>>()?;
Expand Down Expand Up @@ -1171,12 +1160,16 @@ impl<'a, 'gcx, 'tcx> Layout {
let old_ity_size = Int(min_ity).size(dl);
let new_ity_size = Int(ity).size(dl);
for variant in &mut variants {
for offset in &mut variant.offset_after_field {
for offset in &mut variant.offsets[1..] {
if *offset > old_ity_size {
break;
}
*offset = new_ity_size;
}
// We might be making the struct larger.
if variant.min_size <= old_ity_size {
variant.min_size = new_ity_size;
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/librustc_lint/types.rs
Expand Up @@ -738,7 +738,7 @@ impl LateLintPass for VariantSizeDifferences {
.zip(variants)
.map(|(variant, variant_layout)| {
// Subtract the size of the enum discriminant
let bytes = variant_layout.min_size().bytes()
let bytes = variant_layout.min_size.bytes()
.saturating_sub(discr_size);

debug!("- variant `{}` is {} bytes large", variant.node.name, bytes);
Expand Down
47 changes: 18 additions & 29 deletions src/librustc_trans/adt.rs
Expand Up @@ -632,7 +632,7 @@ fn struct_field_ptr<'blk, 'tcx>(bcx: &BlockAndBuilder<'blk, 'tcx>,
let meta = val.meta;


let offset = st.offset_of_field(ix).bytes();
let offset = st.offsets[ix].bytes();
let unaligned_offset = C_uint(bcx.ccx(), offset);

// Get the alignment of the field
Expand Down Expand Up @@ -695,9 +695,9 @@ pub fn trans_const<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, t: Ty<'tcx>, discr: D
let lldiscr = C_integral(Type::from_integer(ccx, d), discr.0 as u64, true);
let mut vals_with_discr = vec![lldiscr];
vals_with_discr.extend_from_slice(vals);
let mut contents = build_const_struct(ccx, &variant.offset_after_field[..],
&vals_with_discr[..], variant.packed);
let needed_padding = l.size(dl).bytes() - variant.min_size().bytes();
let mut contents = build_const_struct(ccx, &variant,
&vals_with_discr[..]);
let needed_padding = l.size(dl).bytes() - variant.min_size.bytes();
if needed_padding > 0 {
contents.push(padding(ccx, needed_padding));
}
Expand All @@ -711,7 +711,7 @@ pub fn trans_const<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, t: Ty<'tcx>, discr: D
layout::Univariant { ref variant, .. } => {
assert_eq!(discr, Disr(0));
let contents = build_const_struct(ccx,
&variant.offset_after_field[..], vals, variant.packed);
&variant, vals);
C_struct(ccx, &contents[..], variant.packed)
}
layout::Vector { .. } => {
Expand All @@ -728,9 +728,7 @@ pub fn trans_const<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, t: Ty<'tcx>, discr: D
}
layout::StructWrappedNullablePointer { ref nonnull, nndiscr, .. } => {
if discr.0 == nndiscr {
C_struct(ccx, &build_const_struct(ccx,
&nonnull.offset_after_field[..],
vals, nonnull.packed),
C_struct(ccx, &build_const_struct(ccx, &nonnull, vals),
false)
} else {
let fields = compute_fields(ccx, t, nndiscr as usize, false);
Expand All @@ -739,10 +737,7 @@ pub fn trans_const<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, t: Ty<'tcx>, discr: D
// field; see #8506.
C_null(type_of::sizing_type_of(ccx, ty))
}).collect::<Vec<ValueRef>>();
C_struct(ccx, &build_const_struct(ccx,
&nonnull.offset_after_field[..],
&vals[..],
false),
C_struct(ccx, &build_const_struct(ccx, &nonnull, &vals[..]),
false)
}
}
Expand All @@ -759,11 +754,10 @@ pub fn trans_const<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, t: Ty<'tcx>, discr: D
/// a two-element struct will locate it at offset 4, and accesses to it
/// will read the wrong memory.
fn build_const_struct<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
offset_after_field: &[layout::Size],
vals: &[ValueRef],
packed: bool)
st: &layout::Struct,
vals: &[ValueRef])
-> Vec<ValueRef> {
assert_eq!(vals.len(), offset_after_field.len());
assert_eq!(vals.len(), st.offsets.len());

if vals.len() == 0 {
return Vec::new();
Expand All @@ -772,24 +766,19 @@ fn build_const_struct<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
// offset of current value
let mut offset = 0;
let mut cfields = Vec::new();
let target_offsets = offset_after_field.iter().map(|i| i.bytes());
for (&val, target_offset) in vals.iter().zip(target_offsets) {
assert!(!is_undef(val));
cfields.push(val);
offset += machine::llsize_of_alloc(ccx, val_ty(val));
if !packed {
let val_align = machine::llalign_of_min(ccx, val_ty(val));
offset = roundup(offset, val_align);
}
if offset != target_offset {
let offsets = st.offsets.iter().map(|i| i.bytes());
for (&val, target_offset) in vals.iter().zip(offsets) {
if offset < target_offset {
cfields.push(padding(ccx, target_offset - offset));
offset = target_offset;
}
assert!(!is_undef(val));
cfields.push(val);
offset += machine::llsize_of_alloc(ccx, val_ty(val));
}

let size = offset_after_field.last().unwrap();
if offset < size.bytes() {
cfields.push(padding(ccx, size.bytes() - offset));
if offset < st.min_size.bytes() {
cfields.push(padding(ccx, st.min_size.bytes() - offset));
}

cfields
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_trans/common.rs
Expand Up @@ -127,7 +127,7 @@ pub fn type_is_imm_pair<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, ty: Ty<'tcx>)
Layout::FatPointer { .. } => true,
Layout::Univariant { ref variant, .. } => {
// There must be only 2 fields.
if variant.offset_after_field.len() != 2 {
if variant.offsets.len() != 2 {
return false;
}

Expand Down
13 changes: 1 addition & 12 deletions src/librustc_trans/glue.rs
Expand Up @@ -335,20 +335,9 @@ pub fn size_and_align_of_dst<'blk, 'tcx>(bcx: &BlockAndBuilder<'blk, 'tcx>,
let layout = ccx.layout_of(t);
debug!("DST {} layout: {:?}", t, layout);

// Returns size in bytes of all fields except the last one
// (we will be recursing on the last one).
fn local_prefix_bytes(variant: &ty::layout::Struct) -> u64 {
let fields = variant.offset_after_field.len();
if fields > 1 {
variant.offset_after_field[fields - 2].bytes()
} else {
0
}
}

let (sized_size, sized_align) = match *layout {
ty::layout::Layout::Univariant { ref variant, .. } => {
(local_prefix_bytes(variant), variant.align.abi())
(variant.offsets.last().map_or(0, |o| o.bytes()), variant.align.abi())
}
_ => {
bug!("size_and_align_of_dst: expcted Univariant for `{}`, found {:#?}",
Expand Down