Skip to content

Commit

Permalink
Auto merge of #39999 - bitshifter:struct_align, r=eddyb
Browse files Browse the repository at this point in the history
Implementation of repr struct alignment RFC 1358.

The main changes around rustc::ty::Layout::struct:
* Added abi_align field which stores abi alignment before repr align is applied
* align field contains transitive repr alignment
* Added padding vec which stores padding required after fields

The main user of this information is rustc_trans::adt::struct_llfields
which determines the LLVM fields to be used by LLVM, including padding
fields.

A possible future optimisation would be to put the padding Vec in an Option, since it will be unused unless you are using repr align.
  • Loading branch information
bors committed Apr 22, 2017
2 parents ff13b7c + 946f8e6 commit 6d841da
Show file tree
Hide file tree
Showing 30 changed files with 692 additions and 86 deletions.
1 change: 1 addition & 0 deletions src/doc/unstable-book/src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@
- [proc_macro](language-features/proc-macro.md)
- [quote](language-features/quote.md)
- [relaxed_adts](language-features/relaxed-adts.md)
- [repr_align](language-features/repr-align.md)
- [repr_simd](language-features/repr-simd.md)
- [rustc_attrs](language-features/rustc-attrs.md)
- [rustc_diagnostic_macros](language-features/rustc-diagnostic-macros.md)
Expand Down
11 changes: 11 additions & 0 deletions src/doc/unstable-book/src/language-features/repr-align.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# `repr_align`

The tracking issue for this feature is: [#33626]

[#33626]: https://github.com/rust-lang/rust/issues/33626

------------------------




3 changes: 2 additions & 1 deletion src/librustc/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1847,5 +1847,6 @@ register_diagnostics! {
E0489, // type/lifetime parameter not in scope here
E0490, // a value of type `..` is borrowed for too long
E0495, // cannot infer an appropriate lifetime due to conflicting requirements
E0566 // conflicting representation hints
E0566, // conflicting representation hints
E0587, // conflicting packed and align representation hints
}
17 changes: 17 additions & 0 deletions src/librustc/hir/check_attr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ impl<'a> CheckAttrVisitor<'a> {
};

let mut conflicting_reprs = 0;
let mut found_packed = false;
let mut found_align = false;

for word in words {

let name = match word.name() {
Expand Down Expand Up @@ -84,6 +87,7 @@ impl<'a> CheckAttrVisitor<'a> {
("attribute should be applied to struct or union",
"a struct or union")
} else {
found_packed = true;
continue
}
}
Expand All @@ -96,6 +100,15 @@ impl<'a> CheckAttrVisitor<'a> {
continue
}
}
"align" => {
found_align = true;
if target != Target::Struct {
("attribute should be applied to struct",
"a struct")
} else {
continue
}
}
"i8" | "u8" | "i16" | "u16" |
"i32" | "u32" | "i64" | "u64" |
"isize" | "usize" => {
Expand All @@ -117,6 +130,10 @@ impl<'a> CheckAttrVisitor<'a> {
span_warn!(self.sess, attr.span, E0566,
"conflicting representation hints");
}
if found_align && found_packed {
struct_span_err!(self.sess, attr.span, E0587,
"conflicting packed and align representation hints").emit();
}
}

fn check_attribute(&self, attr: &ast::Attribute, target: Target) {
Expand Down
89 changes: 83 additions & 6 deletions src/librustc/ty/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -548,8 +548,12 @@ pub type FieldPath = Vec<u32>;
/// A structure, a product type in ADT terms.
#[derive(PartialEq, Eq, Hash, Debug)]
pub struct Struct {
/// Maximum alignment of fields and repr alignment.
pub align: Align,

/// Primitive alignment of fields without repr alignment.
pub primitive_align: Align,

/// If true, no alignment padding is used.
pub packed: bool,

Expand Down Expand Up @@ -583,10 +587,20 @@ impl<'a, 'gcx, 'tcx> Struct {
fn new(dl: &TargetDataLayout, fields: &Vec<&'a Layout>,
repr: &ReprOptions, kind: StructKind,
scapegoat: Ty<'gcx>) -> Result<Struct, LayoutError<'gcx>> {
let packed = repr.packed();
if repr.packed() && repr.align > 0 {
bug!("Struct cannot be packed and aligned");
}

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

let mut ret = Struct {
align: if packed { dl.i8_align } else { dl.aggregate_align },
packed: packed,
align: align,
primitive_align: align,
packed: repr.packed(),
sized: true,
offsets: vec![],
memory_index: vec![],
Expand Down Expand Up @@ -660,7 +674,9 @@ impl<'a, 'gcx, 'tcx> Struct {
// Invariant: offset < dl.obj_size_bound() <= 1<<61
if !ret.packed {
let align = field.align(dl);
let primitive_align = field.primitive_align(dl);
ret.align = ret.align.max(align);
ret.primitive_align = ret.primitive_align.max(primitive_align);
offset = offset.abi_align(align);
}

Expand All @@ -671,6 +687,11 @@ impl<'a, 'gcx, 'tcx> Struct {
.map_or(Err(LayoutError::SizeOverflow(scapegoat)), Ok)?;
}

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

debug!("Struct::new min_size: {:?}", offset);
ret.min_size = offset;
Expand Down Expand Up @@ -836,12 +857,23 @@ impl<'a, 'gcx, 'tcx> Struct {
}
Ok(None)
}

pub fn over_align(&self) -> Option<u32> {
let align = self.align.abi();
let primitive_align = self.primitive_align.abi();
if align > primitive_align {
Some(align as u32)
} else {
None
}
}
}

/// An untagged union.
#[derive(PartialEq, Eq, Hash, Debug)]
pub struct Union {
pub align: Align,
pub primitive_align: Align,

pub min_size: Size,

Expand All @@ -851,8 +883,10 @@ pub struct Union {

impl<'a, 'gcx, 'tcx> Union {
fn new(dl: &TargetDataLayout, packed: bool) -> Union {
let align = if packed { dl.i8_align } else { dl.aggregate_align };
Union {
align: if packed { dl.i8_align } else { dl.aggregate_align },
align: align,
primitive_align: align,
min_size: Size::from_bytes(0),
packed: packed,
}
Expand All @@ -875,6 +909,7 @@ impl<'a, 'gcx, 'tcx> Union {

if !self.packed {
self.align = self.align.max(field.align(dl));
self.primitive_align = self.primitive_align.max(field.primitive_align(dl));
}
self.min_size = cmp::max(self.min_size, field.size(dl));
}
Expand All @@ -888,6 +923,16 @@ impl<'a, 'gcx, 'tcx> Union {
pub fn stride(&self) -> Size {
self.min_size.abi_align(self.align)
}

pub fn over_align(&self) -> Option<u32> {
let align = self.align.abi();
let primitive_align = self.primitive_align.abi();
if align > primitive_align {
Some(align as u32)
} else {
None
}
}
}

/// The first half of a fat pointer.
Expand Down Expand Up @@ -924,6 +969,7 @@ pub enum Layout {
/// If true, the size is exact, otherwise it's only a lower bound.
sized: bool,
align: Align,
primitive_align: Align,
element_size: Size,
count: u64
},
Expand Down Expand Up @@ -970,7 +1016,8 @@ pub enum Layout {
discr: Integer,
variants: Vec<Struct>,
size: Size,
align: Align
align: Align,
primitive_align: Align,
},

/// Two cases distinguished by a nullable pointer: the case with discriminant
Expand Down Expand Up @@ -1118,6 +1165,7 @@ impl<'a, 'gcx, 'tcx> Layout {
Array {
sized: true,
align: element.align(dl),
primitive_align: element.primitive_align(dl),
element_size: element_size,
count: count
}
Expand All @@ -1127,6 +1175,7 @@ impl<'a, 'gcx, 'tcx> Layout {
Array {
sized: false,
align: element.align(dl),
primitive_align: element.primitive_align(dl),
element_size: element.size(dl),
count: 0
}
Expand All @@ -1135,6 +1184,7 @@ impl<'a, 'gcx, 'tcx> Layout {
Array {
sized: false,
align: dl.i8_align,
primitive_align: dl.i8_align,
element_size: Size::from_bytes(1),
count: 0
}
Expand Down Expand Up @@ -1340,6 +1390,7 @@ impl<'a, 'gcx, 'tcx> Layout {
assert!(discr_max >= 0);
let (min_ity, _) = Integer::repr_discr(tcx, ty, &def.repr, 0, discr_max);
let mut align = dl.aggregate_align;
let mut primitive_align = dl.aggregate_align;
let mut size = Size::from_bytes(0);

// We're interested in the smallest alignment, so start large.
Expand Down Expand Up @@ -1369,6 +1420,7 @@ impl<'a, 'gcx, 'tcx> Layout {
}
size = cmp::max(size, st.min_size);
align = align.max(st.align);
primitive_align = primitive_align.max(st.primitive_align);
Ok(st)
}).collect::<Result<Vec<_>, _>>()?;

Expand Down Expand Up @@ -1435,7 +1487,8 @@ impl<'a, 'gcx, 'tcx> Layout {
discr: ity,
variants: variants,
size: size,
align: align
align: align,
primitive_align: primitive_align
}
}

Expand Down Expand Up @@ -1557,6 +1610,30 @@ impl<'a, 'gcx, 'tcx> Layout {
}
}

/// Returns alignment before repr alignment is applied
pub fn primitive_align(&self, dl: &TargetDataLayout) -> Align {
match *self {
Array { primitive_align, .. } | General { primitive_align, .. } => primitive_align,
Univariant { ref variant, .. } |
StructWrappedNullablePointer { nonnull: ref variant, .. } => {
variant.primitive_align
},

_ => self.align(dl)
}
}

/// Returns repr alignment if it is greater than the primitive alignment.
pub fn over_align(&self, dl: &TargetDataLayout) -> Option<u32> {
let align = self.align(dl);
let primitive_align = self.primitive_align(dl);
if align.abi() > primitive_align.abi() {
Some(align.abi() as u32)
} else {
None
}
}

pub fn field_offset<C: HasDataLayout>(&self,
cx: C,
i: usize,
Expand Down
11 changes: 9 additions & 2 deletions src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ use serialize::{self, Encodable, Encoder};
use std::borrow::Cow;
use std::cell::{Cell, RefCell, Ref};
use std::collections::BTreeMap;
use std::cmp;
use std::hash::{Hash, Hasher};
use std::ops::Deref;
use std::rc::Rc;
Expand Down Expand Up @@ -1470,10 +1471,12 @@ impl_stable_hash_for!(struct ReprFlags {
#[derive(Copy, Clone, Eq, PartialEq, RustcEncodable, RustcDecodable, Default)]
pub struct ReprOptions {
pub int: Option<attr::IntType>,
pub align: u16,
pub flags: ReprFlags,
}

impl_stable_hash_for!(struct ReprOptions {
align,
int,
flags
});
Expand All @@ -1482,7 +1485,7 @@ impl ReprOptions {
pub fn new(tcx: TyCtxt, did: DefId) -> ReprOptions {
let mut flags = ReprFlags::empty();
let mut size = None;

let mut max_align = 0;
for attr in tcx.get_attrs(did).iter() {
for r in attr::find_repr_attrs(tcx.sess.diagnostic(), attr) {
flags.insert(match r {
Expand All @@ -1493,6 +1496,10 @@ impl ReprOptions {
size = Some(i);
ReprFlags::empty()
},
attr::ReprAlign(align) => {
max_align = cmp::max(align, max_align);
ReprFlags::empty()
},
});
}
}
Expand All @@ -1506,7 +1513,7 @@ impl ReprOptions {
if !tcx.consider_optimizing(|| format!("Reorder fields of {:?}", tcx.item_path_str(did))) {
flags.insert(ReprFlags::IS_LINEAR);
}
ReprOptions { int: size, flags: flags }
ReprOptions { int: size, align: max_align, flags: flags }
}

#[inline]
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_trans/abi.rs
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,7 @@ impl<'a, 'tcx> ArgType<'tcx> {
// bitcasting to the struct type yields invalid cast errors.

// We instead thus allocate some scratch space...
let llscratch = bcx.alloca(ty, "abi_cast");
let llscratch = bcx.alloca(ty, "abi_cast", None);
base::Lifetime::Start.call(bcx, llscratch);

// ...where we first store the value...
Expand Down
Loading

0 comments on commit 6d841da

Please sign in to comment.