IRGen: Use packed structs to accurately lay out aggregate types.

jckarter · jckarter · commit 2ac198d16b28 · 2013-10-09T04:12:46.000Z
LLVM's normal data layout doesn't jive with our own StructLayout's more aggressive use of padding space, and causes problems such as &lt;rdar://problem/14336903&gt;. Because we do our own alignment and stride management, we can safely use LLVM packed struct types with manual padding to get the level of control we need to accurately reflect our desired struct layout.

Swift SVN r9056
diff --git a/lib/IRGen/GenEnum.cpp b/lib/IRGen/GenEnum.cpp
@@ -2852,7 +2852,7 @@ namespace {
                                                    EnumDecl *theEnum,
                                                    llvm::StructType *enumTy) {
     if (ElementsWithPayload.empty()) {
-      enumTy->setBody(ArrayRef<llvm::Type*>{});
+      enumTy->setBody(ArrayRef<llvm::Type*>{}, /*isPacked*/ true);
       return registerEnumTypeInfo(new LoadableEnumTypeInfo(*this, enumTy,
                                                              Size(0), {},
                                                              Alignment(1),
@@ -2863,9 +2863,9 @@ namespace {
       // Use the singleton element's storage type if fixed-size.
       if (eltTI.isFixedSize()) {
         llvm::Type *body[] = { eltTI.StorageType };
-        enumTy->setBody(body);
+        enumTy->setBody(body, /*isPacked*/ true);
       } else {
-        enumTy->setBody(ArrayRef<llvm::Type*>{});
+        enumTy->setBody(ArrayRef<llvm::Type*>{}, /*isPacked*/ true);
       }
       
       if (TIK <= Opaque) {
@@ -2900,7 +2900,7 @@ namespace {
     Size tagSize(tagBytes);
     
     llvm::Type *body[] = { tagTy };
-    enumTy->setBody(body);
+    enumTy->setBody(body, /*isPacked*/true);
     
     // Unused tag bits in the physical size can be used as spare bits.
     // TODO: We can use all values greater than the largest discriminator as
@@ -2924,10 +2924,10 @@ namespace {
       auto extraTagTy = llvm::IntegerType::get(IGM.getLLVMContext(),
                                                extraTagBits);
       llvm::Type *body[] = { payloadEnumTy, extraTagTy };
-      bodyStruct->setBody(body);
+      bodyStruct->setBody(body, /*isPacked*/true);
     } else {
       llvm::Type *body[] = { payloadEnumTy };
-      bodyStruct->setBody(body);
+      bodyStruct->setBody(body, /*isPacked*/true);
     }
   }
   
@@ -2991,7 +2991,7 @@ namespace {
                                                   llvm::StructType *enumTy) {
     // The body is runtime-dependent, so we can't put anything useful here
     // statically.
-    enumTy->setBody(ArrayRef<llvm::Type*>{});
+    enumTy->setBody(ArrayRef<llvm::Type*>{}, /*isPacked*/true);
 
     // Layout has to be done when the value witness table is instantiated,
     // during initializeValueWitnessTable.
diff --git a/lib/IRGen/StructLayout.cpp b/lib/IRGen/StructLayout.cpp
@@ -211,14 +211,7 @@ void StructLayoutBuilder::addFixedSizeElement(ElementLayout &elt) {
       = eltAlignment.getValue() - offsetFromAlignment.getValue();
     assert(paddingRequired != 0);
 
-    // We don't actually need to uglify the IR unless the natural
-    // alignment of the IR type for the field isn't good enough.
-    // We also don't need to bother actually adding an IR field if
-    // the field can't be statically accessed.
-    Alignment fieldIRAlignment(
-          IGM.DataLayout.getABITypeAlignment(eltTI.StorageType));
-    assert(fieldIRAlignment <= eltAlignment);
-    if (fieldIRAlignment != eltAlignment && isFixedLayout()) {
+    if (isFixedLayout()) {
       auto paddingTy = llvm::ArrayType::get(IGM.Int8Ty, paddingRequired);
       StructFields.push_back(paddingTy);
     }
@@ -292,11 +285,13 @@ void StructLayoutBuilder::addNonFixedSizeElementAtOffsetZero(ElementLayout &elt)
 
 /// Produce the current fields as an anonymous structure.
 llvm::StructType *StructLayoutBuilder::getAsAnonStruct() const {
-  return llvm::StructType::get(IGM.getLLVMContext(), StructFields);
+  auto ty = llvm::StructType::get(IGM.getLLVMContext(), StructFields,
+                                  /*isPacked*/ true);
+  return ty;
 }
 
 /// Set the current fields as the body of the given struct type.
 void StructLayoutBuilder::setAsBodyOfStruct(llvm::StructType *type) const {
   assert(type->isOpaque());
-  type->setBody(StructFields);
+  type->setBody(StructFields, /*isPacked*/ true);
 }
diff --git a/test/IRGen/enum.sil b/test/IRGen/enum.sil
@@ -3,37 +3,37 @@
 import Builtin
 
 // -- Singleton enum. The representation is just the singleton payload.
-// CHECK: %O4enum9Singleton = type { { i64, i64 } }
+// CHECK: %O4enum9Singleton = type <{ <{ i64, i64 }> }>
 
 // -- No-payload enums. The representation is just an enum tag.
-// CHECK: %O4enum10NoPayloads = type { i2 }
-// CHECK: %O4enum11NoPayloads2 = type { i3 }
+// CHECK: %O4enum10NoPayloads = type <{ i2 }>
+// CHECK: %O4enum11NoPayloads2 = type <{ i3 }>
 
 // -- Single-payload enum, no extra inhabitants in the payload type. The
 //    representation adds a tag bit to distinguish payload from enum tag:
 //      case x(i64): X0 X1 X2 ... X63 | 0, where X0...X63 are the payload bits
 //      case y:      0  0  0  ... 0   | 1
 //      case z:      1  0  0  ... 0   | 1
-// CHECK: %O4enum17SinglePayloadNoXI = type { i64, i1 }
-// CHECK: %O4enum18SinglePayloadNoXI2 = type { i64, i1 }
+// CHECK: %O4enum17SinglePayloadNoXI = type <{ i64, i1 }>
+// CHECK: %O4enum18SinglePayloadNoXI2 = type <{ i64, i1 }>
 
 // -- Single-payload enum, spare bits. The representation uses a tag bit
 //    out of the payload to distinguish payload from enum tag:
 //      case x(i3): X0 X1 X2 0 0 0 0 0
 //      case y:     0  0  0  1 0 0 0 0
 //      case z:     1  0  0  1 0 0 0 0
-// CHECK: %O4enum21SinglePayloadSpareBit = type { i64 }
+// CHECK: %O4enum21SinglePayloadSpareBit = type <{ i64 }>
 
 // -- Single-payload enum containing a no-payload enum as its payload.
 //    The representation takes extra inhabitants starting after the greatest
 //    discriminator value used by the nested no-payload enum.
-// CHECK: %O4enum19SinglePayloadNested = type { i8 }
+// CHECK: %O4enum19SinglePayloadNested = type <{ i8 }>
 
 // -- Single-payload enum containing another single-payload enum as its
 //    payload.
 //    The representation takes extra inhabitants from the nested enum's payload
 //    that were unused by the nested enum.
-// CHECK: %O4enum25SinglePayloadNestedNested = type { i8 }
+// CHECK: %O4enum25SinglePayloadNestedNested = type <{ i8 }>
 
 // -- Multi-payload enum, no spare bits. The representation adds tag bits
 //    to discriminate payloads. No-payload cases all share a tag.
@@ -43,7 +43,7 @@ import Builtin
 //      case a:     0  0  0  ... 0  0  | 1 1
 //      case b:     1  0  0  ... 0  0  | 1 1
 //      case c:     0  1  0  ... 0  0  | 1 1
-// CHECK: %O4enum23MultiPayloadNoSpareBits = type { i64, i2 }
+// CHECK: %O4enum23MultiPayloadNoSpareBits = type <{ i64, i2 }>
 
 // -- Multi-payload enum, one spare bit. The representation uses spare bits
 //    common to all payloads to partially discriminate payloads, with added
@@ -54,7 +54,7 @@ import Builtin
 //      case a:     0  0  0  0  0  0  0  1 | 1
 //      case b:     1  0  0  0  0  0  0  1 | 1
 //      case c:     0  1  0  0  0  0  0  1 | 1
-// CHECK: %O4enum23MultiPayloadOneSpareBit = type { i64, i1 }
+// CHECK: %O4enum23MultiPayloadOneSpareBit = type <{ i64, i1 }>
 
 // -- Multi-payload enum, two spare bits. Same as above, except we have enough
 //    spare bits not to require any added tag bits.
@@ -64,12 +64,12 @@ import Builtin
 //      case a:     0  0  0  0  0  0  1  1
 //      case b:     1  0  0  0  0  0  1  1
 //      case c:     0  1  0  0  0  0  1  1
-// CHECK: %O4enum24MultiPayloadTwoSpareBits = type { i64 }
+// CHECK: %O4enum24MultiPayloadTwoSpareBits = type <{ i64 }>
 
 // -- Dynamic enums. The type layout is opaque; we dynamically bitcast to
 //    the element type.
-// CHECK: %O4enum20DynamicSinglePayload = type {}
-// CHECK: %O4enum16DynamicSingleton = type {}
+// CHECK: %O4enum20DynamicSinglePayload = type <{}>
+// CHECK: %O4enum16DynamicSingleton = type <{}>
 
 // -- Dynamic metadata template carries a value witness table pattern
 //    we fill in on instantiation.
@@ -174,10 +174,10 @@ dest(%u2 : $(Builtin.Int64, Builtin.Int64)):
 // CHECK: entry:
 // CHECK:   br label %[[PREDEST:[0-9]+]]
 // CHECK: ; <label>:[[PREDEST]]
-// CHECK:   [[ADDR:%.*]] = bitcast %O4enum9Singleton* %0 to { i64, i64 }*
+// CHECK:   [[ADDR:%.*]] = bitcast %O4enum9Singleton* %0 to <{ i64, i64 }>*
 // CHECK:   br label %[[DEST:[0-9]+]]
 // CHECK: ; <label>:[[DEST]]
-// CHECK:   phi { i64, i64 }* [ [[ADDR]], %[[PREDEST]] ]
+// CHECK:   phi <{ i64, i64 }>* [ [[ADDR]], %[[PREDEST]] ]
 // CHECK:   ret void
 // CHECK: }
 sil @singleton_switch_indirect : $([inout] Singleton) -> () {
@@ -203,10 +203,10 @@ entry(%0 : $Builtin.Int64, %1 : $Builtin.Int64):
 
 // CHECK: define void @singleton_inject_indirect(i64, i64, %O4enum9Singleton*) {
 // CHECK: entry:
-// CHECK:   [[DATA_ADDR:%.*]] = bitcast %O4enum9Singleton* %2 to { i64, i64 }*
-// CHECK:   [[DATA_0_ADDR:%.*]] = getelementptr inbounds { i64, i64 }* [[DATA_ADDR]], i32 0, i32 0
+// CHECK:   [[DATA_ADDR:%.*]] = bitcast %O4enum9Singleton* %2 to <{ i64, i64 }>*
+// CHECK:   [[DATA_0_ADDR:%.*]] = getelementptr inbounds <{ i64, i64 }>* [[DATA_ADDR]], i32 0, i32 0
 // CHECK:   store i64 %0, i64* [[DATA_0_ADDR]], align 8
-// CHECK:   [[DATA_1_ADDR:%.*]] = getelementptr inbounds { i64, i64 }* [[DATA_ADDR]], i32 0, i32 1
+// CHECK:   [[DATA_1_ADDR:%.*]] = getelementptr inbounds <{ i64, i64 }>* [[DATA_ADDR]], i32 0, i32 1
 // CHECK:   store i64 %1, i64* [[DATA_1_ADDR]], align 8
 // CHECK:   ret void
 // CHECK: }
diff --git a/test/IRGen/partial_apply.sil b/test/IRGen/partial_apply.sil
@@ -36,7 +36,7 @@ entry(%x : $Builtin.Int64, %c : $ObjCClass):
 // CHECK-LABEL: define { i8*, %swift.refcounted* } @indirect_partial_apply(i8*, %swift.refcounted*, i64) {
 // CHECK: entry:
 // CHECK:   [[OBJ:%.*]] = call noalias %swift.refcounted* @swift_allocObject(%swift.type* getelementptr inbounds (%swift.full_heapmetadata* @metadata, i32 0, i32 2), i64 40, i64 7)
-// CHECK:   [[DATA_ADDR:%.*]] = bitcast %swift.refcounted* [[OBJ]] to [[DATA_TYPE:{ %swift.refcounted, i64, %swift.refcounted\*, i8\* }]]*
+// CHECK:   [[DATA_ADDR:%.*]] = bitcast %swift.refcounted* [[OBJ]] to [[DATA_TYPE:<{ %swift.refcounted, i64, %swift.refcounted\*, i8\* }>]]*
 // CHECK:   [[X_ADDR:%.*]] = getelementptr inbounds [[DATA_TYPE]]* [[DATA_ADDR]], i32 0, i32 1
 // CHECK:   store i64 %2, i64* [[X_ADDR]], align 8
 // CHECK:   [[CONTEXT_ADDR:%.*]] = getelementptr inbounds [[DATA_TYPE]]* [[DATA_ADDR]], i32 0, i32 2
@@ -69,7 +69,7 @@ entry(%f : $(Builtin.Int64) -> (), %x : $Builtin.Int64):
 // CHECK-LABEL: define { i8*, %swift.refcounted* } @objc_partial_apply(%CSo9ObjCClass*) {
 // CHECK: entry:
 // CHECK:   [[OBJ:%.*]] = call noalias %swift.refcounted* @swift_allocObject(%swift.type* getelementptr inbounds (%swift.full_heapmetadata* @metadata2, i32 0, i32 2), i64 24, i64 7) #0
-// CHECK:   [[DATA_ADDR:%.*]] = bitcast %swift.refcounted* [[OBJ]] to [[DATA_TYPE:{ %swift.refcounted, %CSo9ObjCClass\* }]]*
+// CHECK:   [[DATA_ADDR:%.*]] = bitcast %swift.refcounted* [[OBJ]] to [[DATA_TYPE:<{ %swift.refcounted, %CSo9ObjCClass\* }>]]*
 // CHECK:   [[X_ADDR:%.*]] = getelementptr inbounds [[DATA_TYPE]]* [[DATA_ADDR]], i32 0, i32 1
 // CHECK:   store %CSo9ObjCClass* %0, %CSo9ObjCClass** [[X_ADDR]], align 8
 // CHECK:   [[RET:%.*]] = insertvalue { i8*, %swift.refcounted* } { i8* bitcast (void (i64, %swift.refcounted*)* [[OBJC_PARTIAL_APPLY_STUB:@partial_apply[0-9]*]] to i8*), %swift.refcounted* undef }, %swift.refcounted* [[OBJ]], 1
diff --git a/test/IRGen/unowned.sil b/test/IRGen/unowned.sil
@@ -3,9 +3,9 @@
 // CHECK: [[OPAQUE:%swift.opaque]] = type opaque
 // CHECK: [[TYPE:%swift.type]] = type
 // CHECK: [[REF:%swift.refcounted]] = type
-// CHECK: [[C:%C7unowned1C]] = type { [[REF]] }
+// CHECK: [[C:%C7unowned1C]] = type <{ [[REF]] }>
 // CHECK: [[UNKNOWN:%objc_object]] = type opaque
-// CHECK: [[A:%V7unowned1A]] = type { [[C]]* }
+// CHECK: [[A:%V7unowned1A]] = type <{ [[C]]* }>
 
 class C {}
 protocol [class_protocol] P {
diff --git a/test/IRGen/weak.sil b/test/IRGen/weak.sil
@@ -3,10 +3,10 @@
 // CHECK: [[OPAQUE:%swift.opaque]] = type opaque
 // CHECK: [[TYPE:%swift.type]] = type
 // CHECK: [[REF:%swift.refcounted]] = type
-// CHECK: [[A:%V4weak1A]] = type { [[WEAK:%swift.weak]] }
+// CHECK: [[A:%V4weak1A]] = type <{ [[WEAK:%swift.weak]] }>
 // CHECK: [[WEAK]] = type
-// CHECK: [[C:%C4weak1C]] = type { [[REF]] }
-// CHECK: [[B:%V4weak1B]] = type { { i8**, [[WEAK]] } }
+// CHECK: [[C:%C4weak1C]] = type <{ [[REF]] }>
+// CHECK: [[B:%V4weak1B]] = type <{ { i8**, [[WEAK]] } }>
 // CHECK: [[UNKNOWN:%objc_object]] = type opaque
 
 import swift
