r196997 - [ms-abi] Makes Virtual Base Alignment Look at All Virtual Bases

[David Majnemer]

On Tue, Dec 10, 2013 at 6:21 PM, Warren Hunt <whunt at google.com> wrote:

> Author: whunt
> Date: Tue Dec 10 20:21:03 2013
> New Revision: 196997
>
> URL: http://llvm.org/viewvc/llvm-project?rev=196997&view=rev
> Log:
> [ms-abi] Makes Virtual Base Alignment Look at All Virtual Bases
>
> Prior to this patch, the alignment imposed by virtual bases only
> included direct virtual bases.  This patch fixes it to look at all
> virtual bases.
>
>
> Modified:
>     cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
>     cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp
>
> Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=196997&r1=196996&r2=196997&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
> +++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Tue Dec 10 20:21:03 2013
> @@ -2125,8 +2125,6 @@ public:
>    /// \brief The alignment of the non-virtual portion of the record layout
>    /// Only used for C++ layouts.
>    CharUnits NonVirtualAlignment;
> -  /// \brief The additional alignment imposed by the virtual bases.
> -  CharUnits VirtualAlignment;
>    /// \brief The primary base class (if one exists).
>    const CXXRecordDecl *PrimaryBase;
>    /// \brief The class we share our vb-pointer with.
> @@ -2265,7 +2263,6 @@ MicrosoftRecordLayoutBuilder::initialize
>    HasExtendableVFPtr = false;
>    SharedVBPtrBase = 0;
>    PrimaryBase = 0;
> -  VirtualAlignment = CharUnits::One();
>    LeadsWithZeroSizedBase = false;
>
>    // If the record has a dynamic base class, attempt to choose a primary
> base
> @@ -2285,7 +2282,6 @@ MicrosoftRecordLayoutBuilder::initialize
>        HasZeroSizedSubObject = true;
>      // Handle virtual bases.
>      if (i->isVirtual()) {
> -      VirtualAlignment = std::max(VirtualAlignment,
> getBaseAlignment(Layout));
>        HasVBPtr = true;
>        continue;
>      }
> @@ -2555,7 +2551,14 @@ void MicrosoftRecordLayoutBuilder::layou
>    if (!HasVBPtr)
>      return;
>
> -  updateAlignment(VirtualAlignment);
> +  for (CXXRecordDecl::base_class_const_iterator i = RD->vbases_begin(),
> +                                                e = RD->vbases_end();
> +       i != e; ++i) {
>

I'd make 'i' and 'e' capital seeing as how they are actually class types.


> +    const CXXRecordDecl *BaseDecl =
> +        cast<CXXRecordDecl>(i->getType()->getAs<RecordType>()->getDecl());
>

I think this could be I->getType->getAsCXXRecordDecl()->getDecl()


> +    const ASTRecordLayout &Layout = Context.getASTRecordLayout(BaseDecl);
> +    updateAlignment(getBaseAlignment(Layout));
> +  }
>    PreviousBaseLayout = 0;
>
>    // Zero-sized v-bases obey the alignment attribute so apply it here.
>  The
>
> Modified: cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp?rev=196997&r1=196996&r2=196997&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp (original)
> +++ cfe/trunk/test/Layout/ms-x86-pack-and-align.cpp Tue Dec 10 20:21:03
> 2013
> @@ -114,7 +114,32 @@ struct Z : virtual B {
>
>  #pragma pack(pop)
>
> +struct A1 { long long a; };
> +#pragma pack(push, 1)
> +struct B1 : virtual A1 { char a; };
> +#pragma pack(pop)
> +struct C1 : B1 {};
> +// CHECK: *** Dumping AST Record Layout
> +// CHECK:    0 | struct C1
> +// CHECK:    0 |   struct B1 (base)
> +// CHECK:    0 |     (B1 vbtable pointer)
> +// CHECK:    4 |     char a
> +// CHECK:    8 |   struct A1 (virtual base)
> +// CHECK:    8 |     long long a
> +// CHECK:      | [sizeof=16, align=8
> +// CHECK:      |  nvsize=5, nvalign=1]
> +// CHECK-X64: *** Dumping AST Record Layout
> +// CHECK-X64:    0 | struct C1
> +// CHECK-X64:    0 |   struct B1 (base)
> +// CHECK-X64:    0 |     (B1 vbtable pointer)
> +// CHECK-X64:    8 |     char a
> +// CHECK-X64:   16 |   struct A1 (virtual base)
> +// CHECK-X64:   16 |     long long a
> +// CHECK-X64:      | [sizeof=24, align=8
> +// CHECK-X64:      |  nvsize=9, nvalign=1]
> +
>  int a[
>  sizeof(X)+
>  sizeof(Y)+
> -sizeof(Z)];
> +sizeof(Z)+
> +sizeof(C1)];
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131210/3d8689d2/attachment.html>