r198080 - [ms-cxxabi] Emit fewer trivial return adjusting thunks

Reid Kleckner reid at kleckner.net
Fri Dec 27 11:44:00 PST 2013


Author: rnk
Date: Fri Dec 27 13:43:59 2013
New Revision: 198080

URL: http://llvm.org/viewvc/llvm-project?rev=198080&view=rev
Log:
[ms-cxxabi] Emit fewer trivial return adjusting thunks

Most importantly, this makes our vtable layout match MSVC's.  Previously
we would emit a return adjusting thunk whenever the return types
differed, even if the adjustment would have been trivial.

MSVC does emit some trivial return adjusting thunks, but only if there
was already an overridden method that required a return adjustment.

Added:
    cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp
Modified:
    cfe/trunk/lib/AST/VTableBuilder.cpp
    cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-single-inheritance.cpp

Modified: cfe/trunk/lib/AST/VTableBuilder.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/VTableBuilder.cpp?rev=198080&r1=198079&r2=198080&view=diff
==============================================================================
--- cfe/trunk/lib/AST/VTableBuilder.cpp (original)
+++ cfe/trunk/lib/AST/VTableBuilder.cpp Fri Dec 27 13:43:59 2013
@@ -2541,6 +2541,8 @@ private:
     }
   }
 
+  bool VFTableBuilder::NeedsReturnAdjustingThunk(const CXXMethodDecl *MD);
+
   /// AddMethods - Add the methods of this base subobject and the relevant
   /// subbases to the vftable we're currently laying out.
   void AddMethods(BaseSubobject Base, unsigned BaseDepth,
@@ -2800,6 +2802,24 @@ static void GroupNewVirtualOverloads(
     VirtualMethods.append(Groups[I].rbegin(), Groups[I].rend());
 }
 
+/// We need a return adjusting thunk for this method if its return type is
+/// not trivially convertible to the return type of any of its overridden
+/// methods.
+bool VFTableBuilder::NeedsReturnAdjustingThunk(const CXXMethodDecl *MD) {
+  OverriddenMethodsSetTy OverriddenMethods;
+  ComputeAllOverriddenMethods(MD, OverriddenMethods);
+  for (OverriddenMethodsSetTy::iterator I = OverriddenMethods.begin(),
+                                        E = OverriddenMethods.end();
+       I != E; ++I) {
+    const CXXMethodDecl *OverriddenMD = *I;
+    BaseOffset Adjustment =
+        ComputeReturnAdjustmentBaseOffset(Context, MD, OverriddenMD);
+    if (!Adjustment.isEmpty())
+      return true;
+  }
+  return false;
+}
+
 void VFTableBuilder::AddMethods(BaseSubobject Base, unsigned BaseDepth,
                                 const CXXRecordDecl *LastVBase,
                                 BasesSetVectorTy &VisitedBases) {
@@ -2885,8 +2905,7 @@ void VFTableBuilder::AddMethods(BaseSubo
         AddThunk(MD, VTableThunks[OverriddenMethodInfo.VFTableIndex]);
       }
 
-      if (Context.hasSameType(MD->getResultType(),
-                              OverriddenMD->getResultType())) {
+      if (!NeedsReturnAdjustingThunk(MD)) {
         // No return adjustment needed - just replace the overridden method info
         // with the current info.
         MethodInfo MI(OverriddenMethodInfo.VBTableIndex,
@@ -3049,6 +3068,8 @@ void VFTableBuilder::dumpLayout(raw_ostr
     case VTableComponent::CK_FunctionPointer: {
       const CXXMethodDecl *MD = Component.getFunctionDecl();
 
+      // FIXME: Figure out how to print the real thunk type, since they can
+      // differ in the return type.
       std::string Str = PredefinedExpr::ComputeName(
           PredefinedExpr::PrettyFunctionNoVirtual, MD);
       Out << Str;

Added: cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp?rev=198080&view=auto
==============================================================================
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp (added)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-return-thunks.cpp Fri Dec 27 13:43:59 2013
@@ -0,0 +1,104 @@
+// RUN: %clang_cc1 -fno-rtti %s -emit-llvm -o %t -cxx-abi microsoft -triple=i386-pc-win32 -fdump-vtable-layouts 2>&1 | FileCheck --check-prefix=VFTABLES %s
+// RUN: FileCheck --check-prefix=GLOBALS %s < %t
+// RUN: FileCheck --check-prefix=CODEGEN %s < %t
+
+namespace test1 {
+
+// Some covariant types.
+struct A { int a; };
+struct B { int b; };
+struct C : A, B { int c; };
+struct D : C { int d; };
+struct E : D { int e; };
+
+// One base class and two overrides, all with covariant return types.
+struct H     { virtual B *foo(); };
+struct I : H { virtual C *foo(); };
+struct J : I { virtual D *foo(); J(); };
+struct K : J { virtual E *foo(); K(); };
+
+J::J() {}
+
+// VFTABLES-LABEL: VFTable for 'test1::H' in 'test1::I' in 'test1::J' (3 entries).
+// VFTABLES:    0 | test1::D *test1::J::foo()
+// VFTABLES:         [return adjustment: 4 non-virtual]
+// VFTABLES:    1 | test1::D *test1::J::foo()
+// VFTABLES:    2 | test1::D *test1::J::foo()
+
+// GLOBALS-LABEL: @"\01??_7J at test1@@6B@" = linkonce_odr unnamed_addr constant [3 x i8*]
+// GLOBALS: @"\01?foo at J@test1@@QAEPAUB at 2@XZ"
+// GLOBALS: @"\01?foo at J@test1@@QAEPAUC at 2@XZ"
+// GLOBALS: @"\01?foo at J@test1@@QAEPAUD at 2@XZ"
+// FIXME: Should be UAEPAUD.
+
+K::K() {}
+
+// VFTABLES-LABEL: VFTable for 'test1::H' in 'test1::I' in 'test1::J' in 'test1::K' (4 entries).
+// VFTABLES:   0 | test1::E *test1::K::foo()
+// VFTABLES:        [return adjustment: 4 non-virtual]
+// VFTABLES:   1 | test1::E *test1::K::foo()
+// VFTABLES:   2 | test1::E *test1::K::foo()
+// VFTABLES:   3 | test1::E *test1::K::foo()
+
+// Only B to C requires adjustment, but we get 3 thunks in K's vftable, two of
+// which are trivial.
+// GLOBALS-LABEL: @"\01??_7K at test1@@6B@" = linkonce_odr unnamed_addr constant [4 x i8*]
+// GLOBALS: @"\01?foo at K@test1@@QAEPAUB at 2@XZ"
+// GLOBALS: @"\01?foo at K@test1@@QAEPAUC at 2@XZ"
+// GLOBALS: @"\01?foo at K@test1@@QAEPAUD at 2@XZ"
+// GLOBALS: @"\01?foo at K@test1@@QAEPAUE at 2@XZ"
+// FIXME: Should be UAEPAUE.
+
+//  This thunk has a return adjustment.
+// CODEGEN-LABEL: define {{.*}} @"\01?foo at K@test1@@QAEPAUB at 2@XZ"
+// CODEGEN: call {{.*}} @"\01?foo at K@test1@@UAEPAUE at 2@XZ"
+// CODEGEN: icmp {{.*}}, null
+// CODEGEN: getelementptr
+// CODEGEN: ret
+
+//  These two don't.
+// CODEGEN-LABEL: define {{.*}} @"\01?foo at K@test1@@QAEPAUC at 2@XZ"
+// CODEGEN: call {{.*}} @"\01?foo at K@test1@@UAEPAUE at 2@XZ"
+// CODEGEN-NEXT: ret
+
+// CODEGEN-LABEL: define {{.*}} @"\01?foo at K@test1@@QAEPAUD at 2@XZ"
+// CODEGEN: call {{.*}} @"\01?foo at K@test1@@UAEPAUE at 2@XZ"
+// CODEGEN-NEXT: ret
+
+}
+
+namespace test2 {
+
+// Covariant types.  D* is not trivially convertible to C*.
+struct A { int a; };
+struct B { int b; };
+struct C : B { int c; };
+struct D : A, C { int d; };
+struct E : D { int e; };
+
+// J's foo will require an adjusting thunk, and K will require a trivial thunk.
+struct H     { virtual B *foo(); };
+struct I : H { virtual C *foo(); };
+struct J : I { virtual D *foo(); J(); };
+struct K : J { virtual E *foo(); K(); };
+
+J::J() {}
+
+// VFTABLES-LABEL: VFTable for 'test2::H' in 'test2::I' in 'test2::J' (2 entries).
+// VFTABLES:    0 | test2::D *test2::J::foo()
+// VFTABLES:         [return adjustment: 4 non-virtual]
+// VFTABLES:    1 | test2::D *test2::J::foo()
+
+// GLOBALS-LABEL: @"\01??_7J at test2@@6B@" = linkonce_odr unnamed_addr constant [2 x i8*]
+
+K::K() {}
+
+// VFTABLES-LABEL: VFTable for 'test2::H' in 'test2::I' in 'test2::J' in 'test2::K' (3 entries).
+// VFTABLES:    0 | test2::E *test2::K::foo()
+// VFTABLES:         [return adjustment: 4 non-virtual]
+// VFTABLES:    1 | test2::E *test2::K::foo()
+// VFTABLES:    2 | test2::E *test2::K::foo()
+
+// GLOBALS-LABEL: @"\01??_7K at test2@@6B@" = linkonce_odr unnamed_addr constant [3 x i8*]
+
+}

Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-single-inheritance.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-single-inheritance.cpp?rev=198080&r1=198079&r2=198080&view=diff
==============================================================================
--- cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-single-inheritance.cpp (original)
+++ cfe/trunk/test/CodeGenCXX/microsoft-abi-vtables-single-inheritance.cpp Fri Dec 27 13:43:59 2013
@@ -252,12 +252,11 @@ struct N {
 
 N n;
 
-typedef int int_type;
-struct O { virtual int f(); };
-struct P : O { virtual int_type f(); };
+struct O { virtual A *f(); };
+struct P : O { virtual B *f(); };
 P p;
 // CHECK-O: VFTable for 'O' in 'P' (1 entries)
-// CHECK-O-NEXT: 0 | int_type P::f()
+// CHECK-O-NEXT: 0 | B *P::f()
 
 // CHECK-O: VFTable for 'O' (1 entries)
-// CHECK-O-NEXT: 0 | int O::f()
+// CHECK-O-NEXT: 0 | A *O::f()





More information about the cfe-commits mailing list