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.

Sign up for GitHub

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

ty: use Align for ReprOptions pack and align. #63914

Merged
merged 1 commit into from
Aug 29, 2019
Merged

ty: use Align for ReprOptions pack and align. #63914

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
49 changes: 19 additions & 30 deletions src/librustc/ty/layout.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,14 +273,12 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
repr: &ReprOptions,
kind: StructKind) -> Result<LayoutDetails, LayoutError<'tcx>> {
let dl = self.data_layout();
let packed = repr.packed();
if packed && repr.align > 0 {
let pack = repr.pack;
if pack.is_some() && repr.align.is_some() {
bug!("struct cannot be packed and aligned");
}

let pack = Align::from_bytes(repr.pack as u64).unwrap();

let mut align = if packed {
let mut align = if pack.is_some() {
dl.i8_align
} else {
dl.aggregate_align
Expand All @@ -303,7 +301,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
};
let optimizing = &mut inverse_memory_index[..end];
let field_align = |f: &TyLayout<'_>| {
if packed { f.align.abi.min(pack) } else { f.align.abi }
if let Some(pack) = pack { f.align.abi.min(pack) } else { f.align.abi }
};
match kind {
StructKind::AlwaysSized |
Expand Down Expand Up @@ -334,7 +332,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
let mut largest_niche_available = 0;

if let StructKind::Prefixed(prefix_size, prefix_align) = kind {
let prefix_align = if packed {
let prefix_align = if let Some(pack) = pack {
prefix_align.min(pack)
} else {
prefix_align
Expand All @@ -355,7 +353,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
}

// Invariant: offset < dl.obj_size_bound() <= 1<<61
let field_align = if packed {
let field_align = if let Some(pack) = pack {
field.align.min(AbiAndPrefAlign::new(pack))
} else {
field.align
Expand All @@ -379,10 +377,8 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
.ok_or(LayoutError::SizeOverflow(ty))?;
}

if repr.align > 0 {
let repr_align = repr.align as u64;
align = align.max(AbiAndPrefAlign::new(Align::from_bytes(repr_align).unwrap()));
debug!("univariant repr_align: {:?}", repr_align);
if let Some(repr_align) = repr.align {
align = align.max(AbiAndPrefAlign::new(repr_align));
}

debug!("univariant min_size: {:?}", offset);
Expand Down Expand Up @@ -730,23 +726,18 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
}).collect::<Result<IndexVec<VariantIdx, _>, _>>()?;

if def.is_union() {
let packed = def.repr.packed();
if packed && def.repr.align > 0 {
bug!("Union cannot be packed and aligned");
if def.repr.pack.is_some() && def.repr.align.is_some() {
bug!("union cannot be packed and aligned");
}

let pack = Align::from_bytes(def.repr.pack as u64).unwrap();

let mut align = if packed {
let mut align = if def.repr.pack.is_some() {
dl.i8_align
} else {
dl.aggregate_align
};

if def.repr.align > 0 {
let repr_align = def.repr.align as u64;
align = align.max(
AbiAndPrefAlign::new(Align::from_bytes(repr_align).unwrap()));
if let Some(repr_align) = def.repr.align {
align = align.max(AbiAndPrefAlign::new(repr_align));
}

let optimize = !def.repr.inhibit_union_abi_opt();
Expand All @@ -755,13 +746,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
let index = VariantIdx::new(0);
for field in &variants[index] {
assert!(!field.is_unsized());

let field_align = if packed {
field.align.min(AbiAndPrefAlign::new(pack))
} else {
field.align
};
align = align.max(field_align);
align = align.max(field.align);

// If all non-ZST fields have the same ABI, forward this ABI
if optimize && !field.is_zst() {
Expand Down Expand Up @@ -796,6 +781,10 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
size = cmp::max(size, field.size);
}

if let Some(pack) = def.repr.pack {
align = align.min(AbiAndPrefAlign::new(pack));
}

return Ok(tcx.intern_layout(LayoutDetails {
variants: Variants::Single { index },
fields: FieldPlacement::Union(variants[index].len()),
Expand Down Expand Up @@ -1637,7 +1626,7 @@ impl<'tcx> LayoutCx<'tcx, TyCtxt<'tcx>> {
};

let adt_kind = adt_def.adt_kind();
let adt_packed = adt_def.repr.packed();
let adt_packed = adt_def.repr.pack.is_some();

let build_variant_info = |n: Option<Ident>,
flds: &[ast::Name],
Expand Down
28 changes: 17 additions & 11 deletions src/librustc/ty/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use arena::SyncDroplessArena;
use crate::session::DataTypeKind;

use rustc_serialize::{self, Encodable, Encoder};
use rustc_target::abi::Align;
use std::cell::RefCell;
use std::cmp::{self, Ordering};
use std::fmt;
Expand Down Expand Up @@ -2057,8 +2058,8 @@ impl_stable_hash_for!(struct ReprFlags {
#[derive(Copy, Clone, Debug, Eq, PartialEq, RustcEncodable, RustcDecodable, Default)]
pub struct ReprOptions {
pub int: Option<attr::IntType>,
pub align: u32,
pub pack: u32,
pub align: Option<Align>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can just be Align, although this is probably okay too (cc @oli-obk).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's what I tried initially, but some code in src/librustc_typeck/check/mod.rs explicitly checks if an alignment is set.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also use an enum

enum AlignRepr {
    Align(Align),
    Packed(Align),
}

To encode the fact that there can be only one of them.

pub pack: Option<Align>,
pub flags: ReprFlags,
}

Expand All @@ -2073,18 +2074,19 @@ impl ReprOptions {
pub fn new(tcx: TyCtxt<'_>, did: DefId) -> ReprOptions {
let mut flags = ReprFlags::empty();
let mut size = None;
let mut max_align = 0;
let mut min_pack = 0;
let mut max_align: Option<Align> = None;
let mut min_pack: Option<Align> = None;
for attr in tcx.get_attrs(did).iter() {
for r in attr::find_repr_attrs(&tcx.sess.parse_sess, attr) {
flags.insert(match r {
attr::ReprC => ReprFlags::IS_C,
attr::ReprPacked(pack) => {
min_pack = if min_pack > 0 {
cmp::min(pack, min_pack)
let pack = Align::from_bytes(pack as u64).unwrap();
min_pack = Some(if let Some(min_pack) = min_pack {
min_pack.min(pack)
} else {
pack
};
});
ReprFlags::empty()
},
attr::ReprTransparent => ReprFlags::IS_TRANSPARENT,
Expand All @@ -2094,7 +2096,7 @@ impl ReprOptions {
ReprFlags::empty()
},
attr::ReprAlign(align) => {
max_align = cmp::max(align, max_align);
max_align = max_align.max(Some(Align::from_bytes(align as u64).unwrap()));
ReprFlags::empty()
},
});
Expand All @@ -2113,7 +2115,7 @@ impl ReprOptions {
#[inline]
pub fn c(&self) -> bool { self.flags.contains(ReprFlags::IS_C) }
#[inline]
pub fn packed(&self) -> bool { self.pack > 0 }
pub fn packed(&self) -> bool { self.pack.is_some() }
#[inline]
pub fn transparent(&self) -> bool { self.flags.contains(ReprFlags::IS_TRANSPARENT) }
#[inline]
Expand All @@ -2133,8 +2135,12 @@ impl ReprOptions {
/// Returns `true` if this `#[repr()]` should inhibit struct field reordering
/// optimizations, such as with `repr(C)`, `repr(packed(1))`, or `repr(<int>)`.
pub fn inhibit_struct_field_reordering_opt(&self) -> bool {
self.flags.intersects(ReprFlags::IS_UNOPTIMISABLE) || self.pack == 1 ||
self.int.is_some()
if let Some(pack) = self.pack {
if pack.bytes() == 1 {
return true;
}
}
self.flags.intersects(ReprFlags::IS_UNOPTIMISABLE) || self.int.is_some()
}

/// Returns `true` if this `#[repr()]` should inhibit union ABI optimisations.
Expand Down
14 changes: 9 additions & 5 deletions src/librustc_typeck/check/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1859,14 +1859,18 @@ fn check_packed(tcx: TyCtxt<'_>, sp: Span, def_id: DefId) {
for attr in tcx.get_attrs(def_id).iter() {
for r in attr::find_repr_attrs(&tcx.sess.parse_sess, attr) {
if let attr::ReprPacked(pack) = r {
if pack != repr.pack {
struct_span_err!(tcx.sess, sp, E0634,
"type has conflicting packed representation hints").emit();
if let Some(repr_pack) = repr.pack {
if pack as u64 != repr_pack.bytes() {
struct_span_err!(
tcx.sess, sp, E0634,
"type has conflicting packed representation hints"
).emit();
}
}
}
}
}
if repr.align > 0 {
if repr.align.is_some() {
struct_span_err!(tcx.sess, sp, E0587,
"type has conflicting packed and align representation hints").emit();
}
Expand All @@ -1885,7 +1889,7 @@ fn check_packed_inner(tcx: TyCtxt<'_>, def_id: DefId, stack: &mut Vec<DefId>) ->
}
if let ty::Adt(def, substs) = t.sty {
if def.is_struct() || def.is_union() {
if tcx.adt_def(def.did).repr.align > 0 {
if tcx.adt_def(def.did).repr.align.is_some() {
return true;
}
// push struct def_id before checking fields
Expand Down