Author: rnk Date: Wed Jun 29 13:29:21 2016 New Revision: 274147 URL: http://llvm.org/viewvc/llvm-project?rev=274147&view=rev Log: Re-land "[MS] Don't expect vftables to be provided for extern template instantiations"
Reverts r273305 and re-instates r273296. We needed to fix a bug in Sema::MarkVTableUsed to ensure that operator delete lookup occurs when the vtable is referenced. We already had a special case to look up operator delete when dllimport was used, but I think should really mark virtual destructors referenced any time the vtable is used. Added: cfe/trunk/test/CodeGenCXX/microsoft-abi-extern-template.cpp Modified: cfe/trunk/lib/AST/VTableBuilder.cpp cfe/trunk/lib/CodeGen/CGVTables.cpp cfe/trunk/lib/Sema/SemaDeclCXX.cpp cfe/trunk/test/CodeGenCXX/microsoft-abi-vbtables.cpp cfe/trunk/test/CodeGenCXX/microsoft-abi-vftables.cpp Modified: cfe/trunk/lib/AST/VTableBuilder.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/VTableBuilder.cpp?rev=274147&r1=274146&r2=274147&view=diff ============================================================================== --- cfe/trunk/lib/AST/VTableBuilder.cpp (original) +++ cfe/trunk/lib/AST/VTableBuilder.cpp Wed Jun 29 13:29:21 2016 @@ -2545,14 +2545,13 @@ public: MostDerivedClassLayout(Context.getASTRecordLayout(MostDerivedClass)), WhichVFPtr(*Which), Overriders(MostDerivedClass, CharUnits(), MostDerivedClass) { - // Only include the RTTI component if we know that we will provide a - // definition of the vftable. We always provide the definition of - // dllimported classes. + // Provide the RTTI component if RTTIData is enabled. If the vftable would + // be available externally, we should not provide the RTTI componenent. It + // is currently impossible to get available externally vftables with either + // dllimport or extern template instantiations, but eventually we may add a + // flag to support additional devirtualization that needs this. if (Context.getLangOpts().RTTIData) - if (MostDerivedClass->hasAttr<DLLImportAttr>() || - MostDerivedClass->getTemplateSpecializationKind() != - TSK_ExplicitInstantiationDeclaration) - HasRTTIComponent = true; + HasRTTIComponent = true; LayoutVFTable(); Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=274147&r1=274146&r2=274147&view=diff ============================================================================== --- cfe/trunk/lib/CodeGen/CGVTables.cpp (original) +++ cfe/trunk/lib/CodeGen/CGVTables.cpp Wed Jun 29 13:29:21 2016 @@ -794,6 +794,10 @@ CodeGenModule::getVTableLinkage(const CX return DiscardableODRLinkage; case TSK_ExplicitInstantiationDeclaration: + // Explicit instantiations in MSVC do not provide vtables, so we must emit + // our own. + if (getTarget().getCXXABI().isMicrosoft()) + return DiscardableODRLinkage; return shouldEmitAvailableExternallyVTable(*this, RD) ? llvm::GlobalVariable::AvailableExternallyLinkage : llvm::GlobalVariable::ExternalLinkage; @@ -839,9 +843,9 @@ CodeGenVTables::GenerateClassData(const bool CodeGenVTables::isVTableExternal(const CXXRecordDecl *RD) { assert(RD->isDynamicClass() && "Non-dynamic classes have no VTable."); - // We always synthesize vtables on the import side regardless of whether or - // not it is an explicit instantiation declaration. - if (CGM.getTarget().getCXXABI().isMicrosoft() && RD->hasAttr<DLLImportAttr>()) + // We always synthesize vtables if they are needed in the MS ABI. MSVC doesn't + // emit them even if there is an explicit template instantiation. + if (CGM.getTarget().getCXXABI().isMicrosoft()) return false; // If we have an explicit instantiation declaration (and not a Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=274147&r1=274146&r2=274147&view=diff ============================================================================== --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original) +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed Jun 29 13:29:21 2016 @@ -13425,20 +13425,18 @@ void Sema::MarkVTableUsed(SourceLocation // checks (i.e. operator delete() lookup) when the vtable is marked used, as // the deleting destructor is emitted with the vtable, not with the // destructor definition as in the Itanium ABI. - // If it has a definition, we do the check at that point instead. if (Context.getTargetInfo().getCXXABI().isMicrosoft()) { - if (Class->hasUserDeclaredDestructor() && - !Class->getDestructor()->isDefined() && - !Class->getDestructor()->isDeleted()) { - CXXDestructorDecl *DD = Class->getDestructor(); - ContextRAII SavedContext(*this, DD); - CheckDestructor(DD); - } else if (Class->hasAttr<DLLImportAttr>()) { - // We always synthesize vtables on the import side. To make sure - // CheckDestructor gets called, mark the destructor referenced. - assert(Class->getDestructor() && - "The destructor has always been declared on a dllimport class"); - MarkFunctionReferenced(Loc, Class->getDestructor()); + CXXDestructorDecl *DD = Class->getDestructor(); + if (DD && DD->isVirtual() && !DD->isDeleted()) { + if (Class->hasUserDeclaredDestructor() && !DD->isDefined()) { + // If this is an out-of-line declaration, marking it referenced will + // not do anything. Manually call CheckDestructor to look up operator + // delete(). + ContextRAII SavedContext(*this, DD); + CheckDestructor(DD); + } else { + MarkFunctionReferenced(Loc, Class->getDestructor()); + } } } } Added: cfe/trunk/test/CodeGenCXX/microsoft-abi-extern-template.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-extern-template.cpp?rev=274147&view=auto ============================================================================== --- cfe/trunk/test/CodeGenCXX/microsoft-abi-extern-template.cpp (added) +++ cfe/trunk/test/CodeGenCXX/microsoft-abi-extern-template.cpp Wed Jun 29 13:29:21 2016 @@ -0,0 +1,25 @@ +// RUN: %clang_cc1 -fno-rtti-data -O1 -disable-llvm-optzns %s -emit-llvm -o - -triple x86_64-windows-msvc | FileCheck %s + +// Even though Foo<int> has an extern template declaration, we have to emit our +// own copy the vftable when emitting the available externally constructor. + +// CHECK: @"\01??_7?$Foo@H@@6B@" = linkonce_odr unnamed_addr constant [1 x i8*] [ +// CHECK-SAME: i8* bitcast (i8* (%struct.Foo*, i32)* @"\01??_G?$Foo@H@@UEAAPEAXI@Z" to i8*) +// CHECK-SAME: ], comdat + +// CHECK-LABEL: define %struct.Foo* @"\01?f@@YAPEAU?$Foo@H@@XZ"() +// CHECK: call %struct.Foo* @"\01??0?$Foo@H@@QEAA@XZ"(%struct.Foo* %{{.*}}) + +// CHECK: define available_externally %struct.Foo* @"\01??0?$Foo@H@@QEAA@XZ"(%struct.Foo* returned %this) +// CHECK: store {{.*}} @"\01??_7?$Foo@H@@6B@" + +// CHECK: define linkonce_odr i8* @"\01??_G?$Foo@H@@UEAAPEAXI@Z"(%struct.Foo* %this, i32 %should_call_delete) + +struct Base { + virtual ~Base(); +}; +template <typename T> struct Foo : Base { + Foo() {} +}; +extern template class Foo<int>; +Foo<int> *f() { return new Foo<int>(); } Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-vbtables.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-vbtables.cpp?rev=274147&r1=274146&r2=274147&view=diff ============================================================================== --- cfe/trunk/test/CodeGenCXX/microsoft-abi-vbtables.cpp (original) +++ cfe/trunk/test/CodeGenCXX/microsoft-abi-vbtables.cpp Wed Jun 29 13:29:21 2016 @@ -537,5 +537,5 @@ template <class> struct B : virtual A { extern template class B<int>; template B<int>::B(); -// CHECK-DAG: @"\01??_8?$B@H@Test30@@7B@" = external unnamed_addr constant [2 x i32]{{$}} +// CHECK-DAG: @"\01??_8?$B@H@Test30@@7B@" = linkonce_odr unnamed_addr constant [2 x i32] [i32 0, i32 4], comdat } Modified: cfe/trunk/test/CodeGenCXX/microsoft-abi-vftables.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/microsoft-abi-vftables.cpp?rev=274147&r1=274146&r2=274147&view=diff ============================================================================== --- cfe/trunk/test/CodeGenCXX/microsoft-abi-vftables.cpp (original) +++ cfe/trunk/test/CodeGenCXX/microsoft-abi-vftables.cpp Wed Jun 29 13:29:21 2016 @@ -33,7 +33,7 @@ struct __declspec(dllexport) V { namespace { struct W { - virtual ~W(); + virtual ~W() {} } w; } // RTTI-DAG: [[VTABLE_W:@.*]] = private unnamed_addr constant [2 x i8*] [i8* bitcast ({{.*}} @"\01??_R4W@?A@@6B@" to i8*), i8* bitcast ({{.*}} @"\01??_GW@?A@@UAEPAXI@Z" to i8*)] @@ -49,5 +49,7 @@ template <class> struct Y : virtual X { extern template class Y<int>; template Y<int>::Y(); -// RTTI-DAG: @"\01??_7?$Y@H@@6B@" = external unnamed_addr constant [1 x i8*] -// NO-RTTI-DAG: @"\01??_7?$Y@H@@6B@" = external unnamed_addr constant [1 x i8*] +// RTTI-DAG: [[VTABLE_Y:@.*]] = private unnamed_addr constant [2 x i8*] [i8* bitcast (%rtti.CompleteObjectLocator* @"\01??_R4?$Y@H@@6B@" to i8*), i8* bitcast (i8* (%struct.Y*, i32)* @"\01??_G?$Y@H@@UAEPAXI@Z" to i8*)], comdat($"\01??_7?$Y@H@@6B@") +// RTTI-DAG: @"\01??_7?$Y@H@@6B@" = unnamed_addr alias i8*, getelementptr inbounds ([2 x i8*], [2 x i8*]* [[VTABLE_Y]], i32 0, i32 1) + +// NO-RTTI-DAG: @"\01??_7?$Y@H@@6B@" = linkonce_odr unnamed_addr constant [1 x i8*] [i8* bitcast (i8* (%struct.Y*, i32)* @"\01??_G?$Y@H@@UAEPAXI@Z" to i8*)], comdat _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits