[ms-abi] Change the way alignment is tracked

This patch more cleanly seperates the concepts of Preferred Alignment
and Required Alignment.  Most notable that changes to Required Alignment
do *not* impact preferred alignment until late in struct layout.  This
is observable when using pragma pack and non-virtual bases and the use
of tail padding when laying them out.

Test cases included.

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

diff --git a/lib/AST/RecordLayoutBuilder.cpp b/lib/AST/RecordLayoutBuilder.cpp
index 6a708a7e8d70c65776f38ce46ffb1a82b489232a..ff6b950474d47ec8d03eb607da77eb1fca7715fd 100644 (file)
--- a/lib/AST/RecordLayoutBuilder.cpp
+++ b/lib/AST/RecordLayoutBuilder.cpp
@@ -2077,10 +2077,6 @@ public:
   /// __declspec(align) into account.  It also updates RequiredAlignment as a
   /// side effect because it is most convenient to do so here.
   ElementInfo getAdjustedElementInfo(const FieldDecl *FD);
-  /// \brief Updates the alignment of the record.
-  void updateAlignment(CharUnits MemberAlignment) {
-    Alignment = std::max(Alignment, MemberAlignment);
-  }
   /// \brief Places a field at an offset in CharUnits.
   void placeFieldAtOffset(CharUnits FieldOffset) {
     FieldOffsets.push_back(Context.toBits(FieldOffset));
@@ -2157,7 +2153,9 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo(
   if (Layout.hasZeroSizedSubObject())
     HasZeroSizedSubObject = true;
   // Respect required alignment, this is necessary because we may have adjusted
-  // the alignment in the case of pragam pack.
+  // the alignment in the case of pragam pack.  Note that the required alignment
+  // doesn't actually apply to the struct alignment at this point.
+  Alignment = std::max(Alignment, Info.Alignment);
   Info.Alignment = std::max(Info.Alignment, Layout.getRequiredAlignment());
   Info.Size = Layout.getDataSize();
   return Info;
@@ -2204,6 +2202,9 @@ MicrosoftRecordLayoutBuilder::getAdjustedElementInfo(
     // Capture required alignment as a side-effect.
     RequiredAlignment = std::max(RequiredAlignment, FieldRequiredAlignment);
   }
+  // TODO: Add a Sema warning that MS ignores bitfield alignment in unions.
+  if (!(FD->isBitField() && IsUnion))
+    Alignment = std::max(Alignment, Info.Alignment);
   return Info;
 }
 
@@ -2356,7 +2357,6 @@ void MicrosoftRecordLayoutBuilder::layoutNonVirtualBase(
   CharUnits BaseOffset = Size.RoundUpToAlignment(Info.Alignment);
   Bases.insert(std::make_pair(BaseDecl, BaseOffset));
   Size = BaseOffset + BaseLayout.getDataSize();
-  updateAlignment(Info.Alignment);
   PreviousBaseLayout = &BaseLayout;
   VBPtrOffset = Size;
 }
@@ -2384,7 +2384,6 @@ void MicrosoftRecordLayoutBuilder::layoutField(const FieldDecl *FD) {
     placeFieldAtOffset(FieldOffset);
     Size = FieldOffset + Info.Size;
   }
-  updateAlignment(Info.Alignment);
 }
 
 void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) {
@@ -2412,13 +2411,11 @@ void MicrosoftRecordLayoutBuilder::layoutBitField(const FieldDecl *FD) {
   if (IsUnion) {
     placeFieldAtOffset(CharUnits::Zero());
     Size = std::max(Size, Info.Size);
-    // TODO: Add a Sema warning that MS ignores bitfield alignment in unions.
   } else {
     // Allocate a new block of memory and place the bitfield in it.
     CharUnits FieldOffset = Size.RoundUpToAlignment(Info.Alignment);
     placeFieldAtOffset(FieldOffset);
     Size = FieldOffset + Info.Size;
-    updateAlignment(Info.Alignment);
     RemainingBitsInField = Context.toBits(Info.Size) - Width;
   }
 }
@@ -2443,7 +2440,6 @@ MicrosoftRecordLayoutBuilder::layoutZeroWidthBitField(const FieldDecl *FD) {
     CharUnits FieldOffset = Size.RoundUpToAlignment(Info.Alignment);
     placeFieldAtOffset(FieldOffset);
     Size = FieldOffset;
-    updateAlignment(Info.Alignment);
   }
 }
 
@@ -2475,8 +2471,6 @@ void MicrosoftRecordLayoutBuilder::injectVBPtr(const CXXRecordDecl *RD) {
        i != e; ++i)
        if (i->second >= InjectionSite)
          i->second += Offset;
-  // Update the object alignment.
-  updateAlignment(PointerInfo.Alignment);
   // The presence of a vbptr suppresses zero sized objects that are not in
   // virtual bases.
   HasZeroSizedSubObject = false;
@@ -2500,7 +2494,6 @@ void MicrosoftRecordLayoutBuilder::injectVFPtr(const CXXRecordDecl *RD) {
   for (BaseOffsetsMapTy::iterator i = Bases.begin(), e = Bases.end();
        i != e; ++i)
     i->second += Offset;
-  updateAlignment(PointerInfo.Alignment);
 }
 
 void MicrosoftRecordLayoutBuilder::injectVPtrs(const CXXRecordDecl *RD) {
@@ -2512,6 +2505,7 @@ void MicrosoftRecordLayoutBuilder::injectVPtrs(const CXXRecordDecl *RD) {
     // the record.
     injectVBPtr(RD);
     injectVFPtr(RD);
+    Alignment = std::max(Alignment, PointerInfo.Alignment);
     return;
   }
   // In 64-bit mode, structs with RequiredAlignment greater than 8 get special
@@ -2521,7 +2515,7 @@ void MicrosoftRecordLayoutBuilder::injectVPtrs(const CXXRecordDecl *RD) {
   FieldOffsets.clear();
   Bases.clear();
   Size = CharUnits::Zero();
-  updateAlignment(PointerInfo.Alignment);
+  Alignment = std::max(Alignment, PointerInfo.Alignment);
   if (HasOwnVFPtr)
     Size = PointerInfo.Size;
   layoutNonVirtualBases(RD);
@@ -2619,7 +2613,6 @@ void MicrosoftRecordLayoutBuilder::layoutVirtualBases(const CXXRecordDecl *RD) {
     VBases.insert(std::make_pair(BaseDecl,
         ASTRecordLayout::VBaseInfo(BaseOffset, HasVtordisp)));
     Size = BaseOffset + BaseLayout.getDataSize();
-    updateAlignment(Info.Alignment);
     PreviousBaseLayout = &BaseLayout;
   }
 }

diff --git a/test/Layout/ms-x86-pack-and-align.cpp b/test/Layout/ms-x86-pack-and-align.cpp
index fba485f5a22a6af4fd0f3c7a4e0d6bde3556e92b..5c2710e6d6232844c31985afc53e4fa4ac2d8b3b 100644 (file)
--- a/test/Layout/ms-x86-pack-and-align.cpp
+++ b/test/Layout/ms-x86-pack-and-align.cpp
@@ -74,7 +74,7 @@ struct Y : A, B {
 // CHECK-NEXT:   12 |   char a
 // CHECK-NEXT:   14 |   int b
 // CHECK-NEXT:      | [sizeof=20, align=4
-// CHECK-NEXT:      |  nvsize=20, nvalign=4]
+// CHECK-NEXT:      |  nvsize=18, nvalign=4]
 // CHECK-X64: *** Dumping AST Record Layout
 // CHECK-X64: *** Dumping AST Record Layout
 // CHECK-X64-NEXT:    0 | struct Y
@@ -85,7 +85,7 @@ struct Y : A, B {
 // CHECK-X64-NEXT:   12 |   char a
 // CHECK-X64-NEXT:   14 |   int b
 // CHECK-X64-NEXT:      | [sizeof=20, align=4
-// CHECK-X64-NEXT:      |  nvsize=20, nvalign=4]
+// CHECK-X64-NEXT:      |  nvsize=18, nvalign=4]
 
 struct Z : virtual B {
        char a;
@@ -291,6 +291,36 @@ struct YF {
 // CHECK-X64:           | [sizeof=5, align=1
 // CHECK-X64-NEXT:      |  nvsize=5, nvalign=1]
 
+#pragma pack(16)
+struct __declspec(align(16)) D0 { char a; };
+#pragma pack(1)
+struct D1 : public D0 { char a; };
+#pragma pack(16)
+struct D2 : D1 { char a; };
+
+// CHECK: *** Dumping AST Record Layout
+// CHECK: *** Dumping AST Record Layout
+// CHECK: *** Dumping AST Record Layout
+// CHECK-NEXT:    0 | struct D2
+// CHECK-NEXT:    0 |   struct D1 (base)
+// CHECK-NEXT:    0 |     struct D0 (base)
+// CHECK-NEXT:    0 |       char a
+// CHECK-NEXT:    1 |     char a
+// CHECK-NEXT:    2 |   char a
+// CHECK-NEXT:      | [sizeof=16, align=16
+// CHECK-NEXT:      |  nvsize=16, nvalign=16]
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64: *** Dumping AST Record Layout
+// CHECK-X64-NEXT:    0 | struct D2
+// CHECK-X64-NEXT:    0 |   struct D1 (base)
+// CHECK-X64-NEXT:    0 |     struct D0 (base)
+// CHECK-X64-NEXT:    0 |       char a
+// CHECK-X64-NEXT:    1 |     char a
+// CHECK-X64-NEXT:    2 |   char a
+// CHECK-X64-NEXT:      | [sizeof=16, align=16
+// CHECK-X64-NEXT:      |  nvsize=16, nvalign=16]
+
 int a[
 sizeof(X)+
 sizeof(Y)+
@@ -303,4 +333,6 @@ sizeof(YC)+
 sizeof(YD)+
 sizeof(YE)+
 sizeof(YF)+
+sizeof(YF)+
+sizeof(D2)+
 0];