From af280c03a515b39b4b2c8e221068b3ed6a692cab Mon Sep 17 00:00:00 2001
From: Anders Carlsson <andersca@mac.com>
Date: Tue, 23 Feb 2010 03:48:14 +0000
Subject: [PATCH] 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.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@96881 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/CodeGen/CGVtable.cpp          | 53 +++++++++++++++++++++++++------
 test/CodeGenCXX/vtable-layout.cpp | 29 +++++++++++++++++
 2 files changed, 72 insertions(+), 10 deletions(-)

diff --git a/lib/CodeGen/CGVtable.cpp b/lib/CodeGen/CGVtable.cpp
index 93cd42c9f5..9af81aba96 100644
--- a/lib/CodeGen/CGVtable.cpp
+++ b/lib/CodeGen/CGVtable.cpp
@@ -961,6 +961,10 @@ private:
   
   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 @@ VtableBuilder::AddVCallAndVBaseOffsets(BaseSubobject Base,
   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 @@ void VtableBuilder::AddVCallOffsets(BaseSubobject Base, uint64_t VBaseOffset) {
 
   // 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 @@ VtableBuilder::AddMethods(BaseSubobject Base, PrimaryBasesSetTy &PrimaryBases) {
   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 @@ VtableBuilder::LayoutVtablesForVirtualBases(const CXXRecordDecl *RD,
     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 = 
diff --git a/test/CodeGenCXX/vtable-layout.cpp b/test/CodeGenCXX/vtable-layout.cpp
index 237c9337f4..efd0d2f7bc 100644
--- a/test/CodeGenCXX/vtable-layout.cpp
+++ b/test/CodeGenCXX/vtable-layout.cpp
@@ -481,3 +481,32 @@ struct B : virtual A {
 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() { }
+
+}
+
-- 
2.40.0