Use type-alignment-sized integer for discriminant types

The previous behaviour of using the smallest type possible caused LLVM
to treat padding too conservatively, causing poor codegen. This commit
changes the behaviour to use an type-alignment-sized integer as the
discriminant. This keeps types the same size, but helps LLVM understand
the data structure a little better, resulting in better codegen.

Author: Aatch

src/librustc_trans/trans/adt.rs

@@ -256,7 +256,62 @@ fn represent_type_uncached<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
             assert!((cases.len() - 1) as i64 >= 0);
             let bounds = IntBounds { ulo: 0, uhi: (cases.len() - 1) as u64,
                                      slo: 0, shi: (cases.len() - 1) as i64 };
-            let ity = range_to_inttype(cx, hint, &bounds);
+            let min_ity = range_to_inttype(cx, hint, &bounds);
+
+            // Create the set of structs that represent each variant
+            // Use the minimum integer type we figured out above
+            let fields : Vec<_> = cases.iter().map(|c| {
+                let mut ftys = vec!(ty_of_inttype(min_ity));
+                ftys.push_all(c.tys.as_slice());
+                if dtor { ftys.push(ty::mk_bool()); }
+                mk_struct(cx, ftys.as_slice(), false, t)
+            }).collect();
+
+
+            // Check to see if we should use a different type for the
+            // discriminant. If the overall alignment of the type is
+            // the same as the first field in each variant, we can safely use
+            // an alignment-sized type.
+            // We increase the size of the discriminant to avoid LLVM copying
+            // padding when it doesn't need to. This normally causes unaligned
+            // load/stores and excessive memcpy/memset operations. By using a
+            // bigger integer size, LLVM can be sure about it's contents and
+            // won't be so conservative.
+            // This check is needed to avoid increasing the size of types when
+            // the alignment of the first field is smaller than the overall
+            // alignment of the type.
+            let (_, align) = union_size_and_align(fields.as_slice());
+            let mut use_align = true;
+            for st in fields.iter() {
+                // Get the first non-zero-sized field
+                let field = st.fields.iter().skip(1).filter(|ty| {
+                    let t = type_of::sizing_type_of(cx, **ty);
+                    machine::llsize_of_real(cx, t) != 0 ||
+                    // This case is only relevant for zero-sized types with large alignment
+                    machine::llalign_of_min(cx, t) != 1
+                }).next();
+
+                if let Some(field) = field {
+                    let field_align = type_of::align_of(cx, *field);
+                    if field_align != align {
+                        use_align = false;
+                        break;
+                    }
+                }
+            }
+            let ity = if use_align {
+                // Use the overall alignment
+                match align {
+                    1 => attr::UnsignedInt(ast::TyU8),
+                    2 => attr::UnsignedInt(ast::TyU16),
+                    4 => attr::UnsignedInt(ast::TyU32),
+                    8 if machine::llalign_of_min(cx, Type::i64(cx)) == 8 =>
+                        attr::UnsignedInt(ast::TyU64),
+                    _ => min_ity // use min_ity as a fallback
+                }
+            } else {
+                min_ity
+            };
 
             let fields : Vec<_> = cases.iter().map(|c| {
                 let mut ftys = vec!(ty_of_inttype(ity));
@@ -570,7 +625,7 @@ fn generic_type_of<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
             let discr_ty = ll_inttype(cx, ity);
             let discr_size = machine::llsize_of_alloc(cx, discr_ty);
             let align_units = (size + align_s - 1) / align_s - 1;
-            let pad_ty = match align_s {
+            let fill_ty = match align_s {
                 1 => Type::array(&Type::i8(cx), align_units),
                 2 => Type::array(&Type::i16(cx), align_units),
                 4 => Type::array(&Type::i32(cx), align_units),
@@ -580,11 +635,11 @@ fn generic_type_of<'a, 'tcx>(cx: &CrateContext<'a, 'tcx>,
                                                               align_units),
                 _ => panic!("unsupported enum alignment: {}", align)
             };
-            assert_eq!(machine::llalign_of_min(cx, pad_ty), align);
+            assert_eq!(machine::llalign_of_min(cx, fill_ty), align);
             assert_eq!(align_s % discr_size, 0);
-            let fields = vec!(discr_ty,
-                           Type::array(&discr_ty, align_s / discr_size - 1),
-                           pad_ty);
+            let fields = [discr_ty,
+                          Type::array(&discr_ty, align_s / discr_size - 1),
+                          fill_ty];
             match name {
                 None => Type::struct_(cx, fields.as_slice(), false),
                 Some(name) => {

src/test/run-pass/type-sizes.rs

@@ -18,6 +18,16 @@ struct w {a: int, b: ()}
 struct x {a: int, b: (), c: ()}
 struct y {x: int}
 
+enum e1 {
+    a(u8, u32), b(u32), c
+}
+enum e2 {
+    a(u32), b
+}
+enum e3 {
+    a([u64, ..0], u32), b
+}
+
 pub fn main() {
     assert_eq!(size_of::<u8>(), 1 as uint);
     assert_eq!(size_of::<u32>(), 4 as uint);
@@ -34,4 +44,10 @@ pub fn main() {
     assert_eq!(size_of::<w>(), size_of::<int>());
     assert_eq!(size_of::<x>(), size_of::<int>());
     assert_eq!(size_of::<int>(), size_of::<y>());
+
+    // Make sure enum types are the appropriate size, mostly
+    // around ensuring alignment is handled properly
+    assert_eq!(size_of::<e1>(), 8 as uint);
+    assert_eq!(size_of::<e2>(), 8 as uint);
+    assert_eq!(size_of::<e3>(), 16 as uint);
 }