rustc: represent the discriminant as a field for Layout::{Raw,StructWrapped}NullablePointer.

eddyb · eddyb · commit 1dc572b85e1f · 2017-11-19T02:14:29.000+02:00
diff --git a/src/librustc/ty/layout.rs b/src/librustc/ty/layout.rs
@@ -12,6 +12,7 @@ pub use self::Integer::*;
 pub use self::Layout::*;
 pub use self::Primitive::*;
 
+use rustc_back::slice::ref_slice;
 use session::{self, DataTypeKind, Session};
 use ty::{self, Ty, TyCtxt, TypeFoldable, ReprOptions, ReprFlags};
 
@@ -582,7 +583,7 @@ pub enum Primitive {
     Pointer
 }
 
-impl Primitive {
+impl<'a, 'tcx> Primitive {
     pub fn size<C: HasDataLayout>(self, cx: C) -> Size {
         let dl = cx.data_layout();
 
@@ -611,6 +612,15 @@ impl Primitive {
             Pointer => dl.pointer_align
         }
     }
+
+    pub fn to_ty(&self, tcx: TyCtxt<'a, 'tcx, 'tcx>) -> Ty<'tcx> {
+        match *self {
+            Int(i) => i.to_ty(tcx, false),
+            F32 => tcx.types.f32,
+            F64 => tcx.types.f64,
+            Pointer => tcx.mk_mut_ptr(tcx.mk_nil()),
+        }
+    }
 }
 
 /// A structure, a product type in ADT terms.
@@ -1202,9 +1212,7 @@ impl<'a, 'tcx> Layout {
             let layout = tcx.intern_layout(layout);
             let fields = match *layout {
                 Scalar { .. } |
-                CEnum { .. } |
-                RawNullablePointer { .. } |
-                StructWrappedNullablePointer { .. } => {
+                CEnum { .. } => {
                     FieldPlacement::union(0)
                 }
 
@@ -1241,7 +1249,14 @@ impl<'a, 'tcx> Layout {
                     FieldPlacement::union(def.struct_variant().fields.len())
                 }
 
-                General { .. } => FieldPlacement::union(1)
+                General { .. } |
+                RawNullablePointer { .. } => FieldPlacement::union(1),
+
+                StructWrappedNullablePointer { ref discr_offset, .. } => {
+                    FieldPlacement::Arbitrary {
+                        offsets: ref_slice(discr_offset)
+                    }
+                }
             };
             Ok(CachedLayout {
                 layout,
@@ -1520,7 +1535,7 @@ impl<'a, 'tcx> Layout {
                     if let Some((discr, offset, primitive)) = choice {
                         // HACK(eddyb) work around not being able to move
                         // out of arrays with just the indexing operator.
-                        let st = if discr == 0 { st0 } else { st1 };
+                        let mut st = if discr == 0 { st0 } else { st1 };
 
                         // FIXME(eddyb) should take advantage of a newtype.
                         if offset.bytes() == 0 && primitive.size(dl) == st.stride() &&
@@ -1531,6 +1546,14 @@ impl<'a, 'tcx> Layout {
                             });
                         }
 
+                        let mut discr_align = primitive.align(dl);
+                        if offset.abi_align(discr_align) != offset {
+                            st.packed = true;
+                            discr_align = dl.i8_align;
+                        }
+                        st.align = st.align.max(discr_align);
+                        st.primitive_align = st.primitive_align.max(discr_align);
+
                         return success(StructWrappedNullablePointer {
                             nndiscr: discr as u64,
                             nonnull: st,
@@ -2292,7 +2315,7 @@ impl<'a, 'tcx> FullLayout<'tcx> {
             match tcx.struct_tail(pointee).sty {
                 ty::TySlice(element) => slice(element),
                 ty::TyStr => slice(tcx.types.u8),
-                ty::TyDynamic(..) => tcx.mk_mut_ptr(tcx.mk_nil()),
+                ty::TyDynamic(..) => Pointer.to_ty(tcx),
                 _ => bug!("FullLayout::field_type({:?}): not applicable", self)
             }
         };
@@ -2350,6 +2373,10 @@ impl<'a, 'tcx> FullLayout<'tcx> {
                             General { discr, .. } => {
                                 return [discr.to_ty(tcx, false)][i];
                             }
+                            RawNullablePointer { discr, .. } |
+                            StructWrappedNullablePointer { discr, .. } => {
+                                return [discr.to_ty(tcx)][i];
+                            }
                             _ if def.variants.len() > 1 => return [][i],
 
                             // Enums with one variant behave like structs.
diff --git a/src/librustc_trans/mir/lvalue.rs b/src/librustc_trans/mir/lvalue.rs
@@ -10,7 +10,7 @@
 
 use llvm::{self, ValueRef};
 use rustc::ty::{self, Ty, TypeFoldable};
-use rustc::ty::layout::{self, Align, Layout, LayoutOf, Size};
+use rustc::ty::layout::{self, Align, Layout, LayoutOf};
 use rustc::mir;
 use rustc::mir::tcx::LvalueTy;
 use rustc_data_structures::indexed_vec::Idx;
@@ -205,50 +205,68 @@ impl<'a, 'tcx> LvalueRef<'tcx> {
                 l = l.for_variant(variant_index)
             }
         }
-        let fty = l.field(ccx, ix).ty;
+        let field = l.field(ccx, ix);
+        let offset = l.fields.offset(ix).bytes();
 
         let alignment = self.alignment | Alignment::from(&*l);
 
         // Handle all the non-aggregate cases first.
         match *l {
             layout::UntaggedUnion { .. } => {
-                let ty = ccx.llvm_type_of(fty);
+                let ty = ccx.llvm_type_of(field.ty);
                 return LvalueRef::new_sized(
-                    bcx.pointercast(self.llval, ty.ptr_to()), fty, alignment);
+                    bcx.pointercast(self.llval, ty.ptr_to()), field.ty, alignment);
             }
-            layout::General { .. } if l.variant_index.is_none() => {
-                let ty = ccx.llvm_type_of(fty);
+            // Discriminant field of enums.
+            layout::General { .. } |
+            layout::RawNullablePointer { .. } |
+            layout::StructWrappedNullablePointer { .. } if l.variant_index.is_none() => {
+                let ty = ccx.llvm_type_of(field.ty);
+                let size = field.size(ccx).bytes();
+
+                // If the discriminant is not on a multiple of the primitive's size,
+                // we need to go through i8*. Also assume the worst alignment.
+                if offset % size != 0 {
+                    let byte_ptr = bcx.pointercast(self.llval, Type::i8p(ccx));
+                    let byte_ptr = bcx.inbounds_gep(byte_ptr, &[C_usize(ccx, offset)]);
+                    let byte_align = Alignment::Packed(Align::from_bytes(1, 1).unwrap());
+                    return LvalueRef::new_sized(
+                        bcx.pointercast(byte_ptr, ty.ptr_to()), field.ty, byte_align);
+                }
+
+                let discr_ptr = bcx.pointercast(self.llval, ty.ptr_to());
                 return LvalueRef::new_sized(
-                    bcx.pointercast(self.llval, ty.ptr_to()), fty, alignment);
+                    bcx.inbounds_gep(discr_ptr, &[C_usize(ccx, offset / size)]),
+                    field.ty, alignment);
             }
             layout::RawNullablePointer { nndiscr, .. } |
             layout::StructWrappedNullablePointer { nndiscr,  .. }
                 if l.variant_index.unwrap() as u64 != nndiscr => {
                 // The unit-like case might have a nonzero number of unit-like fields.
                 // (e.d., Result of Either with (), as one side.)
-                let ty = ccx.llvm_type_of(fty);
-                assert_eq!(ccx.size_of(fty).bytes(), 0);
+                let ty = ccx.llvm_type_of(field.ty);
+                assert_eq!(field.size(ccx).bytes(), 0);
                 return LvalueRef::new_sized(
-                    bcx.pointercast(self.llval, ty.ptr_to()), fty,
+                    bcx.pointercast(self.llval, ty.ptr_to()), field.ty,
                     Alignment::Packed(Align::from_bytes(1, 1).unwrap()));
             }
             layout::RawNullablePointer { .. } => {
-                let ty = ccx.llvm_type_of(fty);
+                let ty = ccx.llvm_type_of(field.ty);
                 return LvalueRef::new_sized(
-                    bcx.pointercast(self.llval, ty.ptr_to()), fty, alignment);
+                    bcx.pointercast(self.llval, ty.ptr_to()), field.ty, alignment);
             }
             _ => {}
         }
 
         let simple = || {
             LvalueRef {
                 llval: bcx.struct_gep(self.llval, l.llvm_field_index(ix)),
-                llextra: if !ccx.shared().type_has_metadata(fty) {
-                    ptr::null_mut()
-                } else {
+                llextra: if ccx.shared().type_has_metadata(field.ty) {
                     self.llextra
+                } else {
+                    ptr::null_mut()
                 },
-                ty: LvalueTy::from_ty(fty),
+                ty: LvalueTy::from_ty(field.ty),
                 alignment,
             }
         };
@@ -264,13 +282,13 @@ impl<'a, 'tcx> LvalueRef<'tcx> {
         // Simple case - we can just GEP the field
         //   * Packed struct - There is no alignment padding
         //   * Field is sized - pointer is properly aligned already
-        if is_packed || ccx.shared().type_is_sized(fty) {
+        if is_packed || !field.is_unsized() {
             return simple();
         }
 
         // If the type of the last field is [T], str or a foreign type, then we don't need to do
         // any adjusments
-        match fty.sty {
+        match field.ty.sty {
             ty::TySlice(..) | ty::TyStr | ty::TyForeign(..) => return simple(),
             _ => ()
         }
@@ -299,12 +317,10 @@ impl<'a, 'tcx> LvalueRef<'tcx> {
 
         let meta = self.llextra;
 
-
-        let offset = l.fields.offset(ix).bytes();
         let unaligned_offset = C_usize(ccx, offset);
 
         // Get the alignment of the field
-        let (_, align) = glue::size_and_align_of_dst(bcx, fty, meta);
+        let (_, align) = glue::size_and_align_of_dst(bcx, field.ty, meta);
 
         // Bump the unaligned offset up to the appropriate alignment using the
         // following expression:
@@ -323,39 +339,17 @@ impl<'a, 'tcx> LvalueRef<'tcx> {
         let byte_ptr = bcx.gep(byte_ptr, &[offset]);
 
         // Finally, cast back to the type expected
-        let ll_fty = ccx.llvm_type_of(fty);
+        let ll_fty = ccx.llvm_type_of(field.ty);
         debug!("struct_field_ptr: Field type is {:?}", ll_fty);
 
         LvalueRef {
             llval: bcx.pointercast(byte_ptr, ll_fty.ptr_to()),
             llextra: self.llextra,
-            ty: LvalueTy::from_ty(fty),
+            ty: LvalueTy::from_ty(field.ty),
             alignment,
         }
     }
 
-    // Return a pointer to the discriminant, given its type and offset.
-    fn gepi_discr_at_offset(self, bcx: &Builder,
-                            discr: ty::layout::Primitive,
-                            offset: Size)
-                            -> (ValueRef, Alignment) {
-        let size = discr.size(bcx.ccx);
-        let ptr_ty = Type::from_primitive(bcx.ccx, discr).ptr_to();
-
-        // If the discriminant is not on a multiple of the primitive's size,
-        // we need to go through i8*. Also assume the worst alignment.
-        if offset.bytes() % size.bytes() != 0 {
-            let byte_ptr = bcx.pointercast(self.llval, Type::i8p(bcx.ccx));
-            let byte_ptr = bcx.inbounds_gep(byte_ptr, &[C_usize(bcx.ccx, offset.bytes())]);
-            let byte_align = Alignment::Packed(Align::from_bytes(1, 1).unwrap());
-            return (bcx.pointercast(byte_ptr, ptr_ty), byte_align);
-        }
-
-        let discr_ptr = bcx.pointercast(self.llval, ptr_ty);
-        (bcx.inbounds_gep(discr_ptr, &[C_usize(bcx.ccx, offset.bytes() / size.bytes())]),
-         self.alignment)
-    }
-
     /// Helper for cases where the discriminant is simply loaded.
     fn load_discr(self, bcx: &Builder, ity: layout::Integer, ptr: ValueRef,
                   min: u64, max: u64) -> ValueRef {
@@ -394,16 +388,12 @@ impl<'a, 'tcx> LvalueRef<'tcx> {
                 self.load_discr(bcx, discr, ptr.llval, 0, variants.len() as u64 - 1)
             }
             layout::Univariant { .. } | layout::UntaggedUnion { .. } => C_u8(bcx.ccx, 0),
-            layout::RawNullablePointer { nndiscr, discr } |
-            layout::StructWrappedNullablePointer { nndiscr, discr, .. } => {
-                let discr_offset = match *l {
-                    layout::StructWrappedNullablePointer { discr_offset, .. } => discr_offset,
-                    _ => Size::from_bytes(0),
-                };
-                let (lldiscrptr, alignment) = self.gepi_discr_at_offset(bcx, discr, discr_offset);
-                let lldiscr = bcx.load(lldiscrptr, alignment.non_abi());
+            layout::RawNullablePointer { nndiscr, .. } |
+            layout::StructWrappedNullablePointer { nndiscr, .. } => {
+                let ptr = self.project_field(bcx, 0);
+                let lldiscr = bcx.load(ptr.llval, ptr.alignment.non_abi());
                 let cmp = if nndiscr == 0 { llvm::IntEQ } else { llvm::IntNE };
-                bcx.icmp(cmp, lldiscr, C_null(Type::from_primitive(bcx.ccx, discr)))
+                bcx.icmp(cmp, lldiscr, C_null(bcx.ccx.llvm_type_of(ptr.ty.to_ty(bcx.tcx()))))
             },
             _ => bug!("{} is not an enum", l.ty)
         };
@@ -434,14 +424,14 @@ impl<'a, 'tcx> LvalueRef<'tcx> {
             | layout::Vector { .. } => {
                 assert_eq!(to, 0);
             }
-            layout::RawNullablePointer { nndiscr, discr, .. } |
-            layout::StructWrappedNullablePointer { nndiscr, discr, .. } => {
+            layout::RawNullablePointer { nndiscr, .. } |
+            layout::StructWrappedNullablePointer { nndiscr, .. } => {
                 if to != nndiscr {
-                    let (use_memset, discr_offset) = match *l {
-                        layout::StructWrappedNullablePointer { discr_offset, .. } => {
-                            (target_sets_discr_via_memset(bcx), discr_offset)
+                    let use_memset = match *l {
+                        layout::StructWrappedNullablePointer { .. } => {
+                            target_sets_discr_via_memset(bcx)
                         }
-                        _ => (false, Size::from_bytes(0)),
+                        _ => false,
                     };
                     if use_memset {
                         // Issue #34427: As workaround for LLVM bug on
@@ -454,10 +444,9 @@ impl<'a, 'tcx> LvalueRef<'tcx> {
                         let align = C_u32(bcx.ccx, align.abi() as u32);
                         base::call_memset(bcx, llptr, fill_byte, size, align, false);
                     } else {
-                        let (lldiscrptr, alignment) =
-                            self.gepi_discr_at_offset(bcx, discr, discr_offset);
-                        bcx.store(C_null(Type::from_primitive(bcx.ccx, discr)),
-                            lldiscrptr, alignment.non_abi());
+                        let ptr = self.project_field(bcx, 0);
+                        bcx.store(C_null(bcx.ccx.llvm_type_of(ptr.ty.to_ty(bcx.tcx()))),
+                            ptr.llval, ptr.alignment.non_abi());
                     }
                 }
             }
diff --git a/src/librustc_trans/type_.rs b/src/librustc_trans/type_.rs
@@ -287,14 +287,4 @@ impl Type {
             I128 => Type::i128(cx),
         }
     }
-
-    pub fn from_primitive(cx: &CrateContext, p: layout::Primitive) -> Type {
-        use rustc::ty::layout::Primitive::*;
-        match p {
-            Int(i) => Type::from_integer(cx, i),
-            F32 => Type::f32(cx),
-            F64 => Type::f64(cx),
-            Pointer => Type::i8p(cx),
-        }
-    }
 }
diff --git a/src/test/run-pass/packed-struct-optimized-enum.rs b/src/test/run-pass/packed-struct-optimized-enum.rs
@@ -0,0 +1,25 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+#[repr(packed)]
+#[derive(Copy, Clone)]
+struct Packed<T>(T);
+
+fn main() {
+    let one = (Some(Packed((&(), 0))), true);
+    let two = [one, one];
+    let stride = (&two[1] as *const _ as usize) - (&two[0] as *const _ as usize);
+
+    // This can fail if rustc and LLVM disagree on the size of a type.
+    // In this case, `Option<Packed<(&(), u32)>>` was erronously not
+    // marked as packed despite needing alignment `1` and containing
+    // its `&()` discriminant, which has alignment larger than `1`.
+    assert_eq!(stride, std::mem::size_of_val(&one));
+}

Original file line number	Diff line number	Diff line change
`@@ -287,14 +287,4 @@ impl Type {`
`287`	`287`	`I128 => Type::i128(cx),`
`288`	`288`	`}`
`289`	`289`	`}`
`290`		`-`
`291`		`- pub fn from_primitive(cx: &CrateContext, p: layout::Primitive) -> Type {`
`292`		`- use rustc::ty::layout::Primitive::*;`
`293`		`- match p {`
`294`		`- Int(i) => Type::from_integer(cx, i),`
`295`		`- F32 => Type::f32(cx),`
`296`		`- F64 => Type::f64(cx),`
`297`		`- Pointer => Type::i8p(cx),`
`298`		`- }`
`299`		`- }`
`300`	`290`	`}`