<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Dec 10, 2013 at 6:21 PM, Warren Hunt <span dir="ltr"><<a href="mailto:whunt@google.com" target="_blank">whunt@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: whunt<br>
Date: Tue Dec 10 20:21:03 2013<br>
New Revision: 196997<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=196997&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=196997&view=rev</a><br>
Log:<br>
[ms-abi] Makes Virtual Base Alignment Look at All Virtual Bases<br>
<br>
Prior to this patch, the alignment imposed by virtual bases only<br>
included direct virtual bases.  This patch fixes it to look at all<br>
virtual bases.<br>
<br>
<br>
Modified:<br>
    cfe/trunk/lib/AST/RecordLayoutBuilder.cpp<br>
    cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp<br>
<br>
Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=196997&r1=196996&r2=196997&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=196997&r1=196996&r2=196997&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)<br>
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Tue Dec 10 20:21:03 2013<br>
@@ -2125,8 +2125,6 @@ public:<br>
   /// \brief The alignment of the non-virtual portion of the record layout<br>
   /// Only used for C++ layouts.<br>
   CharUnits NonVirtualAlignment;<br>
-  /// \brief The additional alignment imposed by the virtual bases.<br>
-  CharUnits VirtualAlignment;<br>
   /// \brief The primary base class (if one exists).<br>
   const CXXRecordDecl *PrimaryBase;<br>
   /// \brief The class we share our vb-pointer with.<br>
@@ -2265,7 +2263,6 @@ MicrosoftRecordLayoutBuilder::initialize<br>
   HasExtendableVFPtr = false;<br>
   SharedVBPtrBase = 0;<br>
   PrimaryBase = 0;<br>
-  VirtualAlignment = CharUnits::One();<br>
   LeadsWithZeroSizedBase = false;<br>
<br>
   // If the record has a dynamic base class, attempt to choose a primary base<br>
@@ -2285,7 +2282,6 @@ MicrosoftRecordLayoutBuilder::initialize<br>
       HasZeroSizedSubObject = true;<br>
     // Handle virtual bases.<br>
     if (i->isVirtual()) {<br>
-      VirtualAlignment = std::max(VirtualAlignment, getBaseAlignment(Layout));<br>
       HasVBPtr = true;<br>
       continue;<br>
     }<br>
@@ -2555,7 +2551,14 @@ void MicrosoftRecordLayoutBuilder::layou<br>
   if (!HasVBPtr)<br>
     return;<br>
<br>
-  updateAlignment(VirtualAlignment);<br>
+  for (CXXRecordDecl::base_class_const_iterator i = RD->vbases_begin(),<br>
+                                                e = RD->vbases_end();<br>
+       i != e; ++i) {<br></blockquote><div><br></div><div>I'd make 'i' and 'e' capital seeing as how they are actually class types.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

+    const CXXRecordDecl *BaseDecl =<br>
+        cast<CXXRecordDecl>(i->getType()->getAs<RecordType>()->getDecl());<br></blockquote><div><br></div><div>I think this could be I->getType->getAsCXXRecordDecl()->getDecl()</div><div> </div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
+    const ASTRecordLayout &Layout = Context.getASTRecordLayout(BaseDecl);<br>
+    updateAlignment(getBaseAlignment(Layout));<br>
+  }<br>
   PreviousBaseLayout = 0;<br>
<br>
   // Zero-sized v-bases obey the alignment attribute so apply it here.  The<br>
<br>
Modified: cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp?rev=196997&r1=196996&r2=196997&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp?rev=196997&r1=196996&r2=196997&view=diff</a><br>

==============================================================================<br>
--- cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp (original)<br>
+++ cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp Tue Dec 10 20:21:03 2013<br>
@@ -114,7 +114,32 @@ struct Z : virtual B {<br>
<br>
 #pragma pack(pop)<br>
<br>
+struct A1 { long long a; };<br>
+#pragma pack(push, 1)<br>
+struct B1 : virtual A1 { char a; };<br>
+#pragma pack(pop)<br>
+struct C1 : B1 {};<br>
+// CHECK: *** Dumping AST Record Layout<br>
+// CHECK:    0 | struct C1<br>
+// CHECK:    0 |   struct B1 (base)<br>
+// CHECK:    0 |     (B1 vbtable pointer)<br>
+// CHECK:    4 |     char a<br>
+// CHECK:    8 |   struct A1 (virtual base)<br>
+// CHECK:    8 |     long long a<br>
+// CHECK:      | [sizeof=16, align=8<br>
+// CHECK:      |  nvsize=5, nvalign=1]<br>
+// CHECK-X64: *** Dumping AST Record Layout<br>
+// CHECK-X64:    0 | struct C1<br>
+// CHECK-X64:    0 |   struct B1 (base)<br>
+// CHECK-X64:    0 |     (B1 vbtable pointer)<br>
+// CHECK-X64:    8 |     char a<br>
+// CHECK-X64:   16 |   struct A1 (virtual base)<br>
+// CHECK-X64:   16 |     long long a<br>
+// CHECK-X64:      | [sizeof=24, align=8<br>
+// CHECK-X64:      |  nvsize=9, nvalign=1]<br>
+<br>
 int a[<br>
 sizeof(X)+<br>
 sizeof(Y)+<br>
-sizeof(Z)];<br>
+sizeof(Z)+<br>
+sizeof(C1)];<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div></div>