Improve support for non-virtual 'this' pointer adjustments. With this, it should...

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

diff --git a/lib/CodeGen/CGVtable.cpp b/lib/CodeGen/CGVtable.cpp
index 15b9a924df633626ead8f35963b4e748990644b0..3651d1c214d0d8d564517ae482ebeba21f3b0f7b 100644 (file)
--- a/lib/CodeGen/CGVtable.cpp
+++ b/lib/CodeGen/CGVtable.cpp
@@ -136,9 +136,9 @@ private:
                           SubobjectOffsetsMapTy &Offsets);
   
   /// ComputeThisAdjustmentBaseOffset - Compute the base offset for adjusting
-  /// the 'this' pointer from BaseRD to DerivedRD.
-  BaseOffset ComputeThisAdjustmentBaseOffset(const CXXRecordDecl *BaseRD,
-                                             const CXXRecordDecl *DerivedRD);
+  /// the 'this' pointer from the base subobject to the derived subobject.
+  BaseOffset ComputeThisAdjustmentBaseOffset(BaseSubobject BaseSubobject,
+                                             BaseSubobject Derived);
                                              
   static void MergeSubobjectOffsets(const SubobjectOffsetsMapTy &NewOffsets,
                                     SubobjectOffsetsMapTy &Offsets);
@@ -229,22 +229,11 @@ void FinalOverriders::AddOverriders(BaseSubobject Base,
   }
 }
 
-static FinalOverriders::BaseOffset 
-ComputeBaseOffset(ASTContext &Context, const CXXRecordDecl *BaseRD,
-                  const CXXRecordDecl *DerivedRD) {
-  CXXBasePaths Paths(/*FindAmbiguities=*/false,
-                     /*RecordPaths=*/true, /*DetectVirtual=*/false);
-  
-  if (!const_cast<CXXRecordDecl *>(DerivedRD)->
-      isDerivedFrom(const_cast<CXXRecordDecl *>(BaseRD), Paths)) {
-    assert(false && "Class must be derived from the passed in base class!");
-    return FinalOverriders::BaseOffset();
-  }
-
+static FinalOverriders::BaseOffset
+ComputeBaseOffset(ASTContext &Context, const CXXRecordDecl *DerivedRD,
+                  const CXXBasePath &Path) {
   int64_t NonVirtualOffset = 0;
 
-  const CXXBasePath &Path = Paths.front();
-  
   unsigned NonVirtualStart = 0;
   const CXXRecordDecl *VirtualBase = 0;
   
@@ -280,6 +269,22 @@ ComputeBaseOffset(ASTContext &Context, const CXXRecordDecl *BaseRD,
   // CharUnits.
   return FinalOverriders::BaseOffset(DerivedRD, VirtualBase, 
                                      NonVirtualOffset / 8);
+  
+}
+
+static FinalOverriders::BaseOffset 
+ComputeBaseOffset(ASTContext &Context, const CXXRecordDecl *BaseRD,
+                  const CXXRecordDecl *DerivedRD) {
+  CXXBasePaths Paths(/*FindAmbiguities=*/false,
+                     /*RecordPaths=*/true, /*DetectVirtual=*/false);
+  
+  if (!const_cast<CXXRecordDecl *>(DerivedRD)->
+      isDerivedFrom(const_cast<CXXRecordDecl *>(BaseRD), Paths)) {
+    assert(false && "Class must be derived from the passed in base class!");
+    return FinalOverriders::BaseOffset();
+  }
+
+  return ComputeBaseOffset(Context, DerivedRD, Paths.front());
 }
 
 static FinalOverriders::BaseOffset
@@ -337,9 +342,12 @@ ComputeReturnAdjustmentBaseOffset(ASTContext &Context,
 }
 
 FinalOverriders::BaseOffset
-FinalOverriders::ComputeThisAdjustmentBaseOffset(const CXXRecordDecl *BaseRD,
-                                               const CXXRecordDecl *DerivedRD) {
-  CXXBasePaths Paths(/*FindAmbiguities=*/false,
+FinalOverriders::ComputeThisAdjustmentBaseOffset(BaseSubobject Base,
+                                                 BaseSubobject Derived) {
+  const CXXRecordDecl *BaseRD = Base.getBase();
+  const CXXRecordDecl *DerivedRD = Derived.getBase();
+  
+  CXXBasePaths Paths(/*FindAmbiguities=*/true,
                      /*RecordPaths=*/true, /*DetectVirtual=*/true);
 
   if (!const_cast<CXXRecordDecl *>(DerivedRD)->
@@ -350,10 +358,15 @@ FinalOverriders::ComputeThisAdjustmentBaseOffset(const CXXRecordDecl *BaseRD,
 
   assert(!Paths.getDetectedVirtual() && "FIXME: Handle virtual bases!");
 
-  // FIXME: We need to go through all paths here, just not the first one.
-  BaseOffset Offset = ComputeBaseOffset(Context, BaseRD, DerivedRD);
+  BaseOffset Offset;
+  
+  // FIXME: This is not going to be enough with virtual bases.
+  // FIXME: We should not use / 8 here.
+  int64_t DerivedToBaseOffset = 
+    (Base.getBaseOffset() - Derived.getBaseOffset()) / 8;
+  
+  Offset.NonVirtualOffset = -DerivedToBaseOffset;
 
-  Offset.NonVirtualOffset = -Offset.NonVirtualOffset;
   return Offset;
 }
   
@@ -380,8 +393,9 @@ void FinalOverriders::PropagateOverrider(const CXXMethodDecl *OldMD,
     for (unsigned I = 0, E = OffsetVector.size(); I != E; ++I) {
       uint64_t Offset = OffsetVector[I];
 
+      BaseSubobject OverriddenSubobject = BaseSubobject(OverriddenRD, Offset);
       BaseSubobjectMethodPairTy SubobjectAndMethod =
-        std::make_pair(BaseSubobject(OverriddenRD, Offset), OverriddenMD);
+        std::make_pair(OverriddenSubobject, OverriddenMD);
       
       OverriderInfo &Overrider = OverridersMap[SubobjectAndMethod];
 
@@ -402,8 +416,8 @@ void FinalOverriders::PropagateOverrider(const CXXMethodDecl *OldMD,
         // Check if we need a 'this' adjustment base offset as well.
         if (Offset != NewBase.getBaseOffset()) {
           BaseOffset ThisBaseOffset =
-            ComputeThisAdjustmentBaseOffset(OverriddenMD->getParent(),
-                                            NewMD->getParent());
+            ComputeThisAdjustmentBaseOffset(OverriddenSubobject,
+                                            NewBase);
           assert(!ThisBaseOffset.isEmpty() && 
                  "Should not get an empty 'this' adjustment!");
           
@@ -921,7 +935,7 @@ void VtableBuilder::layoutVtable(BaseSubobject Base) {
   
   // First, add the offset to top.
   // FIXME: This is not going to be right for construction vtables.
-  // FIXME: We should not use -8 here.
+  // FIXME: We should not use / 8 here.
   int64_t OffsetToTop = -(int64_t)Base.getBaseOffset() / 8;
   Components.push_back(VtableComponent::MakeOffsetToTop(OffsetToTop));
   

diff --git a/test/CodeGenCXX/vtable-layout.cpp b/test/CodeGenCXX/vtable-layout.cpp
index 181ebc5c3e0cd041599131758235632067c022be..e151de14ed9be49c12f856ca37770ac1aa3e1fb2 100644 (file)
--- a/test/CodeGenCXX/vtable-layout.cpp
+++ b/test/CodeGenCXX/vtable-layout.cpp
@@ -279,4 +279,45 @@ struct C : A1, A2 {
 };
 void C::f() { }
 
-}
\ No newline at end of file
+}
+
+namespace Test7 {
+
+// Test that the D::f overrider for A::f have different 'this' pointer
+// adjustments in the two A base subobjects.
+
+struct A {
+  virtual void f();
+  int a;
+};
+
+struct B1 : A { };
+struct B2 : A { };
+
+struct C { virtual void c(); };
+
+// CHECK:     Vtable for 'Test7::D' (10 entries).
+// CHECK-NEXT:   0 | offset_to_top (0)
+// CHECK-NEXT:   1 | Test7::D RTTI
+// CHECK-NEXT:       -- (Test7::C, 0) vtable address --
+// CHECK-NEXT:       -- (Test7::D, 0) vtable address --
+// CHECK-NEXT:   2 | void Test7::C::c()
+// CHECK-NEXT:   3 | void Test7::D::f()
+// CHECK-NEXT:   4 | offset_to_top (-8)
+// CHECK-NEXT:   5 | Test7::D RTTI
+// CHECK-NEXT:       -- (Test7::A, 8) vtable address --
+// CHECK-NEXT:       -- (Test7::B1, 8) vtable address --
+// CHECK-NEXT:   6 | void Test7::D::f()
+// CHECK-NEXT:       [this adjustment: -8 non-virtual]
+// CHECK-NEXT:   7 | offset_to_top (-24)
+// CHECK-NEXT:   8 | Test7::D RTTI
+// CHECK-NEXT:       -- (Test7::A, 24) vtable address --
+// CHECK-NEXT:       -- (Test7::B2, 24) vtable address --
+// CHECK-NEXT:   9 | void Test7::D::f()
+// CHECK-NEXT:       [this adjustment: -24 non-virtual]
+struct D : C, B1, B2 {
+  virtual void f();
+};
+void D::f() { }
+
+}