Auto merge of #37429 - camlorn:univariant_layout_optimization, r=eddyb

struct field reordering and optimization

This is work in progress.  The goal is to divorce the order of fields in source code from the order of fields in the LLVM IR, then optimize structs (and tuples/enum variants)by always ordering fields from least to most aligned.  It does not work yet.  I intend to check compiler memory usage as a benchmark, and a crater run will probably be required.

I don't know enough of the compiler to complete this work unaided.  If you see places that still need updating, please mention them.  The only one I know of currently is debuginfo, which I'm putting off intentionally until a bit later.

r? @eddyb

Author: bors

src/librustc/session/code_stats.rs

@@ -142,7 +142,12 @@ impl CodeStats {
                 max_variant_size = cmp::max(max_variant_size, size);
 
                 let mut min_offset = discr_size;
-                for field in fields {
+
+                // We want to print fields by increasing offset.
+                let mut fields = fields.clone();
+                fields.sort_by_key(|f| f.offset);
+
+                for field in fields.iter() {
                     let FieldInfo { ref name, offset, size, align } = *field;
 
                     // Include field alignment in output only if it caused padding injection

src/librustc/ty/layout.rs

@@ -31,6 +31,7 @@ mod imp {
     mod os {
         use libc;
 
+        #[repr(C)]
         pub struct flock {
             pub l_type: libc::c_short,
             pub l_whence: libc::c_short,
@@ -53,6 +54,7 @@ mod imp {
     mod os {
         use libc;
 
+        #[repr(C)]
         pub struct flock {
             pub l_start: libc::off_t,
             pub l_len: libc::off_t,
@@ -76,6 +78,7 @@ mod imp {
     mod os {
         use libc;
 
+        #[repr(C)]
         pub struct flock {
             pub l_start: libc::off_t,
             pub l_len: libc::off_t,
@@ -98,6 +101,7 @@ mod imp {
     mod os {
         use libc;
 
+        #[repr(C)]
         pub struct flock {
             pub l_type: libc::c_short,
             pub l_whence: libc::c_short,
@@ -119,6 +123,7 @@ mod imp {
     mod os {
         use libc;
 
+        #[repr(C)]
         pub struct flock {
             pub l_start: libc::off_t,
             pub l_len: libc::off_t,
@@ -141,6 +146,7 @@ mod imp {
     mod os {
         use libc;
 
+        #[repr(C)]
         pub struct flock {
             pub l_type: libc::c_short,
             pub l_whence: libc::c_short,

src/librustc_data_structures/flock.rs

@@ -750,7 +750,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for VariantSizeDifferences {
                 if let Layout::General { ref variants, ref size, discr, .. } = *layout {
                     let discr_size = Primitive::Int(discr).size(&cx.tcx.data_layout).bytes();
 
-                    debug!("enum `{}` is {} bytes large", t, size.bytes());
+                    debug!("enum `{}` is {} bytes large with layout:\n{:#?}",
+                      t, size.bytes(), layout);
 
                     let (largest, slargest, largest_index) = enum_definition.variants
                         .iter()

src/librustc_lint/types.rs

@@ -151,14 +151,14 @@ pub fn finish_type_of<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
         | layout::UntaggedUnion { .. } | layout::RawNullablePointer { .. } => { }
         layout::Univariant { ..}
         | layout::StructWrappedNullablePointer { .. } => {
-            let (nonnull_variant, packed) = match *l {
-                layout::Univariant { ref variant, .. } => (0, variant.packed),
+            let (nonnull_variant_index, nonnull_variant, packed) = match *l {
+                layout::Univariant { ref variant, .. } => (0, variant, variant.packed),
                 layout::StructWrappedNullablePointer { nndiscr, ref nonnull, .. } =>
-                    (nndiscr, nonnull.packed),
+                    (nndiscr, nonnull, nonnull.packed),
                 _ => unreachable!()
             };
-            let fields = compute_fields(cx, t, nonnull_variant as usize, true);
-            llty.set_struct_body(&struct_llfields(cx, &fields, false, false),
+            let fields = compute_fields(cx, t, nonnull_variant_index as usize, true);
+            llty.set_struct_body(&struct_llfields(cx, &fields, nonnull_variant, false, false),
                                  packed)
         },
         _ => bug!("This function cannot handle {} with layout {:#?}", t, l)
@@ -188,7 +188,7 @@ fn generic_type_of<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
             let fields = compute_fields(cx, t, nndiscr as usize, false);
             match name {
                 None => {
-                    Type::struct_(cx, &struct_llfields(cx, &fields, sizing, dst),
+                    Type::struct_(cx, &struct_llfields(cx, &fields, nonnull, sizing, dst),
                                   nonnull.packed)
                 }
                 Some(name) => {
@@ -203,7 +203,7 @@ fn generic_type_of<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
             let fields = compute_fields(cx, t, 0, true);
             match name {
                 None => {
-                    let fields = struct_llfields(cx, &fields, sizing, dst);
+                    let fields = struct_llfields(cx, &fields, &variant, sizing, dst);
                     Type::struct_(cx, &fields, variant.packed)
                 }
                 Some(name) => {
@@ -291,12 +291,14 @@ fn union_fill(cx: &CrateContext, size: u64, align: u64) -> Type {
 
 
 fn struct_llfields<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>, fields: &Vec<Ty<'tcx>>,
+                             variant: &layout::Struct,
                              sizing: bool, dst: bool) -> Vec<Type> {
+    let fields = variant.field_index_by_increasing_offset().map(|i| fields[i as usize]);
     if sizing {
-        fields.iter().filter(|&ty| !dst || type_is_sized(cx.tcx(), *ty))
-            .map(|&ty| type_of::sizing_type_of(cx, ty)).collect()
+        fields.filter(|ty| !dst || type_is_sized(cx.tcx(), *ty))
+            .map(|ty| type_of::sizing_type_of(cx, ty)).collect()
     } else {
-        fields.iter().map(|&ty| type_of::in_memory_type_of(cx, ty)).collect()
+        fields.map(|ty| type_of::in_memory_type_of(cx, ty)).collect()
     }
 }
 
@@ -564,16 +566,16 @@ pub fn trans_field_ptr_builder<'blk, 'tcx>(bcx: &BlockAndBuilder<'blk, 'tcx>,
 fn struct_field_ptr<'blk, 'tcx>(bcx: &BlockAndBuilder<'blk, 'tcx>,
                                 st: &layout::Struct, fields: &Vec<Ty<'tcx>>, val: MaybeSizedValue,
                                 ix: usize, needs_cast: bool) -> ValueRef {
-    let ccx = bcx.ccx();
     let fty = fields[ix];
+    let ccx = bcx.ccx();
     let ll_fty = type_of::in_memory_type_of(bcx.ccx(), fty);
     if bcx.is_unreachable() {
         return C_undef(ll_fty.ptr_to());
     }
 
     let ptr_val = if needs_cast {
-        let fields = fields.iter().map(|&ty| {
-            type_of::in_memory_type_of(ccx, ty)
+        let fields = st.field_index_by_increasing_offset().map(|i| {
+            type_of::in_memory_type_of(ccx, fields[i])
         }).collect::<Vec<_>>();
         let real_ty = Type::struct_(ccx, &fields[..], st.packed);
         bcx.pointercast(val.value, real_ty.ptr_to())
@@ -585,15 +587,15 @@ fn struct_field_ptr<'blk, 'tcx>(bcx: &BlockAndBuilder<'blk, 'tcx>,
     //   * First field - Always aligned properly
     //   * Packed struct - There is no alignment padding
     //   * Field is sized - pointer is properly aligned already
-    if ix == 0 || st.packed || type_is_sized(bcx.tcx(), fty) {
-        return bcx.struct_gep(ptr_val, ix);
+    if st.offsets[ix] == layout::Size::from_bytes(0) || st.packed || type_is_sized(bcx.tcx(), fty) {
+        return bcx.struct_gep(ptr_val, st.memory_index[ix] as usize);
     }
 
     // If the type of the last field is [T] or str, then we don't need to do
     // any adjusments
     match fty.sty {
         ty::TySlice(..) | ty::TyStr => {
-            return bcx.struct_gep(ptr_val, ix);
+            return bcx.struct_gep(ptr_val, st.memory_index[ix] as usize);
         }
         _ => ()
     }
@@ -755,8 +757,12 @@ fn build_const_struct<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>,
     // offset of current value
     let mut offset = 0;
     let mut cfields = Vec::new();
-    let offsets = st.offsets.iter().map(|i| i.bytes());
-    for (&val, target_offset) in vals.iter().zip(offsets) {
+    cfields.reserve(st.offsets.len()*2);
+
+    let parts = st.field_index_by_increasing_offset().map(|i| {
+        (&vals[i], st.offsets[i].bytes())
+    });
+    for (&val, target_offset) in parts {
         if offset < target_offset {
             cfields.push(padding(ccx, target_offset - offset));
             offset = target_offset;
@@ -807,14 +813,11 @@ pub fn const_get_field<'a, 'tcx>(ccx: &CrateContext<'a, 'tcx>, t: Ty<'tcx>,
     let l = ccx.layout_of(t);
     match *l {
         layout::CEnum { .. } => bug!("element access in C-like enum const"),
-        layout::Univariant { .. } | layout::Vector { .. } => const_struct_field(val, ix),
+        layout::Univariant { ref variant, .. } => {
+            const_struct_field(val, variant.memory_index[ix] as usize)
+        }
+        layout::Vector { .. } => const_struct_field(val, ix),
         layout::UntaggedUnion { .. } => const_struct_field(val, 0),
-        layout::General { .. } => const_struct_field(val, ix + 1),
-        layout::RawNullablePointer { .. } => {
-            assert_eq!(ix, 0);
-            val
-        },
-        layout::StructWrappedNullablePointer{ .. } => const_struct_field(val, ix),
         _ => bug!("{} does not have fields.", t)
     }
 }

src/librustc_trans/adt.rs

@@ -827,7 +827,9 @@ pub fn alloca(cx: Block, ty: Type, name: &str) -> ValueRef {
         }
     }
     DebugLoc::None.apply(cx.fcx);
-    Alloca(cx, ty, name)
+    let result = Alloca(cx, ty, name);
+    debug!("alloca({:?}) = {:?}", name, result);
+    result
 }
 
 impl<'blk, 'tcx> FunctionContext<'blk, 'tcx> {
@@ -1868,7 +1870,8 @@ fn gather_type_sizes<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>) {
         match **layout {
             Layout::StructWrappedNullablePointer { nonnull: ref variant_layout,
                                                    nndiscr,
-                                                   discrfield: _ } => {
+                                                   discrfield: _,
+                                                   discrfield_source: _ } => {
                 debug!("print-type-size t: `{:?}` adt struct-wrapped nullable nndiscr {} is {:?}",
                        ty, nndiscr, variant_layout);
                 let variant_def = &adt_def.variants[nndiscr as usize];

src/librustc_trans/base.rs

@@ -870,25 +870,28 @@ impl<'tcx> MemberDescriptionFactory<'tcx> {
 
 // Creates MemberDescriptions for the fields of a struct
 struct StructMemberDescriptionFactory<'tcx> {
+    ty: Ty<'tcx>,
     variant: &'tcx ty::VariantDef,
     substs: &'tcx Substs<'tcx>,
-    is_simd: bool,
     span: Span,
 }
 
 impl<'tcx> StructMemberDescriptionFactory<'tcx> {
     fn create_member_descriptions<'a>(&self, cx: &CrateContext<'a, 'tcx>)
                                       -> Vec<MemberDescription> {
-        let field_size = if self.is_simd {
-            let fty = monomorphize::field_ty(cx.tcx(),
-                                             self.substs,
-                                             &self.variant.fields[0]);
-            Some(machine::llsize_of_alloc(
-                cx,
-                type_of::type_of(cx, fty)
-            ) as usize)
-        } else {
-            None
+        let layout = cx.layout_of(self.ty);
+
+        let tmp;
+        let offsets = match *layout {
+            layout::Univariant { ref variant, .. } => &variant.offsets,
+            layout::Vector { element, count } => {
+                let element_size = element.size(&cx.tcx().data_layout).bytes();
+                tmp = (0..count).
+                  map(|i| layout::Size::from_bytes(i*element_size))
+                  .collect::<Vec<layout::Size>>();
+                &tmp
+            }
+            _ => bug!("{} is not a struct", self.ty)
         };
 
         self.variant.fields.iter().enumerate().map(|(i, f)| {
@@ -899,11 +902,7 @@ impl<'tcx> StructMemberDescriptionFactory<'tcx> {
             };
             let fty = monomorphize::field_ty(cx.tcx(), self.substs, f);
 
-            let offset = if self.is_simd {
-                FixedMemberOffset { bytes: i * field_size.unwrap() }
-            } else {
-                ComputedMemberOffset
-            };
+            let offset = FixedMemberOffset { bytes: offsets[i].bytes() as usize};
 
             MemberDescription {
                 name: name,
@@ -945,9 +944,9 @@ fn prepare_struct_metadata<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
         struct_metadata_stub,
         struct_llvm_type,
         StructMDF(StructMemberDescriptionFactory {
+            ty: struct_type,
             variant: variant,
             substs: substs,
-            is_simd: struct_type.is_simd(),
             span: span,
         })
     )
@@ -959,13 +958,21 @@ fn prepare_struct_metadata<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
 
 // Creates MemberDescriptions for the fields of a tuple
 struct TupleMemberDescriptionFactory<'tcx> {
+    ty: Ty<'tcx>,
     component_types: Vec<Ty<'tcx>>,
     span: Span,
 }
 
 impl<'tcx> TupleMemberDescriptionFactory<'tcx> {
     fn create_member_descriptions<'a>(&self, cx: &CrateContext<'a, 'tcx>)
                                       -> Vec<MemberDescription> {
+        let layout = cx.layout_of(self.ty);
+        let offsets = if let layout::Univariant { ref variant, .. } = *layout {
+            &variant.offsets
+        } else {
+            bug!("{} is not a tuple", self.ty);
+        };
+
         self.component_types
             .iter()
             .enumerate()
@@ -974,7 +981,7 @@ impl<'tcx> TupleMemberDescriptionFactory<'tcx> {
                 name: format!("__{}", i),
                 llvm_type: type_of::type_of(cx, component_type),
                 type_metadata: type_metadata(cx, component_type, self.span),
-                offset: ComputedMemberOffset,
+                offset: FixedMemberOffset { bytes: offsets[i].bytes() as usize },
                 flags: DIFlags::FlagZero,
             }
         }).collect()
@@ -1001,6 +1008,7 @@ fn prepare_tuple_metadata<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
                            NO_SCOPE_METADATA),
         tuple_llvm_type,
         TupleMDF(TupleMemberDescriptionFactory {
+            ty: tuple_type,
             component_types: component_types.to_vec(),
             span: span,
         })
@@ -1239,7 +1247,7 @@ impl<'tcx> EnumMemberDescriptionFactory<'tcx> {
             },
             layout::StructWrappedNullablePointer { nonnull: ref struct_def,
                                                 nndiscr,
-                                                ref discrfield, ..} => {
+                                                ref discrfield_source, ..} => {
                 // Create a description of the non-null variant
                 let (variant_type_metadata, variant_llvm_type, member_description_factory) =
                     describe_enum_variant(cx,
@@ -1262,12 +1270,12 @@ impl<'tcx> EnumMemberDescriptionFactory<'tcx> {
                 // member's name.
                 let null_variant_index = (1 - nndiscr) as usize;
                 let null_variant_name = adt.variants[null_variant_index].name;
-                let discrfield = discrfield.iter()
+                let discrfield_source = discrfield_source.iter()
                                            .skip(1)
                                            .map(|x| x.to_string())
                                            .collect::<Vec<_>>().join("$");
                 let union_member_name = format!("RUST$ENCODED$ENUM${}${}",
-                                                discrfield,
+                                                discrfield_source,
                                                 null_variant_name);
 
                 // Create the (singleton) list of descriptions of union members.
@@ -1289,6 +1297,8 @@ impl<'tcx> EnumMemberDescriptionFactory<'tcx> {
 
 // Creates MemberDescriptions for the fields of a single enum variant.
 struct VariantMemberDescriptionFactory<'tcx> {
+    // Cloned from the layout::Struct describing the variant.
+    offsets: &'tcx [layout::Size],
     args: Vec<(String, Ty<'tcx>)>,
     discriminant_type_metadata: Option<DIType>,
     span: Span,
@@ -1305,7 +1315,7 @@ impl<'tcx> VariantMemberDescriptionFactory<'tcx> {
                     Some(metadata) if i == 0 => metadata,
                     _ => type_metadata(cx, ty, self.span)
                 },
-                offset: ComputedMemberOffset,
+                offset: FixedMemberOffset { bytes: self.offsets[i].bytes() as usize },
                 flags: DIFlags::FlagZero
             }
         }).collect()
@@ -1325,7 +1335,7 @@ enum EnumDiscriminantInfo {
 // full RecursiveTypeDescription.
 fn describe_enum_variant<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
                                    enum_type: Ty<'tcx>,
-                                   struct_def: &layout::Struct,
+                                   struct_def: &'tcx layout::Struct,
                                    variant: &'tcx ty::VariantDef,
                                    discriminant_info: EnumDiscriminantInfo,
                                    containing_scope: DIScope,
@@ -1409,6 +1419,7 @@ fn describe_enum_variant<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
 
     let member_description_factory =
         VariantMDF(VariantMemberDescriptionFactory {
+            offsets: &struct_def.offsets[..],
             args: args,
             discriminant_type_metadata: match discriminant_info {
                 RegularDiscriminant(discriminant_type_metadata) => {