r206908 - Fix PR19487, PR19505 and PR19506 -- redundant vtordisp thunks when the final overrider is present in both a vbase and nvbase

[Timur Iskhodzhanov]

Author: timurrrr
Date: Tue Apr 22 12:32:02 2014
New Revision: 206908

URL: http://llvm.org/viewvc/llvm-project?rev=206908&view=rev
Log:
Fix PR19487, PR19505 and PR19506 -- redundant vtordisp thunks when the final overrider is present in both a vbase and nvbase

Reviewed at http://reviews.llvm.org/D3449

Modified:
    cfe/trunk/lib/AST/VTableBuilder.cpp
    cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance-vtordisps.cpp

Modified: cfe/trunk/lib/AST/VTableBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/VTableBuilder.cpp?rev=206908&r1=206907&r2=206908&view=diff
==============================================================================
--- cfe/trunk/lib/AST/VTableBuilder.cpp (original)
+++ cfe/trunk/lib/AST/VTableBuilder.cpp Tue Apr 22 12:32:02 2014
@@ -64,10 +64,14 @@ public:
     /// Method - The method decl of the overrider.
     const CXXMethodDecl *Method;
 
+    /// VirtualBase - The virtual base class subobject of this overridder.
+    /// Note that this records the closest derived virtual base class subobject.
+    const CXXRecordDecl *VirtualBase;
+
     /// Offset - the base offset of the overrider's parent in the layout class.
     CharUnits Offset;
     
-    OverriderInfo() : Method(0), Offset(CharUnits::Zero()) { }
+    OverriderInfo() : Method(0), VirtualBase(0), Offset(CharUnits::Zero()) { }
   };
 
 private:
@@ -201,6 +205,7 @@ FinalOverriders::FinalOverriders(const C
       
       Overrider.Offset = OverriderOffset;
       Overrider.Method = Method.Method;
+      Overrider.VirtualBase = Method.InVirtualSubobject;
     }
   }
 
@@ -2716,20 +2721,10 @@ void VFTableBuilder::CalculateVtordispAd
       VBaseMap.find(WhichVFPtr.getVBaseWithVPtr());
   assert(VBaseMapEntry != VBaseMap.end());
 
-  // If there's no vtordisp, we don't need any vtordisp adjustment.
-  if (!VBaseMapEntry->second.hasVtorDisp())
-    return;
-
-  const CXXRecordDecl *OverriderRD = Overrider.Method->getParent();
-  const CXXRecordDecl *OverriderVBase = 0;
-  if (OverriderRD != MostDerivedClass) {
-    OverriderVBase =
-        ComputeBaseOffset(Context, OverriderRD, MostDerivedClass).VirtualBase;
-  }
-
-  // If the final overrider is defined in the same vbase as the initial
-  // declaration, we don't need a vtordisp thunk at all.
-  if (OverriderVBase == WhichVFPtr.getVBaseWithVPtr())
+  // If there's no vtordisp or the final overrider is defined in the same vbase
+  // as the initial declaration, we don't need any vtordisp adjustment.
+  if (!VBaseMapEntry->second.hasVtorDisp() ||
+      Overrider.VirtualBase == WhichVFPtr.getVBaseWithVPtr())
     return;
 
   // OK, now we know we need to use a vtordisp thunk.
@@ -2740,7 +2735,8 @@ void VFTableBuilder::CalculateVtordispAd
 
   // A simple vtordisp thunk will suffice if the final overrider is defined
   // in either the most derived class or its non-virtual base.
-  if (OverriderRD == MostDerivedClass || !OverriderVBase)
+  if (Overrider.Method->getParent() == MostDerivedClass ||
+      !Overrider.VirtualBase)
     return;
 
   // Otherwise, we need to do use the dynamic offset of the final overrider
@@ -2750,7 +2746,7 @@ void VFTableBuilder::CalculateVtordispAd
        MostDerivedClassLayout.getVBPtrOffset()).getQuantity();
   TA.Virtual.Microsoft.VBOffsetOffset =
       Context.getTypeSizeInChars(Context.IntTy).getQuantity() *
-      VTables.getVBTableIndex(MostDerivedClass, OverriderVBase);
+      VTables.getVBTableIndex(MostDerivedClass, Overrider.VirtualBase);
 
   TA.NonVirtual = (ThisOffset - Overrider.Offset).getQuantity();
 }

Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance-vtordisps.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance-vtordisps.cpp?rev=206908&r1=206907&r2=206908&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance-vtordisps.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-virtual-inheritance-vtordisps.cpp Tue Apr 22 12:32:02 2014
@@ -480,3 +480,58 @@ struct C : virtual B {
 
 C c;
 }
+
+namespace pr19505 {
+struct A {
+  virtual void f();
+  virtual void z();
+};
+
+struct B : A {
+  virtual void f();
+};
+
+struct C : A, B {
+  virtual void g();
+};
+
+struct X : B, virtual C {
+  X() {}
+  virtual void g();
+
+  // CHECK-LABEL: VFTable for 'pr19505::A' in 'pr19505::B' in 'pr19505::C' in 'pr19505::X' (2 entries).
+  // CHECK-NEXT:   0 | void pr19505::B::f()
+  // CHECK-NEXT:   1 | void pr19505::A::z()
+
+  // MANGLING-DAG: @"\01??_7X at pr19505@@6BB at 1@@" = {{.*}}@"\01?f at B@pr19505@@UAEXXZ"
+} x;
+
+void build_vftable(X *obj) { obj->g(); }
+}
+
+namespace pr19506 {
+struct A {
+  virtual void f();
+  virtual void g();
+};
+
+struct B : A {
+  virtual void f();
+};
+
+struct C : B {};
+
+struct X : C, virtual B {
+  virtual void g();
+  X() {}
+
+  // CHECK-LABEL: VFTable for 'pr19506::A' in 'pr19506::B' in 'pr19506::X' (2 entries).
+  // CHECK-NEXT:   0 | void pr19506::B::f()
+  // CHECK-NEXT:   1 | void pr19506::X::g()
+  // CHECK-NEXT:       [this adjustment: vtordisp at -4, -12 non-virtual]
+
+  // MANGLING-DAG: @"\01??_7X at pr19506@@6BB at 1@@" = {{.*}}@"\01?f at B@pr19506@@UAEXXZ"
+} x;
+
+void build_vftable(X *obj) { obj->g(); }
+}