[cfe-commits] r142325 - in /cfe/trunk: lib/AST/RecordLayoutBuilder.cpp lib/CodeGen/CGRecordLayoutBuilder.cpp test/Sema/ms_class_layout.cpp

Eli Friedman eli.friedman at gmail.com
Mon Oct 17 17:55:28 PDT 2011


Author: efriedma
Date: Mon Oct 17 19:55:28 2011
New Revision: 142325

URL: http://llvm.org/viewvc/llvm-project?rev=142325&view=rev
Log:
Rewrite parts of MS ABI C++ layout.  Based on work by r4start; I ended up doing this while I was trying to review his patch.


Modified:
    cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
    cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp
    cfe/trunk/test/Sema/ms_class_layout.cpp

Modified: cfe/trunk/lib/AST/RecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/RecordLayoutBuilder.cpp?rev=142325&r1=142324&r2=142325&view=diff
==============================================================================
--- cfe/trunk/lib/AST/RecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/AST/RecordLayoutBuilder.cpp Mon Oct 17 19:55:28 2011
@@ -669,7 +669,7 @@
 
   void SelectPrimaryVBase(const CXXRecordDecl *RD);
 
-  CharUnits GetVirtualPointersSize(const CXXRecordDecl *RD) const;
+  void EnsureVTablePointerAlignment();
 
   /// LayoutNonVirtualBases - Determines the primary base class (if any) and
   /// lays it out. Will then proceed to lay out all non-virtual base clasess.
@@ -681,6 +681,9 @@
   void AddPrimaryVirtualBaseOffsets(const BaseSubobjectInfo *Info,
                                     CharUnits Offset);
 
+  bool HasNewVirtualFunction(const CXXRecordDecl *RD) const;
+  bool BaseHasVFPtr(const CXXRecordDecl *RD) const;
+
   /// LayoutVirtualBases - Lays out all the virtual bases.
   void LayoutVirtualBases(const CXXRecordDecl *RD,
                           const CXXRecordDecl *MostDerivedClass);
@@ -730,12 +733,6 @@
   void setDataSize(CharUnits NewSize) { DataSize = Context.toBits(NewSize); }
   void setDataSize(uint64_t NewSize) { DataSize = NewSize; }
 
-  bool HasVBPtr(const CXXRecordDecl *RD) const;
-  bool HasNewVirtualFunction(const CXXRecordDecl *RD) const;
-
-  /// Add vbptr or vfptr to layout.
-  void AddVPointer();
-
   RecordLayoutBuilder(const RecordLayoutBuilder&);   // DO NOT IMPLEMENT
   void operator=(const RecordLayoutBuilder&); // DO NOT IMPLEMENT
 public:
@@ -778,11 +775,6 @@
   }
 }
 
-CharUnits
-RecordLayoutBuilder::GetVirtualPointersSize(const CXXRecordDecl *RD) const {
-  return Context.toCharUnitsFromBits(Context.getTargetInfo().getPointerWidth(0));
-}
-
 /// DeterminePrimaryBase - Determine the primary base of the given class.
 void RecordLayoutBuilder::DeterminePrimaryBase(const CXXRecordDecl *RD) {
   // If the class isn't dynamic, it won't have a primary base.
@@ -813,44 +805,30 @@
     }
   }
 
-  // Otherwise, it is the first nearly empty virtual base that is not an
-  // indirect primary virtual base class, if one exists.
+  // The Microsoft ABI doesn't have primary virtual bases.
+  if (Context.getTargetInfo().getCXXABI() == CXXABI_Microsoft) {
+    assert(!PrimaryBase && "Should not get here with a primary base!");
+    return;
+  }
+
+  // Under the Itanium ABI, if there is no non-virtual primary base class,
+  // try to compute the primary virtual base.  The primary virtual base is
+  // the first nearly empty virtual base that is not an indirect primary
+  // virtual base class, if one exists.
   if (RD->getNumVBases() != 0) {
     SelectPrimaryVBase(RD);
     if (PrimaryBase)
       return;
   }
 
-  // Otherwise, it is the first nearly empty virtual base that is not an
-  // indirect primary virtual base class, if one exists.
+  // Otherwise, it is the first indirect primary base class, if one exists.
   if (FirstNearlyEmptyVBase) {
     PrimaryBase = FirstNearlyEmptyVBase;
     PrimaryBaseIsVirtual = true;
     return;
   }
 
-  // Otherwise there is no primary base class.
   assert(!PrimaryBase && "Should not get here with a primary base!");
-
-  // Allocate the virtual table pointer at offset zero.
-  assert(DataSize == 0 && "Vtable pointer must be at offset zero!");
-
-  // Update the size.
-  setSize(getSize() + GetVirtualPointersSize(RD));
-  setDataSize(getSize());
-
-  CharUnits UnpackedBaseAlign = 
-    Context.toCharUnitsFromBits(Context.getTargetInfo().getPointerAlign(0));
-  CharUnits BaseAlign = (Packed) ? CharUnits::One() : UnpackedBaseAlign;
-
-  // The maximum field alignment overrides base align.
-  if (!MaxFieldAlignment.isZero()) {
-    BaseAlign = std::min(BaseAlign, MaxFieldAlignment);
-    UnpackedBaseAlign = std::min(UnpackedBaseAlign, MaxFieldAlignment);
-  }
-
-  // Update the alignment.
-  UpdateAlignment(BaseAlign, UnpackedBaseAlign);
 }
 
 BaseSubobjectInfo *
@@ -959,6 +937,25 @@
 }
 
 void
+RecordLayoutBuilder::EnsureVTablePointerAlignment() {
+  CharUnits UnpackedBaseAlign = 
+    Context.toCharUnitsFromBits(Context.getTargetInfo().getPointerAlign(0));
+  CharUnits BaseAlign = (Packed) ? CharUnits::One() : UnpackedBaseAlign;
+
+  // The maximum field alignment overrides base align.
+  if (!MaxFieldAlignment.isZero()) {
+    BaseAlign = std::min(BaseAlign, MaxFieldAlignment);
+    UnpackedBaseAlign = std::min(UnpackedBaseAlign, MaxFieldAlignment);
+  }
+
+  // Round up the current record size to pointer alignment.
+  setDataSize(getDataSize().RoundUpToAlignment(BaseAlign));
+
+  // Update the alignment.
+  UpdateAlignment(BaseAlign, UnpackedBaseAlign);
+}
+
+void
 RecordLayoutBuilder::LayoutNonVirtualBases(const CXXRecordDecl *RD) {
   // Then, determine the primary base class.
   DeterminePrimaryBase(RD);
@@ -992,6 +989,18 @@
     }
   }
 
+  if (Context.getTargetInfo().getCXXABI() != CXXABI_Microsoft &&
+      !PrimaryBase && RD->isDynamicClass()) {
+    // Under the Itanium ABI, a dynamic class without a primary base has a
+    // vtable pointer.  It is placed at offset 0.
+    assert(DataSize == 0 && "Vtable pointer must be at offset zero!");
+    EnsureVTablePointerAlignment();
+    CharUnits PtrWidth = 
+      Context.toCharUnitsFromBits(Context.getTargetInfo().getPointerWidth(0));
+    setSize(getSize() + PtrWidth);
+    setDataSize(getSize());
+  }
+
   // Now lay out the non-virtual bases.
   for (CXXRecordDecl::base_class_const_iterator I = RD->bases_begin(),
          E = RD->bases_end(); I != E; ++I) {
@@ -1013,6 +1022,30 @@
 
     LayoutNonVirtualBase(BaseInfo);
   }
+
+  if (Context.getTargetInfo().getCXXABI() == CXXABI_Microsoft) {
+    // Under the MS ABI, there are separate virtual function table and
+    // virtual base table pointers. A vfptr is necessary a if a class defines
+    // a virtual function which is not overriding a function from a base;
+    // a vbptr is necessary if a class has virtual bases. Either can come
+    // from a primary base, if it exists.  Otherwise, they are placed
+    // after any base classes.
+    CharUnits PtrWidth = 
+      Context.toCharUnitsFromBits(Context.getTargetInfo().getPointerWidth(0));
+    if (HasNewVirtualFunction(RD) &&
+        (!PrimaryBase || !BaseHasVFPtr(PrimaryBase))) {
+      EnsureVTablePointerAlignment();
+      setSize(getSize() + PtrWidth);
+      setDataSize(getSize());
+    }
+    if (RD->getNumVBases() &&
+        (!PrimaryBase || !PrimaryBase->getNumVBases())) {
+      EnsureVTablePointerAlignment();
+      VBPtrOffset = getSize();
+      setSize(getSize() + PtrWidth);
+      setDataSize(getSize());
+    }
+  }
 }
 
 void RecordLayoutBuilder::LayoutNonVirtualBase(const BaseSubobjectInfo *Base) {
@@ -1061,18 +1094,6 @@
   }
 }
 
-void RecordLayoutBuilder::AddVPointer() {
-  CharUnits PtrWidth = 
-    Context.toCharUnitsFromBits(Context.getTargetInfo().getPointerWidth(0));
-  setSize(getSize() + PtrWidth);
-  setDataSize(getSize());
-
-  if (Alignment > PtrWidth) {
-    setSize(getSize() + (Alignment - PtrWidth));
-    setDataSize(getSize());
-  }
-}
-
 bool 
 RecordLayoutBuilder::HasNewVirtualFunction(const CXXRecordDecl *RD) const {
   for (CXXRecordDecl::method_iterator method = RD->method_begin();
@@ -1086,18 +1107,15 @@
   return false;
 }
 
-bool
-RecordLayoutBuilder::HasVBPtr(const CXXRecordDecl *RD) const {
-  if (!RD->getNumBases())
-    return false;
-
-  for (CXXRecordDecl::base_class_const_iterator I = RD->bases_begin(),
-       E = RD->bases_end(); I != E; ++I) {
-    if (!I->isVirtual()) {
-      return false;
-    }
-  }
-  return true;
+bool 
+RecordLayoutBuilder::BaseHasVFPtr(const CXXRecordDecl *Base) const {
+  // FIXME: This function is inefficient.
+  if (HasNewVirtualFunction(Base))
+    return true;
+  const ASTRecordLayout &Layout = Context.getASTRecordLayout(Base);
+  if (const CXXRecordDecl *PrimaryBase = Layout.getPrimaryBase())
+    return BaseHasVFPtr(PrimaryBase);
+  return false;
 }
 
 void
@@ -1244,8 +1262,8 @@
 
 void RecordLayoutBuilder::Layout(const CXXRecordDecl *RD) {
   if (Context.getTargetInfo().getCXXABI() == CXXABI_Microsoft) {
-    MSLayout(RD);
-    return ;
+    //MSLayout(RD);
+    //return ;
   }
 
   InitializeLayout(RD);
@@ -1753,32 +1771,9 @@
 
 void RecordLayoutBuilder::MSLayout(const CXXRecordDecl *RD) {
 
-  bool IsVBPtrAddedToLayout = false;
-
   InitializeLayout(RD);
 
-  if (HasVBPtr(RD)) {
-    // If all bases are virtual and the class declares a new virtual function,
-    // MSVC builds a vfptr.
-    if (HasNewVirtualFunction(RD)) {
-      AddVPointer();
-    }
-
-    VBPtrOffset = getSize();
-    AddVPointer();
-    IsVBPtrAddedToLayout = true;
-
-    ComputeBaseSubobjectInfo(RD);
-  } else {
-    LayoutNonVirtualBases(RD);
-  }
-
-  if (RD->getNumVBases() &&
-      !IsVBPtrAddedToLayout) {
-    // Add vbptr.
-    VBPtrOffset = getSize();
-    AddVPointer();
-  }
+  LayoutNonVirtualBases(RD);
 
   LayoutFields(RD);
 

Modified: cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp?rev=142325&r1=142324&r2=142325&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGRecordLayoutBuilder.cpp Mon Oct 17 19:55:28 2011
@@ -669,7 +669,13 @@
 
   // Check if we need to add a vtable pointer.
   if (RD->isDynamicClass()) {
-    if (!PrimaryBase) {
+    if (PrimaryBase) {
+      if (!Layout.isPrimaryBaseVirtual())
+        LayoutNonVirtualBase(PrimaryBase, CharUnits::Zero());
+      else
+        LayoutVirtualBase(PrimaryBase, CharUnits::Zero());
+    } else if (Types.getContext().getTargetInfo().getCXXABI() !=
+               CXXABI_Microsoft) {
       llvm::Type *FunctionType =
         llvm::FunctionType::get(llvm::Type::getInt32Ty(Types.getLLVMContext()),
                                 /*isVarArg=*/true);
@@ -678,11 +684,7 @@
       assert(NextFieldOffset.isZero() &&
              "VTable pointer must come first!");
       AppendField(CharUnits::Zero(), VTableTy->getPointerTo());
-    } else {
-      if (!Layout.isPrimaryBaseVirtual())
-        LayoutNonVirtualBase(PrimaryBase, CharUnits::Zero());
-      else
-        LayoutVirtualBase(PrimaryBase, CharUnits::Zero());
+      
     }
   }
 

Modified: cfe/trunk/test/Sema/ms_class_layout.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/ms_class_layout.cpp?rev=142325&r1=142324&r2=142325&view=diff
==============================================================================
--- cfe/trunk/test/Sema/ms_class_layout.cpp (original)
+++ cfe/trunk/test/Sema/ms_class_layout.cpp Mon Oct 17 19:55:28 2011
@@ -53,9 +53,12 @@
 
 struct G
 {
-    virtual ~G(){}
-    int a;
-    double b;
+    int g_field;
+};
+
+struct H : public G, 
+           public virtual D
+{
 };
 
 #pragma pack(pop)
@@ -67,7 +70,7 @@
   C* c;
   c->foo();
   DerivedStruct* v;
-  G* g;
+  H* g;
   BaseStruct* u;
   return 0;
 }
@@ -174,3 +177,20 @@
 // CHECK: 96 |   int x
 // CHECK-NEXT: sizeof=104, dsize=104, align=8
 // CHECK-NEXT: nvsize=104, nvalign=8
+
+// CHECK:      0 | struct G
+// CHECK-NEXT: 0 |   int g_field
+// CHECK-NEXT: sizeof=4, dsize=4, align=4
+// CHECK-NEXT: nvsize=4, nvalign=4
+
+// FIXME: Dump should not be showing vfptr, and vbptr is in the wrong order.
+// CHECK:       0 | struct H
+// CHECK-NEXT:  0 |   (H vtable pointer)
+// CHECK-NEXT:  4 |   (H vbtable pointer)
+// CHECK-NEXT:  0 |   struct G (base)
+// CHECK-NEXT:  0 |     int g_field
+// CHECK-NEXT:  8 |   class D (virtual base)
+// CHECK-NEXT:  8 |     (D vtable pointer)
+// CHECK-NEXT: 16 |     double a
+// CHECK-NEXT: sizeof=24, dsize=24, align=8
+// CHECK-NEXT: nvsize=24, nvalign=8





More information about the cfe-commits mailing list