ms_struct layout replaces platform-specific behavior like

useBitFieldTypeAlignment() and appears to ignore the special
bit-packing semantics of __attribute__((packed)).

Further flesh out an already-extensive comment.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@201282 91177308-0d34-0410-b5e6-96231b3b80d8

lib/AST/RecordLayoutBuilder.cpp

@@ -1458,21 +1458,50 @@ void RecordLayoutBuilder::LayoutBitField(const FieldDecl *D) {
   // for ms_struct records it's also a multiple of the
   // LastBitfieldTypeSize (if set).
 
-  // The basic bitfield layout rule for ms_struct is to allocate an
-  // entire unit of the bitfield's declared type (e.g. 'unsigned
-  // long'), then parcel it up among successive bitfields whose
-  // declared types have the same size, making a new unit as soon as
-  // the last can no longer store the whole value.
-
-  // The standard bitfield layout rule for non-ms_struct is to place
-  // bitfields at the next available bit offset where the entire
-  // bitfield would fit in an aligned storage unit of the declared
-  // type (even if there are also non-bitfields within that same
-  // unit).  However, some targets (those that !useBitFieldTypeAlignment())
-  // don't require this storage unit to be aligned, and therefore
-  // always put the bit-field at the next available bit offset.
-  // Such targets generally do interpret zero-width bitfields as 
-  // forcing the use of a new storage unit.
+  // The struct-layout algorithm is dictated by the platform ABI,
+  // which in principle could use almost any rules it likes.  In
+  // practice, UNIXy targets tend to inherit the algorithm described
+  // in the System V generic ABI.  The basic bitfield layout rule in
+  // System V is to place bitfields at the next available bit offset
+  // where the entire bitfield would fit in an aligned storage unit of
+  // the declared type; it's okay if an earlier or later non-bitfield
+  // is allocated in the same storage unit.  However, some targets
+  // (those that !useBitFieldTypeAlignment(), e.g. ARM APCS) don't
+  // require this storage unit to be aligned, and therefore always put
+  // the bitfield at the next available bit offset.
+
+  // ms_struct basically requests a complete replacement of the
+  // platform ABI's struct-layout algorithm, with the high-level goal
+  // of duplicating MSVC's layout.  For non-bitfields, this follows
+  // the the standard algorithm.  The basic bitfield layout rule is to
+  // allocate an entire unit of the bitfield's declared type
+  // (e.g. 'unsigned long'), then parcel it up among successive
+  // bitfields whose declared types have the same size, making a new
+  // unit as soon as the last can no longer store the whole value.
+  // Since it completely replaces the platform ABI's algorithm,
+  // settings like !useBitFieldTypeAlignment() do not apply.
+
+  // A zero-width bitfield forces the use of a new storage unit for
+  // later bitfields.  In general, this occurs by rounding up the
+  // current size of the struct as if the algorithm were about to
+  // place a non-bitfield of the field's formal type.  Usually this
+  // does not change the alignment of the struct itself, but it does
+  // on some targets (those that useZeroLengthBitfieldAlignment(),
+  // e.g. ARM).  In ms_struct layout, zero-width bitfields are
+  // ignored unless they follow a non-zero-width bitfield.
+
+  // A field alignment restriction (e.g. from #pragma pack) or
+  // specification (e.g. from __attribute__((aligned))) changes the
+  // formal alignment of the field.  For System V, this alters the
+  // required alignment of the notional storage unit that must contain
+  // the bitfield.  For ms_struct, this only affects the placement of
+  // new storage units.  In both cases, the effect of #pragma pack is
+  // ignored on zero-width bitfields.
+
+  // On System V, a packed field (e.g. from #pragma pack or
+  // __attribute__((packed))) always uses the next available bit
+  // offset.
+
 
   // First, some simple bookkeeping to perform for ms_struct structs.
   if (IsMsStruct) {
@@ -1504,7 +1533,7 @@ void RecordLayoutBuilder::LayoutBitField(const FieldDecl *D) {
     IsUnion ? 0 : (getDataSizeInBits() - UnfilledBitsInLastUnit);
 
   // Handle targets that don't honor bitfield type alignment.
-  if (!Context.getTargetInfo().useBitFieldTypeAlignment()) {
+  if (!IsMsStruct && !Context.getTargetInfo().useBitFieldTypeAlignment()) {
     // Some such targets do honor it on zero-width bitfields.
     if (FieldSize == 0 &&
         Context.getTargetInfo().useZeroLengthBitfieldAlignment()) {
@@ -1524,7 +1553,7 @@ void RecordLayoutBuilder::LayoutBitField(const FieldDecl *D) {
   unsigned UnpackedFieldAlign = FieldAlign;
 
   // Ignore the field alignment if the field is packed.
-  if (FieldPacked)
+  if (!IsMsStruct && FieldPacked)
     FieldAlign = 1;
 
   // But, if there's an 'aligned' attribute on the field, honor that.

test/CodeGen/ms_struct-bitfield.c

@@ -1,7 +1,11 @@
 // RUN: %clang_cc1 -emit-llvm -o - -triple x86_64-apple-darwin9 %s | FileCheck %s
+// RUN: %clang_cc1 -emit-llvm -o - -triple thumbv7-apple-ios -target-abi apcs-gnu %s | FileCheck %s -check-prefix=CHECK-ARM
 
 // rdar://8823265
 
+// Note that we're declaring global variables with these types,
+// triggering both Sema and IRGen struct layout.
+
 #define ATTR __attribute__((__ms_struct__))
 
 struct
@@ -12,6 +16,7 @@ struct
 } ATTR t1;
 int s1 = sizeof(t1);
 // CHECK: @s1 = global i32 2
+// CHECK-ARM: @s1 = global i32 2
 
 struct
 {
@@ -23,6 +28,7 @@ struct
 } ATTR t2;
 int s2 = sizeof(t2);
 // CHECK: @s2 = global i32 2
+// CHECK-ARM: @s2 = global i32 2
 
 struct
 {
@@ -36,6 +42,7 @@ struct
 } ATTR t3;
 int s3 = sizeof(t3);
 // CHECK: @s3 = global i32 2
+// CHECK-ARM: @s3 = global i32 2
 
 struct
 {
@@ -44,6 +51,7 @@ struct
 } ATTR t4;
 int s4 = sizeof(t4);
 // CHECK: @s4 = global i32 1
+// CHECK-ARM: @s4 = global i32 1
 
 struct
 {
@@ -54,6 +62,7 @@ struct
 } ATTR t5;
 int s5 = sizeof(t5);
 // CHECK: @s5 = global i32 1
+// CHECK-ARM: @s5 = global i32 1
 
 struct
 {
@@ -64,6 +73,7 @@ struct
 } ATTR t6;
 int s6 = sizeof(t6);
 // CHECK: @s6 = global i32 1
+// CHECK-ARM: @s6 = global i32 1
 
 struct
 {
@@ -84,6 +94,7 @@ struct
 } ATTR t7;
 int s7 = sizeof(t7);
 // CHECK: @s7 = global i32 9
+// CHECK-ARM: @s7 = global i32 9
 
 struct
 {
@@ -93,6 +104,7 @@ struct
 } ATTR t8;
 int s8 = sizeof(t8);
 // CHECK: @s8 = global i32 0
+// CHECK-ARM: @s8 = global i32 0
 
 struct
 {
@@ -125,6 +137,7 @@ struct
 } ATTR t9;
 int s9 = sizeof(t9);
 // CHECK: @s9 = global i32 28
+// CHECK-ARM: @s9 = global i32 28
 
 struct
 {
@@ -134,3 +147,33 @@ struct
 } ATTR t10;
 int s10 = sizeof(t10);
 // CHECK: @s10 = global i32 16
+// CHECK-ARM: @s10 = global i32 8
+
+// rdar://16041826 - ensure that ms_structs work correctly on a
+// !useBitFieldTypeAlignment() target
+struct {
+  unsigned int a : 31;
+  unsigned int b : 2;
+  unsigned int c;
+} ATTR t11;
+int s11 = sizeof(t11);
+// CHECK: @s11 = global i32 12
+// CHECK-ARM: @s11 = global i32 12
+
+struct {
+  unsigned char a : 3;
+  unsigned char b : 4;
+  unsigned short c : 6;
+} ATTR t12;
+int s12 = sizeof(t12);
+// CHECK: @s12 = global i32 4
+// CHECK-ARM: @s12 = global i32 4
+
+struct {
+  unsigned char a : 3;
+  unsigned char b : 4;
+  __attribute__((packed)) unsigned short c : 6;
+} ATTR t13;
+int s13 = sizeof(t13);
+// CHECK: @s13 = global i32 4
+// CHECK-ARM: @s13 = global i32 4