Author: rafael Date: Tue Sep 3 16:05:13 2013 New Revision: 189852 URL: http://llvm.org/viewvc/llvm-project?rev=189852&view=rev Log: Don't emit an available_externally vtable pointing to linkonce_odr funcs.
This fixes pr13124. >From the discussion at http://lists.cs.uiuc.edu/pipermail/cfe-dev/2012-June/022606.html we know that we cannot make funcions in a weak_odr vtable also weak_odr. They should remain linkonce_odr. The side effect is that we cannot emit a available_externally vtable unless we also emit a copy of the function. This also has an issue: If codegen is going to output a function, sema has to mark it used. Given llvm.org/pr9114, it looks like sema cannot be more aggressive at marking functions used because of vtables. This leaves us with a few unpleasant options: * Marking functions in vtables used if possible. This sounds a bit sloppy, so we should avoid it. * Producing available_externally vtables only when all the functions in it are already used or weak_odr. This would cover cases like -------------------- struct foo { virtual ~foo(); }; struct bar : public foo { virtual void zed(); }; void f() { foo *x(new bar); delete x; } void g(bar *x) { x->~bar(); // force the destructor to be used } -------------------------- and ---------------------------------- template<typename T> struct bar { virtual ~bar(); }; template<typename T> bar<T>::~bar() { } // make the destructor weak_odr instead of linkonce_odr extern template class bar<int>; void f() { bar<int> *x(new bar<int>); delete x; } ---------------------------- These look like corner cases, so it is unclear if it is worth it. * And finally: Just nuke this optimization. That is what this patch implements. Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp cfe/trunk/test/CodeGenCXX/thunks-available-externally.cpp cfe/trunk/test/CodeGenCXX/vtable-available-externally.cpp cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=189852&r1=189851&r2=189852&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CGVTables.cpp (original) +++ cfe/trunk/lib/CodeGen/CGVTables.cpp Tue Sep 3 16:05:13 2013 @@ -753,12 +753,7 @@ CodeGenModule::getVTableLinkage(const CX switch (keyFunction->getTemplateSpecializationKind()) { case TSK_Undeclared: case TSK_ExplicitSpecialization: - // When compiling with optimizations turned on, we emit all vtables, - // even if the key function is not defined in the current translation - // unit. If this is the case, use available_externally linkage. - if (!def && CodeGenOpts.OptimizationLevel) - return llvm::GlobalVariable::AvailableExternallyLinkage; - + assert(def && "Should not have been asked to emit this"); if (keyFunction->isInlined()) return !Context.getLangOpts().AppleKext ? llvm::GlobalVariable::LinkOnceODRLinkage : @@ -777,9 +772,7 @@ CodeGenModule::getVTableLinkage(const CX llvm::Function::InternalLinkage; case TSK_ExplicitInstantiationDeclaration: - return !Context.getLangOpts().AppleKext ? - llvm::GlobalVariable::AvailableExternallyLinkage : - llvm::Function::InternalLinkage; + llvm_unreachable("Should not have been asked to emit this"); } } @@ -795,7 +788,7 @@ CodeGenModule::getVTableLinkage(const CX return llvm::GlobalVariable::LinkOnceODRLinkage; case TSK_ExplicitInstantiationDeclaration: - return llvm::GlobalVariable::AvailableExternallyLinkage; + llvm_unreachable("Should not have been asked to emit this"); case TSK_ExplicitInstantiationDefinition: return llvm::GlobalVariable::WeakODRLinkage; @@ -897,16 +890,6 @@ bool CodeGenVTables::isVTableExternal(co /// we define that v-table? static bool shouldEmitVTableAtEndOfTranslationUnit(CodeGenModule &CGM, const CXXRecordDecl *RD) { - // If we're building with optimization, we always emit v-tables - // since that allows for virtual function calls to be devirtualized. - // If the v-table is defined strongly elsewhere, this definition - // will be emitted available_externally. - // - // However, we don't want to do this in -fapple-kext mode, because - // kext mode does not permit devirtualization. - if (CGM.getCodeGenOpts().OptimizationLevel && !CGM.getLangOpts().AppleKext) - return true; - return !CGM.getVTables().isVTableExternal(RD); } Modified: cfe/trunk/test/CodeGenCXX/thunks-available-externally.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/thunks-available-externally.cpp?rev=189852&r1=189851&r2=189852&view=diff ============================================================================== --- cfe/trunk/test/CodeGenCXX/thunks-available-externally.cpp (original) +++ cfe/trunk/test/CodeGenCXX/thunks-available-externally.cpp Tue Sep 3 16:05:13 2013 @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -emit-llvm -O3 -o - | FileCheck %s +// RUN: %clang_cc1 %s -I%S -triple=x86_64-apple-darwin10 -emit-llvm-only -O3 // Check that we don't assert on this case. namespace Test1 { @@ -58,15 +58,6 @@ static void f(B* b) { b->f(); } -// CHECK-LABEL: define void @_ZN5Test21fEv() -// CHECK: call void @_ZN5Test21C1fEv -// CHECK: ret void -// CHECK-LABEL: define available_externally void @_ZThn16_N5Test21C1fEv -void f() { - C c; - f(&c); -} - } // Test that we don't assert. Modified: cfe/trunk/test/CodeGenCXX/vtable-available-externally.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/vtable-available-externally.cpp?rev=189852&r1=189851&r2=189852&view=diff ============================================================================== --- cfe/trunk/test/CodeGenCXX/vtable-available-externally.cpp (original) +++ cfe/trunk/test/CodeGenCXX/vtable-available-externally.cpp Tue Sep 3 16:05:13 2013 @@ -2,18 +2,10 @@ // RUN: FileCheck --check-prefix=CHECK-TEST1 %s < %t // RUN: FileCheck --check-prefix=CHECK-TEST2 %s < %t // RUN: FileCheck --check-prefix=CHECK-TEST5 %s < %t -// RUN: FileCheck --check-prefix=CHECK-TEST7 %s < %t #include <typeinfo> -// Test1::A's key function (f) is not defined in this translation -// unit, but in order to devirtualize calls, we emit the v-table with -// available_externally linkage. -// -// There's no real reason to do this to the RTTI, though. - -// CHECK-TEST1: @_ZTVN5Test11AE = available_externally -// CHECK-TEST1: @_ZTIN5Test11AE = external constant i8* +// CHECK-TEST1: @_ZTVN5Test11AE = external unnamed_addr constant namespace Test1 { struct A { @@ -159,14 +151,4 @@ struct c11 : c10, c1{ struct c28 : virtual c11{ void f6 (); }; - -// CHECK-TEST7-LABEL: define void @_ZN5Test79check_c28Ev -// CHECK-TEST7: call void @_ZN5Test73c282f6Ev -// CHECK-TEST7: ret void -void check_c28 () { - c28 obj; - c11 *ptr = &obj; - ptr->f6 (); -} - } Modified: cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp?rev=189852&r1=189851&r2=189852&view=diff ============================================================================== --- cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp (original) +++ cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp Tue Sep 3 16:05:13 2013 @@ -148,13 +148,13 @@ void use_F() { // F<int> is an explicit template instantiation declaration without a // key function, so its vtable should have external linkage. // CHECK-DAG: @_ZTV1FIiE = external unnamed_addr constant -// CHECK-OPT-DAG: @_ZTV1FIiE = available_externally unnamed_addr constant +// CHECK-OPT-DAG: @_ZTV1FIiE = external unnamed_addr constant // E<int> is an explicit template instantiation declaration. It has a // key function that is not instantiated, so we should only reference // its vtable, not define it. // CHECK-DAG: @_ZTV1EIiE = external unnamed_addr constant -// CHECK-OPT-DAG: @_ZTV1EIiE = available_externally unnamed_addr constant +// CHECK-OPT-DAG: @_ZTV1EIiE = external unnamed_addr constant // The anonymous struct for e has no linkage, so the vtable should have // internal linkage. @@ -202,17 +202,14 @@ void use_H() { } // I<int> has an explicit instantiation declaration and needs a VTT and -// construction vtables. We emit the VTT available_externally, but point it at -// internal construction vtables because there is no way to form a reference to -// the real construction vtables. +// construction vtables. // CHECK-DAG: @_ZTV1IIiE = external unnamed_addr constant // CHECK-DAG: @_ZTT1IIiE = external unnamed_addr constant // CHECK-NOT: @_ZTC1IIiE // -// CHECK-OPT-DAG: @_ZTV1IIiE = available_externally unnamed_addr constant -// CHECK-OPT-DAG: @_ZTT1IIiE = available_externally unnamed_addr constant {{.*}} @_ZTC1IIiE0_6VBase2 -// CHECK-OPT-DAG: @_ZTC1IIiE0_6VBase2 = internal unnamed_addr constant +// CHECK-OPT-DAG: @_ZTV1IIiE = external unnamed_addr constant +// CHECK-OPT-DAG: @_ZTT1IIiE = external unnamed_addr constant struct VBase1 { virtual void f(); }; struct VBase2 : virtual VBase1 {}; template<typename T> struct I : VBase2 {}; _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
