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

Anders Carlsson andersca at mac.com
Tue Mar 9 22:51:42 PST 2010


Author: andersca
Date: Wed Mar 10 00:51:42 2010
New Revision: 98139

URL: http://llvm.org/viewvc/llvm-project?rev=98139&view=rev
Log:
Don't accidentally mark some functions in construction vtables as unused. Also land the test for a previous checkin, now that it's correct.

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=98139&r1=98138&r2=98139&view=diff
==============================================================================
--- cfe/trunk/lib/CodeGen/CGVtable.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGVtable.cpp Wed Mar 10 00:51:42 2010
@@ -1210,15 +1210,17 @@
   ///   thunk. Since we require that a call to C::f() first convert to A*, 
   ///   C-in-D's copy of A's vtable is never referenced, so this is not 
   ///   necessary.
-  bool IsOverriderUsed(BaseSubobject Base, 
-                       BaseSubobject FirstBaseInPrimaryBaseChain,
-                       uint64_t OffsetInLayoutClass,
-                       FinalOverriders::OverriderInfo Overrider) const;
+  bool IsOverriderUsed(const CXXMethodDecl *Overrider,
+                       uint64_t BaseOffsetInLayoutClass,
+                       const CXXRecordDecl *FirstBaseInPrimaryBaseChain,
+                       uint64_t FirstBaseOffsetInLayoutClass) const;
+
   
   /// AddMethods - Add the methods of this base subobject and all its
   /// primary bases to the vtable components vector.
-  void AddMethods(BaseSubobject Base, BaseSubobject FirstBaseInPrimaryBaseChain,
-                  uint64_t OffsetInLayoutClass,
+  void AddMethods(BaseSubobject Base, uint64_t BaseOffsetInLayoutClass,                  
+                  const CXXRecordDecl *FirstBaseInPrimaryBaseChain,
+                  uint64_t FirstBaseOffsetInLayoutClass,
                   PrimaryBasesSetVectorTy &PrimaryBases);
 
   // LayoutVtable - Layout the vtable for the given base class, including its
@@ -1511,13 +1513,13 @@
 }
 
 bool 
-VtableBuilder::IsOverriderUsed(BaseSubobject Base, 
-                               BaseSubobject FirstBaseInPrimaryBaseChain,
-                               uint64_t OffsetInLayoutClass,
-                               FinalOverriders::OverriderInfo Overrider) const {
+VtableBuilder::IsOverriderUsed(const CXXMethodDecl *Overrider,
+                               uint64_t BaseOffsetInLayoutClass,
+                               const CXXRecordDecl *FirstBaseInPrimaryBaseChain,
+                               uint64_t FirstBaseOffsetInLayoutClass) const {
   // If the base and the first base in the primary base chain have the same
   // offsets, then this overrider will be used.
-  if (Base.getBaseOffset() == OffsetInLayoutClass)
+  if (BaseOffsetInLayoutClass == FirstBaseOffsetInLayoutClass)
    return true;
 
   // We know now that Base (or a direct or indirect base of it) is a primary
@@ -1526,12 +1528,12 @@
   
   // If the overrider is the first base in the primary base chain, we know
   // that the overrider will be used.
-  if (Overrider.Method->getParent() == FirstBaseInPrimaryBaseChain.getBase())
+  if (Overrider->getParent() == FirstBaseInPrimaryBaseChain)
     return true;
   
   VtableBuilder::PrimaryBasesSetVectorTy PrimaryBases;
 
-  const CXXRecordDecl *RD = FirstBaseInPrimaryBaseChain.getBase();
+  const CXXRecordDecl *RD = FirstBaseInPrimaryBaseChain;
   PrimaryBases.insert(RD);
 
   // Now traverse the base chain, starting with the first base, until we find
@@ -1553,7 +1555,7 @@
       // Now check if this is the primary base that is not a primary base in the
       // most derived class.
       if (LayoutClassLayout.getVBaseClassOffset(PrimaryBase) !=
-          OffsetInLayoutClass) {
+          FirstBaseOffsetInLayoutClass) {
         // We found it, stop walking the chain.
         break;
       }
@@ -1570,7 +1572,7 @@
   
   // If the final overrider is an override of one of the primary bases,
   // then we know that it will be used.
-  return OverridesIndirectMethodInBases(Overrider.Method, PrimaryBases);
+  return OverridesIndirectMethodInBases(Overrider, PrimaryBases);
 }
 
 /// FindNearestOverriddenMethod - Given a method, returns the overridden method
@@ -1595,17 +1597,17 @@
   return 0;
 }  
 
-void 
-VtableBuilder::AddMethods(BaseSubobject Base, 
-                          BaseSubobject FirstBaseInPrimaryBaseChain,
-                          uint64_t OffsetInLayoutClass,                          
+void
+VtableBuilder::AddMethods(BaseSubobject Base, uint64_t BaseOffsetInLayoutClass,                  
+                          const CXXRecordDecl *FirstBaseInPrimaryBaseChain,
+                          uint64_t FirstBaseOffsetInLayoutClass,
                           PrimaryBasesSetVectorTy &PrimaryBases) {
   const CXXRecordDecl *RD = Base.getBase();
-
   const ASTRecordLayout &Layout = Context.getASTRecordLayout(RD);
 
   if (const CXXRecordDecl *PrimaryBase = Layout.getPrimaryBase()) {
-    uint64_t BaseOffset;
+    uint64_t PrimaryBaseOffset;
+    uint64_t PrimaryBaseOffsetInLayoutClass;
     if (Layout.getPrimaryBaseWasVirtual()) {
       assert(Layout.getVBaseClassOffset(PrimaryBase) == 0 &&
              "Primary vbase should have a zero offset!");
@@ -1613,17 +1615,25 @@
       const ASTRecordLayout &MostDerivedClassLayout =
         Context.getASTRecordLayout(MostDerivedClass);
       
-      BaseOffset = MostDerivedClassLayout.getVBaseClassOffset(PrimaryBase);
+      PrimaryBaseOffset = 
+        MostDerivedClassLayout.getVBaseClassOffset(PrimaryBase);
+      
+      const ASTRecordLayout &LayoutClassLayout =
+        Context.getASTRecordLayout(LayoutClass);
+
+      PrimaryBaseOffsetInLayoutClass =
+        LayoutClassLayout.getVBaseClassOffset(PrimaryBase);
     } else {
       assert(Layout.getBaseClassOffset(PrimaryBase) == 0 &&
              "Primary base should have a zero offset!");
 
-      BaseOffset = Base.getBaseOffset();
+      PrimaryBaseOffset = Base.getBaseOffset();
+      PrimaryBaseOffsetInLayoutClass = BaseOffsetInLayoutClass;
     }
 
-    // FIXME: OffsetInLayoutClass is not right here.
-    AddMethods(BaseSubobject(PrimaryBase, BaseOffset), 
-               FirstBaseInPrimaryBaseChain, OffsetInLayoutClass, PrimaryBases);
+    AddMethods(BaseSubobject(PrimaryBase, PrimaryBaseOffset),
+               PrimaryBaseOffsetInLayoutClass, FirstBaseInPrimaryBaseChain, 
+               FirstBaseOffsetInLayoutClass, PrimaryBases);
     
     if (!PrimaryBases.insert(PrimaryBase))
       assert(false && "Found a duplicate primary base!");
@@ -1674,9 +1684,10 @@
     MethodInfoMap.insert(std::make_pair(MD, MethodInfo));
 
     // Check if this overrider is going to be used.
-    if (!IsOverriderUsed(Base, FirstBaseInPrimaryBaseChain, OffsetInLayoutClass,
-                         Overrider)) {
-      const CXXMethodDecl *OverriderMD = Overrider.Method;
+    const CXXMethodDecl *OverriderMD = Overrider.Method;
+    if (!IsOverriderUsed(OverriderMD, BaseOffsetInLayoutClass,
+                         FirstBaseInPrimaryBaseChain, 
+                         FirstBaseOffsetInLayoutClass)) {
       Components.push_back(VtableComponent::MakeUnusedFunction(OverriderMD));
       continue;
     }
@@ -1739,7 +1750,8 @@
 
   // Now go through all virtual member functions and add them.
   PrimaryBasesSetVectorTy PrimaryBases;
-  AddMethods(Base, Base, OffsetInLayoutClass, PrimaryBases);
+  AddMethods(Base, OffsetInLayoutClass, Base.getBase(), OffsetInLayoutClass, 
+             PrimaryBases);
 
   // Compute 'this' pointer adjustments.
   ComputeThisAdjustments();

Modified: cfe/trunk/test/CodeGenCXX/vtable-layout.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/vtable-layout.cpp?rev=98139&r1=98138&r2=98139&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/vtable-layout.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/vtable-layout.cpp Wed Mar 10 00:51:42 2010
@@ -1036,3 +1036,59 @@
 void C::g() { }
 
 }
+
+namespace Test26 {
+
+// Test that we generate the right number of entries in the C-in-D construction vtable, and that
+// we don't mark A::a as unused.
+
+struct A {
+  virtual void a();
+};
+
+struct B {
+  virtual void c();
+};
+
+struct C : virtual A {
+  virtual void b();
+};
+
+// CHECK:      Vtable for 'Test26::D' (15 entries).
+// CHECK-NEXT:    0 | vbase_offset (8)
+// CHECK-NEXT:    1 | vbase_offset (8)
+// CHECK-NEXT:    2 | vbase_offset (0)
+// CHECK-NEXT:    3 | vcall_offset (0)
+// CHECK-NEXT:    4 | offset_to_top (0)
+// CHECK-NEXT:    5 | Test26::D RTTI
+// CHECK-NEXT:        -- (Test26::B, 0) vtable address --
+// CHECK-NEXT:        -- (Test26::D, 0) vtable address --
+// CHECK-NEXT:    6 | void Test26::B::c()
+// CHECK-NEXT:    7 | void Test26::D::d()
+// CHECK-NEXT:    8 | vcall_offset (0)
+// CHECK-NEXT:    9 | vbase_offset (0)
+// CHECK-NEXT:   10 | vcall_offset (0)
+// CHECK-NEXT:   11 | offset_to_top (-8)
+// CHECK-NEXT:   12 | Test26::D RTTI
+// CHECK-NEXT:        -- (Test26::A, 8) vtable address --
+// CHECK-NEXT:        -- (Test26::C, 8) vtable address --
+// CHECK-NEXT:   13 | void Test26::A::a()
+// CHECK-NEXT:   14 | void Test26::C::b()
+
+// CHECK:      Construction vtable for ('Test26::C', 8) in 'Test26::D' (7 entries).
+// CHECK-NEXT:    0 | vcall_offset (0)
+// CHECK-NEXT:    1 | vbase_offset (0)
+// CHECK-NEXT:    2 | vcall_offset (0)
+// CHECK-NEXT:    3 | offset_to_top (0)
+// CHECK-NEXT:    4 | Test26::C RTTI
+// CHECK-NEXT:        -- (Test26::A, 8) vtable address --
+// CHECK-NEXT:        -- (Test26::C, 8) vtable address --
+// CHECK-NEXT:    5 | void Test26::A::a()
+// CHECK-NEXT:    6 | void Test26::C::b()
+class D : virtual B, virtual C {
+  virtual void d();
+};
+void D::d() { } 
+
+
+}
\ No newline at end of file





More information about the cfe-commits mailing list