Fix a bug where we were generating an unnecessary vtable for a virtual base that...

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@97303 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/CodeGen/CGVtable.cpp b/lib/CodeGen/CGVtable.cpp
index 3dc29ad9cafa3612af0f05a65499cda17a81be3c..3c8ddfaed5739aa9e5d35d06cb057bd673c4272f 100644 (file)
--- a/lib/CodeGen/CGVtable.cpp
+++ b/lib/CodeGen/CGVtable.cpp
@@ -1220,7 +1220,12 @@ private:
   /// LayoutSecondaryVtables - Layout the secondary vtables for the given base
   /// subobject.
   void LayoutSecondaryVtables(BaseSubobject Base);
-  
+
+  /// DeterminePrimaryVirtualBases - Determine the primary virtual bases in this
+  /// class hierarchy.
+  void DeterminePrimaryVirtualBases(const CXXRecordDecl *RD, 
+                                    VisitedVirtualBasesSetTy &VBases);
+
   /// LayoutVtablesForVirtualBases - Layout vtables for all virtual bases of the
   /// given base (excluding any primary bases).
   void LayoutVtablesForVirtualBases(const CXXRecordDecl *RD, 
@@ -1418,9 +1423,6 @@ VtableBuilder::AddMethods(BaseSubobject Base,
         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!");
@@ -1498,6 +1500,11 @@ void VtableBuilder::LayoutVtable() {
                                       /*BaseIsVirtual=*/false);
   
   VisitedVirtualBasesSetTy VBases;
+  
+  // Determine the primary virtual bases.
+  DeterminePrimaryVirtualBases(MostDerivedClass, VBases);
+  VBases.clear();
+  
   LayoutVtablesForVirtualBases(MostDerivedClass, VBases);
 }
   
@@ -1591,6 +1598,31 @@ void VtableBuilder::LayoutSecondaryVtables(BaseSubobject Base) {
   }
 }
 
+void
+VtableBuilder::DeterminePrimaryVirtualBases(const CXXRecordDecl *RD, 
+                                            VisitedVirtualBasesSetTy &VBases) {
+  const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD);
+  
+  // Check if this base has a primary base.
+  if (const CXXRecordDecl *PrimaryBase = Layout.getPrimaryBase()) {
+    // Check if it's virtual
+    if (Layout.getPrimaryBaseWasVirtual())
+      PrimaryVirtualBases.insert(PrimaryBase);
+  }
+
+  // Traverse bases, looking for more primary virtual bases.
+  for (CXXRecordDecl::base_class_const_iterator I = RD->bases_begin(),
+       E = RD->bases_end(); I != E; ++I) {
+    const CXXRecordDecl *BaseDecl = 
+      cast<CXXRecordDecl>(I->getType()->getAs<RecordType>()->getDecl());
+
+    if (I->isVirtual() && !VBases.insert(BaseDecl))
+      continue;
+
+    DeterminePrimaryVirtualBases(BaseDecl, VBases);
+  }
+}
+
 void
 VtableBuilder::LayoutVtablesForVirtualBases(const CXXRecordDecl *RD, 
                                             VisitedVirtualBasesSetTy &VBases) {
@@ -1598,9 +1630,6 @@ VtableBuilder::LayoutVtablesForVirtualBases(const CXXRecordDecl *RD,
   //   Then come the virtual base virtual tables, also in inheritance graph
   //   order, and again excluding primary bases (which share virtual tables with
   //   the classes for which they are primary).
-  const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD);
-  const CXXRecordDecl *PrimaryBase = Layout.getPrimaryBase();
-
   for (CXXRecordDecl::base_class_const_iterator I = RD->bases_begin(),
        E = RD->bases_end(); I != E; ++I) {
     const CXXRecordDecl *BaseDecl = 
@@ -1609,8 +1638,7 @@ VtableBuilder::LayoutVtablesForVirtualBases(const CXXRecordDecl *RD,
     // 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 && !PrimaryVirtualBases.count(BaseDecl) && 
-        VBases.insert(BaseDecl)) {
+        !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 c22222f9ce3d3b85f3ba60192c8cea83ce485e33..abf9d130970dd0210f5fc2211dfcf15a4ad7df3f 100644 (file)
--- a/test/CodeGenCXX/vtable-layout.cpp
+++ b/test/CodeGenCXX/vtable-layout.cpp
@@ -539,3 +539,33 @@ void D::f() { }
 
 }
 
+namespace Test15 {
+
+// Test that we don't emit an extra vtable for B since it's a primary base of C.
+struct A { virtual void a(); };
+struct B { virtual void b(); };
+
+struct C : virtual B { };
+
+// CHECK:      Vtable for 'Test15::D' (11 entries).
+// CHECK-NEXT:    0 | vbase_offset (8)
+// CHECK-NEXT:    1 | vbase_offset (8)
+// CHECK-NEXT:    2 | offset_to_top (0)
+// CHECK-NEXT:    3 | Test15::D RTTI
+// CHECK-NEXT:        -- (Test15::A, 0) vtable address --
+// CHECK-NEXT:        -- (Test15::D, 0) vtable address --
+// CHECK-NEXT:    4 | void Test15::A::a()
+// CHECK-NEXT:    5 | void Test15::D::f()
+// CHECK-NEXT:    6 | vbase_offset (0)
+// CHECK-NEXT:    7 | vcall_offset (0)
+// CHECK-NEXT:    8 | offset_to_top (-8)
+// CHECK-NEXT:    9 | Test15::D RTTI
+// CHECK-NEXT:        -- (Test15::B, 8) vtable address --
+// CHECK-NEXT:        -- (Test15::C, 8) vtable address --
+// CHECK-NEXT:   10 | void Test15::B::b()
+struct D : A, virtual B, virtual C { 
+  virtual void f();
+};
+void D::f() { } 
+
+}