[cfe-commits] r96881 - in /cfe/trunk: lib/CodeGen/CGVtable.cpp test/CodeGenCXX/vtable-layout.cpp

[Anders Carlsson]

Author: andersca
Date: Mon Feb 22 21:48:14 2010
New Revision: 96881

URL: http://llvm.org/viewvc/llvm-project?rev=96881&view=rev
Log:
More fixes. Don't try to emit a virtual base vtable if the virtual base in question is a primary virtual base of some other base.

Modified:
    cfe/trunk/lib/CodeGen/CGVtable.cpp
    cfe/trunk/test/CodeGenCXX/vtable-layout.cpp

Modified: cfe/trunk/lib/CodeGen/CGVtable.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVtable.cpp?rev=96881&r1=96880&r2=96881&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGVtable.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGVtable.cpp Mon Feb 22 21:48:14 2010
@@ -961,6 +961,10 @@
   
   typedef llvm::SmallPtrSet<const CXXRecordDecl *, 4> VisitedVirtualBasesSetTy;
 
+  /// PrimaryVirtualBases - All known virtual bases who is a primary base of
+  /// some other base.
+  VisitedVirtualBasesSetTy PrimaryVirtualBases;
+
   /// AddVCallAndVBaseOffsets - Add vcall offsets and vbase offsets for the
   /// given base subobject.
   void AddVCallAndVBaseOffsets(BaseSubobject Base, bool BaseIsVirtual,
@@ -1107,7 +1111,7 @@
   AddVBaseOffsets(Base.getBase(), OffsetToTop, VBases);
 
   // We only want to add vcall offsets for virtual bases.
-  if (BaseIsVirtual && OffsetToTop != 0)
+  if (BaseIsVirtual)
     AddVCallOffsets(Base, Base.getBaseOffset());
 }
 
@@ -1117,9 +1121,24 @@
 
   // Handle the primary base first.
   if (const CXXRecordDecl *PrimaryBase = Layout.getPrimaryBase()) {
+    uint64_t PrimaryBaseOffset;
+    
     // Get the base offset of the primary base.
-    uint64_t PrimaryBaseOffset = Base.getBaseOffset() + 
-      Layout.getBaseClassOffset(PrimaryBase);
+    if (Layout.getPrimaryBaseWasVirtual()) {
+      assert(Layout.getVBaseClassOffset(PrimaryBase) == 0 &&
+             "Primary vbase should have a zero offset!");
+      
+      const ASTRecordLayout &MostDerivedClassLayout =
+        Context.getASTRecordLayout(MostDerivedClass);
+      
+      PrimaryBaseOffset = 
+        MostDerivedClassLayout.getVBaseClassOffset(PrimaryBase);
+    } else {
+      assert(Layout.getBaseClassOffset(PrimaryBase) == 0 &&
+             "Primary base should have a zero offset!");
+
+      PrimaryBaseOffset = Base.getBaseOffset();
+    }
     
     AddVCallOffsets(BaseSubobject(PrimaryBase, PrimaryBaseOffset),
                     VBaseOffset);
@@ -1245,13 +1264,26 @@
   const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD);
 
   if (const CXXRecordDecl *PrimaryBase = Layout.getPrimaryBase()) {
-    if (Layout.getPrimaryBaseWasVirtual())
-      assert(false && "FIXME: Handle vbases here.");
-    else
+    uint64_t BaseOffset;
+    if (Layout.getPrimaryBaseWasVirtual()) {
+      assert(Layout.getVBaseClassOffset(PrimaryBase) == 0 &&
+             "Primary vbase should have a zero offset!");
+      
+      const ASTRecordLayout &MostDerivedClassLayout =
+        Context.getASTRecordLayout(MostDerivedClass);
+      
+      BaseOffset = MostDerivedClassLayout.getVBaseClassOffset(PrimaryBase);
+      
+      // Keep track of this primary virtual base.
+      PrimaryVirtualBases.insert(PrimaryBase);
+    } else {
       assert(Layout.getBaseClassOffset(PrimaryBase) == 0 &&
              "Primary base should have a zero offset!");
+
+      BaseOffset = Base.getBaseOffset();
+    }
     
-    AddMethods(BaseSubobject(PrimaryBase, Base.getBaseOffset()), PrimaryBases);
+    AddMethods(BaseSubobject(PrimaryBase, BaseOffset), PrimaryBases);
     
     if (!PrimaryBases.insert(PrimaryBase))
       assert(false && "Found a duplicate primary base!");
@@ -1409,10 +1441,11 @@
     const CXXRecordDecl *BaseDecl = 
       cast<CXXRecordDecl>(I->getType()->getAs<RecordType>()->getDecl());
 
-    // Check if this base needs a vtable. (If it's virtual, and we haven't
-    // visited it before).
+    // Check if this base needs a vtable. (If it's virtual, not a primary base
+    // of some other class, and we haven't visited it before).
     if (I->isVirtual() && BaseDecl->isDynamicClass() && 
-        BaseDecl != PrimaryBase && VBases.insert(BaseDecl)) {
+        BaseDecl != PrimaryBase && !PrimaryVirtualBases.count(BaseDecl) && 
+        VBases.insert(BaseDecl)) {
       const ASTRecordLayout &MostDerivedClassLayout =
         Context.getASTRecordLayout(MostDerivedClass);
       uint64_t BaseOffset = 

Modified: cfe/trunk/test/CodeGenCXX/vtable-layout.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/vtable-layout.cpp?rev=96881&r1=96880&r2=96881&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/vtable-layout.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/vtable-layout.cpp Mon Feb 22 21:48:14 2010
@@ -481,3 +481,32 @@
 void B::f() { } 
 
 }
+
+namespace Test13 {
+
+// Test that we don't try to emit a vtable for 'A' twice.
+struct A {
+  virtual void f();
+};
+
+struct B : virtual A {
+  virtual void f();
+};
+
+// CHECK:      Vtable for 'Test13::C' (6 entries).
+// CHECK-NEXT:    0 | vbase_offset (0)
+// CHECK-NEXT:    1 | vbase_offset (0)
+// CHECK-NEXT:    2 | vcall_offset (0)
+// CHECK-NEXT:    3 | offset_to_top (0)
+// CHECK-NEXT:    4 | Test13::C RTTI
+// CHECK-NEXT:        -- (Test13::A, 0) vtable address --
+// CHECK-NEXT:        -- (Test13::B, 0) vtable address --
+// CHECK-NEXT:        -- (Test13::C, 0) vtable address --
+// CHECK-NEXT:    5 | void Test13::C::f()
+struct C : virtual B, virtual A {
+  virtual void f();
+};
+void C::f() { }
+
+}
+