From 3e3b23e283a01c27838620db2aaae56aea621ef0 Mon Sep 17 00:00:00 2001 From: Piotr Padlewski Date: Thu, 27 Aug 2015 21:35:41 +0000 Subject: [PATCH] Assume loads fix #2 There was linker problem, and it turns out that it is not always safe to refer to vtable. If the vtable is used, then we can refer to it without any problem, but because we don't know when it will be used or not, we can only check if vtable is external or it is safe to to emit it speculativly (when class it doesn't have any inline virtual functions). It should be fixed in the future. http://reviews.llvm.org/D12385 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@246214 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/CGCXXABI.h | 6 +- lib/CodeGen/CGClass.cpp | 9 +- lib/CodeGen/CGVTables.cpp | 6 +- lib/CodeGen/ItaniumCXXABI.cpp | 5 +- lib/CodeGen/MicrosoftCXXABI.cpp | 3 +- test/CodeGenCXX/template-instantiation.cpp | 3 +- test/CodeGenCXX/thunks.cpp | 6 +- test/CodeGenCXX/vtable-assume-load.cpp | 133 +++++++++++++++++- .../vtable-available-externally.cpp | 11 +- 9 files changed, 156 insertions(+), 26 deletions(-) diff --git a/lib/CodeGen/CGCXXABI.h b/lib/CodeGen/CGCXXABI.h index 158d29ab51..259c878f7f 100644 --- a/lib/CodeGen/CGCXXABI.h +++ b/lib/CodeGen/CGCXXABI.h @@ -218,8 +218,10 @@ public: virtual void emitThrow(CodeGenFunction &CGF, const CXXThrowExpr *E) = 0; virtual llvm::GlobalVariable *getThrowInfo(QualType T) { return nullptr; } - virtual bool canEmitAvailableExternallyVTable( - const CXXRecordDecl *RD) const = 0; + /// \brief Determine whether it's possible to emit a vtable for \p RD, even + /// though we do not know that the vtable has been marked as used by semantic + /// analysis. + virtual bool canSpeculativelyEmitVTable(const CXXRecordDecl *RD) const = 0; virtual void emitBeginCatch(CodeGenFunction &CGF, const CXXCatchStmt *C) = 0; diff --git a/lib/CodeGen/CGClass.cpp b/lib/CodeGen/CGClass.cpp index cd39749889..507bae8d09 100644 --- a/lib/CodeGen/CGClass.cpp +++ b/lib/CodeGen/CGClass.cpp @@ -1857,8 +1857,15 @@ void CodeGenFunction::EmitCXXConstructorCall(const CXXConstructorDecl *D, // with a vtable. We don't do this for base subobjects for two reasons: // first, it's incorrect for classes with virtual bases, and second, we're // about to overwrite the vptrs anyway. + // We also have to make sure if we can refer to vtable: + // - If vtable is external then it's safe to use it (for available_externally + // CGVTables will make sure if it can emit it). + // - Otherwise we can refer to vtable if it's safe to speculatively emit. + // FIXME: If vtable is used by ctor/dtor, we are always safe to refer to it. if (CGM.getCodeGenOpts().OptimizationLevel > 0 && - ClassDecl->isDynamicClass() && Type != Ctor_Base) + ClassDecl->isDynamicClass() && Type != Ctor_Base && + (CGM.getVTables().isVTableExternal(ClassDecl) || + CGM.getCXXABI().canSpeculativelyEmitVTable(ClassDecl))) EmitVTableAssumptionLoads(ClassDecl, This); } diff --git a/lib/CodeGen/CGVTables.cpp b/lib/CodeGen/CGVTables.cpp index fcb5c36649..bd620a1ad4 100644 --- a/lib/CodeGen/CGVTables.cpp +++ b/lib/CodeGen/CGVTables.cpp @@ -682,7 +682,7 @@ CodeGenVTables::GenerateConstructionVTable(const CXXRecordDecl *RD, static bool shouldEmitAvailableExternallyVTable(const CodeGenModule &CGM, const CXXRecordDecl *RD) { return CGM.getCodeGenOpts().OptimizationLevel > 0 && - CGM.getCXXABI().canEmitAvailableExternallyVTable(RD); + CGM.getCXXABI().canSpeculativelyEmitVTable(RD); } /// Compute the required linkage of the v-table for the given class. @@ -832,11 +832,11 @@ bool CodeGenVTables::isVTableExternal(const CXXRecordDecl *RD) { /// we define that v-table? static bool shouldEmitVTableAtEndOfTranslationUnit(CodeGenModule &CGM, const CXXRecordDecl *RD) { - // If vtable is internal then it has to be done + // If vtable is internal then it has to be done. if (!CGM.getVTables().isVTableExternal(RD)) return true; - // If it's external then maybe we will need it as available_externally + // If it's external then maybe we will need it as available_externally. return shouldEmitAvailableExternallyVTable(CGM, RD); } diff --git a/lib/CodeGen/ItaniumCXXABI.cpp b/lib/CodeGen/ItaniumCXXABI.cpp index ce37a7fbe0..31a1e488bc 100644 --- a/lib/CodeGen/ItaniumCXXABI.cpp +++ b/lib/CodeGen/ItaniumCXXABI.cpp @@ -229,7 +229,7 @@ public: void emitVirtualInheritanceTables(const CXXRecordDecl *RD) override; - bool canEmitAvailableExternallyVTable(const CXXRecordDecl *RD) const override; + bool canSpeculativelyEmitVTable(const CXXRecordDecl *RD) const override; void setThunkLinkage(llvm::Function *Thunk, bool ForVTable, GlobalDecl GD, bool ReturnAdjustment) override { @@ -1523,8 +1523,7 @@ void ItaniumCXXABI::emitVirtualInheritanceTables(const CXXRecordDecl *RD) { VTables.EmitVTTDefinition(VTT, CGM.getVTableLinkage(RD), RD); } -bool ItaniumCXXABI::canEmitAvailableExternallyVTable( - const CXXRecordDecl *RD) const { +bool ItaniumCXXABI::canSpeculativelyEmitVTable(const CXXRecordDecl *RD) const { // We don't emit available_externally vtables if we are in -fapple-kext mode // because kext mode does not permit devirtualization. if (CGM.getLangOpts().AppleKext) diff --git a/lib/CodeGen/MicrosoftCXXABI.cpp b/lib/CodeGen/MicrosoftCXXABI.cpp index bbc5c2fe87..de97ec3d5d 100644 --- a/lib/CodeGen/MicrosoftCXXABI.cpp +++ b/lib/CodeGen/MicrosoftCXXABI.cpp @@ -106,8 +106,7 @@ public: QualType DestTy) override; bool EmitBadCastCall(CodeGenFunction &CGF) override; - bool canEmitAvailableExternallyVTable( - const CXXRecordDecl *RD) const override { + bool canSpeculativelyEmitVTable(const CXXRecordDecl *RD) const override { return false; } diff --git a/test/CodeGenCXX/template-instantiation.cpp b/test/CodeGenCXX/template-instantiation.cpp index 0eab85abee..90b8099f0a 100644 --- a/test/CodeGenCXX/template-instantiation.cpp +++ b/test/CodeGenCXX/template-instantiation.cpp @@ -1,7 +1,5 @@ // RUN: %clang_cc1 %s -O1 -disable-llvm-optzns -triple=x86_64-apple-darwin10 -emit-llvm -o - | FileCheck %s -// CHECK: @_ZTVN5test018stdio_sync_filebufIA4_iEE = linkonce_odr unnamed_addr constant - // CHECK: @_ZN7PR100011xE = global // CHECK-NOT: @_ZN7PR100014kBarE = external global i32 // @@ -14,6 +12,7 @@ // CHECK: @_ZN7PR100011SIiE3arrE = linkonce_odr global [3 x i32] // CHECK-NOT: @_ZN7PR100011SIiE3arr2E = linkonce_odr global [3 x i32]A +// CHECK: @_ZTVN5test018stdio_sync_filebufIA4_iEE = linkonce_odr unnamed_addr constant // CHECK-NOT: _ZTVN5test31SIiEE // CHECK-NOT: _ZTSN5test31SIiEE diff --git a/test/CodeGenCXX/thunks.cpp b/test/CodeGenCXX/thunks.cpp index 3e5f0059b8..d460fe3fd5 100644 --- a/test/CodeGenCXX/thunks.cpp +++ b/test/CodeGenCXX/thunks.cpp @@ -398,11 +398,13 @@ D::~D() {} // Checking with opt // CHECK-OPT-LABEL: define internal void @_ZThn8_N6Test4B12_GLOBAL__N_11C1fEv(%"struct.Test4B::(anonymous namespace)::C"* %this) unnamed_addr #0 align 2 +// This is from Test5: +// CHECK-OPT-LABEL: define linkonce_odr void @_ZTv0_n24_N5Test51B1fEv + // This is from Test10: // CHECK-OPT-LABEL: define linkonce_odr void @_ZN6Test101C3fooEv // CHECK-OPT-LABEL: define linkonce_odr void @_ZThn8_N6Test101C3fooEv -// This is from Test5: -// CHECK-OPT-LABEL: define linkonce_odr void @_ZTv0_n24_N5Test51B1fEv + // CHECK: attributes [[NUW]] = { nounwind uwtable{{.*}} } diff --git a/test/CodeGenCXX/vtable-assume-load.cpp b/test/CodeGenCXX/vtable-assume-load.cpp index 5ea9e75524..17cd477f19 100644 --- a/test/CodeGenCXX/vtable-assume-load.cpp +++ b/test/CodeGenCXX/vtable-assume-load.cpp @@ -6,7 +6,9 @@ // RUN: FileCheck --check-prefix=CHECK3 --input-file=%t.ll %s // RUN: FileCheck --check-prefix=CHECK4 --input-file=%t.ll %s // RUN: FileCheck --check-prefix=CHECK-MS --input-file=%t.ms.ll %s - +// RUN: FileCheck --check-prefix=CHECK6 --input-file=%t.ll %s +// RUN: FileCheck --check-prefix=CHECK7 --input-file=%t.ll %s +// RUN: FileCheck --check-prefix=CHECK8 --input-file=%t.ll %s namespace test1 { struct A { @@ -150,7 +152,7 @@ void test() { } } // test4 -namespace test5 { +namespace testMS { struct __declspec(novtable) S { virtual void foo(); @@ -159,8 +161,8 @@ struct __declspec(novtable) S { void g(S &s) { s.foo(); } // if struct has novtable specifier, then we can't generate assumes -// CHECK-MS-LABEL: define void @"\01?test@test5@@YAXXZ"() -// CHECK-MS: call x86_thiscallcc %"struct.test5::S"* @"\01??0S@test5@@QAE@XZ"( +// CHECK-MS-LABEL: define void @"\01?test@testMS@@YAXXZ"() +// CHECK-MS: call x86_thiscallcc %"struct.testMS::S"* @"\01??0S@testMS@@QAE@XZ"( // CHECK-MS-NOT: @llvm.assume // CHECK-MS-LABEL: } @@ -169,4 +171,125 @@ void test() { g(s); } -} // test5 +} // testMS + +namespace test6 { +// CHECK6: @_ZTVN5test61AE = external +struct A { + A(); + virtual void foo(); + virtual ~A() {} +}; +struct B : A { + B(); +}; +// Because A's vtable is external, it's safe to generate assumption loads. +// CHECK6-LABEL: define void @_ZN5test61gEv() +// CHECK6: call void @_ZN5test61AC1Ev( +// CHECK6: call void @llvm.assume( + +// We can't emit assumption loads for B, because if we would refer to vtable +// it would refer to functions that will not be able to find (like implicit +// inline destructor). + +// CHECK6-LABEL: call void @_ZN5test61BC1Ev( +// CHECK6-NOT: call void @llvm.assume( +// CHECK6-LABEL: } +void g() { + A *a = new A; + B *b = new B; +} + +} + +namespace test7 { +// Because A's key function is defined here, vtable is generated in this TU +// CHECK7: @_ZTVN5test71AE = unnamed_addr constant +struct A { + A(); + virtual void foo(); + virtual void bar(); +}; +void A::foo() {} + +// CHECK7-LABEL: define void @_ZN5test71gEv() +// CHECK7: call void @_ZN5test71AC1Ev( +// CHECK7: call void @llvm.assume( +// CHECK7-LABEL: } +void g() { + A *a = new A(); + a->bar(); +} +} + +namespace test8 { + +struct A { + virtual void foo(); + virtual void bar(); +}; + +// CHECK8-DAG: @_ZTVN5test81BE = available_externally unnamed_addr constant +struct B : A { + B(); + void foo(); + void bar(); +}; + +// CHECK8-DAG: @_ZTVN5test81CE = linkonce_odr unnamed_addr constant +struct C : A { + C(); + void bar(); + void foo() {} +}; +inline void C::bar() {} + +// CHECK8-DAG: @_ZTVN5test81DE = external unnamed_addr constant +struct D : A { + D(); + void foo(); + void inline bar(); +}; +void D::bar() {} + +// CHECK8-DAG: @_ZTVN5test81EE = linkonce_odr unnamed_addr constant +struct E : A { + E(); +}; + +// CHECK8-LABEL: define void @_ZN5test81bEv() +// CHECK8: call void @llvm.assume( +// CHECK8-LABEL: } +void b() { + B b; + b.bar(); +} + +// FIXME: C has inline virtual functions which prohibits as from generating +// assumption loads, but because vtable is generated in this TU (key function +// defined here) it would be correct to refer to it. +// CHECK8-LABEL: define void @_ZN5test81cEv() +// CHECK8-NOT: call void @llvm.assume( +// CHECK8-LABEL: } +void c() { + C c; + c.bar(); +} + +// CHECK8-LABEL: define void @_ZN5test81dEv() +// CHECK8: call void @llvm.assume( +// CHECK8-LABEL: } +void d() { + D d; + d.bar(); +} + +// CHECK8-LABEL: define void @_ZN5test81eEv() +// CHECK8: call void @llvm.assume( +// CHECK8-LABEL: } +void e() { + E e; + e.bar(); +} +} + diff --git a/test/CodeGenCXX/vtable-available-externally.cpp b/test/CodeGenCXX/vtable-available-externally.cpp index 8134047c9f..ba56499f67 100644 --- a/test/CodeGenCXX/vtable-available-externally.cpp +++ b/test/CodeGenCXX/vtable-available-externally.cpp @@ -184,8 +184,8 @@ void f() { } // Test8 namespace Test9 { -// all virtual functions are outline, so we can assume that it will -// be generated in translation unit where foo is defined +// All virtual functions are outline, so we can assume that it will +// be generated in translation unit where foo is defined. // CHECK-TEST9-DAG: @_ZTVN5Test91AE = available_externally unnamed_addr constant // CHECK-TEST9-DAG: @_ZTVN5Test91BE = available_externally unnamed_addr constant struct A { @@ -217,14 +217,14 @@ struct A { }; void A::foo() {} -// Because key function is inline we will generate vtable as linkonce_odr +// Because key function is inline we will generate vtable as linkonce_odr. // CHECK-TEST10-DAG: @_ZTVN6Test101DE = linkonce_odr unnamed_addr constant struct D : A { void bar(); }; inline void D::bar() {} -// because B has outline key function then we can refer to +// Because B has outline all virtual functions, we can refer to them. // CHECK-TEST10-DAG: @_ZTVN6Test101BE = available_externally unnamed_addr constant struct B : A { void foo(); @@ -233,7 +233,7 @@ struct B : A { // C's key function (car) is outline, but C has inline virtual function so we // can't guarantee that we will be able to refer to bar from name -// so (at the moment) we can't emit vtable available_externally +// so (at the moment) we can't emit vtable available_externally. // CHECK-TEST10-DAG: @_ZTVN6Test101CE = external unnamed_addr constant struct C : A { void bar() {} // defined in body - not key function @@ -365,4 +365,3 @@ void test() { } } - -- 2.40.0