(-old cfe-commits, +new cfe-commits) On Fri, Jan 18, 2019 at 8:21 AM Nico Weber <tha...@chromium.org> wrote:
> I found this comment useful, but from your reply it sounds like it's no > longer true: > > - // We must indicate which VFTable is larger to support linking > between > - // translation units which do and do not have RTTI data. The > largest > - // VFTable contains the RTTI data; translation units which > reference > - // the smaller VFTable always reference it relative to the first > - // virtual method. > > If it's no longer true, why do we still call > setSelectionKind(llvm::Comdat::Largest)? > > On Thu, Jan 17, 2019 at 5:09 PM David Majnemer <david.majne...@gmail.com> > wrote: > >> Yes, the comments don't look like they were relevant after this change. >> >> On Sat, Jan 12, 2019 at 4:52 PM Nico Weber <tha...@chromium.org> wrote: >> >>> This removed two of the comments you had added in >>> https://reviews.llvm.org/rL212142 -- was that intentional? >>> >>> On Wed, Mar 18, 2015 at 6:08 PM David Majnemer <david.majne...@gmail.com> >>> wrote: >>> >>>> Author: majnemer >>>> Date: Wed Mar 18 17:04:43 2015 >>>> New Revision: 232680 >>>> >>>> URL: http://llvm.org/viewvc/llvm-project?rev=232680&view=rev >>>> Log: >>>> MS ABI: Don't try to emit VF/VB-Tables for extern class templates >>>> >>>> There will be an explicit template instantiation in another translation >>>> unit which will provide the definition of the VF/VB-Tables. >>>> >>>> This fixes PR22932. >>>> >>>> Modified: >>>> cfe/trunk/lib/AST/VTableBuilder.cpp >>>> cfe/trunk/lib/CodeGen/CGVTables.cpp >>>> cfe/trunk/lib/CodeGen/CodeGenModule.cpp >>>> cfe/trunk/lib/CodeGen/MicrosoftCXXABI.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=232680&r1=232679&r2=232680&view=diff >>>> >>>> ============================================================================== >>>> --- cfe/trunk/lib/AST/VTableBuilder.cpp (original) >>>> +++ cfe/trunk/lib/AST/VTableBuilder.cpp Wed Mar 18 17:04:43 2015 >>>> @@ -2589,7 +2589,9 @@ public: >>>> // Only include the RTTI component if we know that we will provide >>>> a >>>> // definition of the vftable. >>>> HasRTTIComponent = Context.getLangOpts().RTTIData && >>>> - !MostDerivedClass->hasAttr<DLLImportAttr>(); >>>> + !MostDerivedClass->hasAttr<DLLImportAttr>() && >>>> + >>>> MostDerivedClass->getTemplateSpecializationKind() != >>>> + TSK_ExplicitInstantiationDeclaration; >>>> >>>> LayoutVFTable(); >>>> >>>> >>>> Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp >>>> URL: >>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=232680&r1=232679&r2=232680&view=diff >>>> >>>> ============================================================================== >>>> --- cfe/trunk/lib/CodeGen/CGVTables.cpp (original) >>>> +++ cfe/trunk/lib/CodeGen/CGVTables.cpp Wed Mar 18 17:04:43 2015 >>>> @@ -743,7 +743,7 @@ CodeGenModule::getVTableLinkage(const CX >>>> return DiscardableODRLinkage; >>>> >>>> case TSK_ExplicitInstantiationDeclaration: >>>> - llvm_unreachable("Should not have been asked to emit this"); >>>> + return llvm::GlobalVariable::ExternalLinkage; >>>> >>>> case TSK_ExplicitInstantiationDefinition: >>>> return NonDiscardableODRLinkage; >>>> >>>> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp >>>> URL: >>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=232680&r1=232679&r2=232680&view=diff >>>> >>>> ============================================================================== >>>> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original) >>>> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Wed Mar 18 17:04:43 2015 >>>> @@ -1851,7 +1851,8 @@ CodeGenModule::CreateOrReplaceCXXRuntime >>>> OldGV->eraseFromParent(); >>>> } >>>> >>>> - if (supportsCOMDAT() && GV->isWeakForLinker()) >>>> + if (supportsCOMDAT() && GV->isWeakForLinker() && >>>> + !GV->hasAvailableExternallyLinkage()) >>>> GV->setComdat(TheModule.getOrInsertComdat(GV->getName())); >>>> >>>> return GV; >>>> >>>> Modified: cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp >>>> URL: >>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp?rev=232680&r1=232679&r2=232680&view=diff >>>> >>>> ============================================================================== >>>> --- cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp (original) >>>> +++ cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp Wed Mar 18 17:04:43 2015 >>>> @@ -1487,102 +1487,97 @@ llvm::GlobalVariable *MicrosoftCXXABI::g >>>> #endif >>>> } >>>> >>>> - for (size_t J = 0, F = VFPtrs.size(); J != F; ++J) { >>>> - if (VFPtrs[J]->FullOffsetInMDC != VPtrOffset) >>>> - continue; >>>> - SmallString<256> VFTableName; >>>> - mangleVFTableName(getMangleContext(), RD, VFPtrs[J], VFTableName); >>>> - StringRef VTableName = VFTableName; >>>> - >>>> - uint64_t NumVTableSlots = >>>> - VTContext.getVFTableLayout(RD, VFPtrs[J]->FullOffsetInMDC) >>>> - .getNumVTableComponents(); >>>> - llvm::GlobalValue::LinkageTypes VTableLinkage = >>>> - llvm::GlobalValue::ExternalLinkage; >>>> - llvm::ArrayType *VTableType = >>>> - llvm::ArrayType::get(CGM.Int8PtrTy, NumVTableSlots); >>>> - if (getContext().getLangOpts().RTTIData) { >>>> - VTableLinkage = llvm::GlobalValue::PrivateLinkage; >>>> - VTableName = ""; >>>> - } >>>> + VPtrInfo *const *VFPtrI = >>>> + std::find_if(VFPtrs.begin(), VFPtrs.end(), [&](VPtrInfo *VPI) { >>>> + return VPI->FullOffsetInMDC == VPtrOffset; >>>> + }); >>>> + if (VFPtrI == VFPtrs.end()) { >>>> + VFTablesMap[ID] = nullptr; >>>> + return nullptr; >>>> + } >>>> + VPtrInfo *VFPtr = *VFPtrI; >>>> + >>>> + SmallString<256> VFTableName; >>>> + mangleVFTableName(getMangleContext(), RD, VFPtr, VFTableName); >>>> + >>>> + llvm::GlobalValue::LinkageTypes VFTableLinkage = >>>> CGM.getVTableLinkage(RD); >>>> + bool VFTableComesFromAnotherTU = >>>> + llvm::GlobalValue::isAvailableExternallyLinkage(VFTableLinkage) >>>> || >>>> + llvm::GlobalValue::isExternalLinkage(VFTableLinkage); >>>> + bool VTableAliasIsRequred = >>>> + !VFTableComesFromAnotherTU && >>>> getContext().getLangOpts().RTTIData; >>>> + >>>> + if (llvm::GlobalValue *VFTable = >>>> + CGM.getModule().getNamedGlobal(VFTableName)) { >>>> + VFTablesMap[ID] = VFTable; >>>> + return VTableAliasIsRequred >>>> + ? cast<llvm::GlobalVariable>( >>>> + cast<llvm::GlobalAlias>(VFTable)->getBaseObject()) >>>> + : cast<llvm::GlobalVariable>(VFTable); >>>> + } >>>> >>>> - VTable = CGM.getModule().getNamedGlobal(VFTableName); >>>> - if (!VTable) { >>>> - // Create a backing variable for the contents of VTable. The >>>> VTable may >>>> - // or may not include space for a pointer to RTTI data. >>>> - llvm::GlobalValue *VFTable = VTable = new llvm::GlobalVariable( >>>> - CGM.getModule(), VTableType, /*isConstant=*/true, >>>> VTableLinkage, >>>> - /*Initializer=*/nullptr, VTableName); >>>> - VTable->setUnnamedAddr(true); >>>> - >>>> - // Only insert a pointer into the VFTable for RTTI data if we >>>> are not >>>> - // importing it. We never reference the RTTI data directly so >>>> there is no >>>> - // need to make room for it. >>>> - if (getContext().getLangOpts().RTTIData && >>>> - !RD->hasAttr<DLLImportAttr>()) { >>>> - llvm::Value *GEPIndices[] = {llvm::ConstantInt::get(CGM.IntTy, >>>> 0), >>>> - llvm::ConstantInt::get(CGM.IntTy, >>>> 1)}; >>>> - // Create a GEP which points just after the first entry in the >>>> VFTable, >>>> - // this should be the location of the first virtual method. >>>> - llvm::Constant *VTableGEP = >>>> - llvm::ConstantExpr::getInBoundsGetElementPtr(VTable, >>>> GEPIndices); >>>> - // The symbol for the VFTable is an alias to the GEP. It is >>>> - // transparent, to other modules, what the nature of this >>>> symbol is; all >>>> - // that matters is that the alias be the address of the first >>>> virtual >>>> - // method. >>>> - VFTable = llvm::GlobalAlias::create( >>>> - >>>> cast<llvm::SequentialType>(VTableGEP->getType())->getElementType(), >>>> - /*AddressSpace=*/0, llvm::GlobalValue::ExternalLinkage, >>>> - VFTableName.str(), VTableGEP, &CGM.getModule()); >>>> - } else { >>>> - // We don't need a GlobalAlias to be a symbol for the VTable >>>> if we won't >>>> - // be referencing any RTTI data. The GlobalVariable will end >>>> up being >>>> - // an appropriate definition of the VFTable. >>>> - VTable->setName(VFTableName.str()); >>>> - } >>>> - >>>> - VFTable->setUnnamedAddr(true); >>>> - if (RD->hasAttr<DLLImportAttr>()) >>>> - >>>> VFTable->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass); >>>> - else if (RD->hasAttr<DLLExportAttr>()) >>>> - >>>> VFTable->setDLLStorageClass(llvm::GlobalValue::DLLExportStorageClass); >>>> - >>>> - llvm::GlobalValue::LinkageTypes VFTableLinkage = >>>> CGM.getVTableLinkage(RD); >>>> - if (VFTable != VTable) { >>>> - if >>>> (llvm::GlobalValue::isAvailableExternallyLinkage(VFTableLinkage)) { >>>> - // AvailableExternally implies that we grabbed the data from >>>> another >>>> - // executable. No need to stick the alias in a Comdat. >>>> - } else if >>>> (llvm::GlobalValue::isInternalLinkage(VFTableLinkage) || >>>> - llvm::GlobalValue::isWeakODRLinkage(VFTableLinkage) >>>> || >>>> - >>>> llvm::GlobalValue::isLinkOnceODRLinkage(VFTableLinkage)) { >>>> - // The alias is going to be dropped into a Comdat, no need >>>> to make it >>>> - // weak. >>>> - if (!llvm::GlobalValue::isInternalLinkage(VFTableLinkage)) >>>> - VFTableLinkage = llvm::GlobalValue::ExternalLinkage; >>>> - llvm::Comdat *C = >>>> - CGM.getModule().getOrInsertComdat(VFTable->getName()); >>>> - // We must indicate which VFTable is larger to support >>>> linking between >>>> - // translation units which do and do not have RTTI data. >>>> The largest >>>> - // VFTable contains the RTTI data; translation units which >>>> reference >>>> - // the smaller VFTable always reference it relative to the >>>> first >>>> - // virtual method. >>>> - C->setSelectionKind(llvm::Comdat::Largest); >>>> - VTable->setComdat(C); >>>> - } else { >>>> - llvm_unreachable("unexpected linkage for vftable!"); >>>> - } >>>> - } else { >>>> - if (llvm::GlobalValue::isWeakForLinker(VFTableLinkage)) >>>> - VTable->setComdat( >>>> - CGM.getModule().getOrInsertComdat(VTable->getName())); >>>> - } >>>> - VFTable->setLinkage(VFTableLinkage); >>>> - CGM.setGlobalVisibility(VFTable, RD); >>>> - VFTablesMap[ID] = VFTable; >>>> + uint64_t NumVTableSlots = >>>> + VTContext.getVFTableLayout(RD, VFPtr->FullOffsetInMDC) >>>> + .getNumVTableComponents(); >>>> + llvm::GlobalValue::LinkageTypes VTableLinkage = >>>> + VTableAliasIsRequred ? llvm::GlobalValue::PrivateLinkage : >>>> VFTableLinkage; >>>> + >>>> + StringRef VTableName = VTableAliasIsRequred ? StringRef() : >>>> VFTableName.str(); >>>> + >>>> + llvm::ArrayType *VTableType = >>>> + llvm::ArrayType::get(CGM.Int8PtrTy, NumVTableSlots); >>>> + >>>> + // Create a backing variable for the contents of VTable. The VTable >>>> may >>>> + // or may not include space for a pointer to RTTI data. >>>> + llvm::GlobalValue *VFTable; >>>> + VTable = new llvm::GlobalVariable(CGM.getModule(), VTableType, >>>> + /*isConstant=*/true, VTableLinkage, >>>> + /*Initializer=*/nullptr, >>>> VTableName); >>>> + VTable->setUnnamedAddr(true); >>>> + >>>> + llvm::Comdat *C = nullptr; >>>> + if (!VFTableComesFromAnotherTU && >>>> + (llvm::GlobalValue::isWeakForLinker(VFTableLinkage) || >>>> + (llvm::GlobalValue::isLocalLinkage(VFTableLinkage) && >>>> + VTableAliasIsRequred))) >>>> + C = CGM.getModule().getOrInsertComdat(VFTableName.str()); >>>> + >>>> + // Only insert a pointer into the VFTable for RTTI data if we are not >>>> + // importing it. We never reference the RTTI data directly so there >>>> is no >>>> + // need to make room for it. >>>> + if (VTableAliasIsRequred) { >>>> + llvm::Value *GEPIndices[] = {llvm::ConstantInt::get(CGM.IntTy, 0), >>>> + llvm::ConstantInt::get(CGM.IntTy, 1)}; >>>> + // Create a GEP which points just after the first entry in the >>>> VFTable, >>>> + // this should be the location of the first virtual method. >>>> + llvm::Constant *VTableGEP = >>>> + llvm::ConstantExpr::getInBoundsGetElementPtr(VTable, >>>> GEPIndices); >>>> + if (llvm::GlobalValue::isWeakForLinker(VFTableLinkage)) { >>>> + VFTableLinkage = llvm::GlobalValue::ExternalLinkage; >>>> + if (C) >>>> + C->setSelectionKind(llvm::Comdat::Largest); >>>> } >>>> - break; >>>> + VFTable = llvm::GlobalAlias::create( >>>> + >>>> cast<llvm::SequentialType>(VTableGEP->getType())->getElementType(), >>>> + /*AddressSpace=*/0, VFTableLinkage, VFTableName.str(), >>>> VTableGEP, >>>> + &CGM.getModule()); >>>> + VFTable->setUnnamedAddr(true); >>>> + } else { >>>> + // We don't need a GlobalAlias to be a symbol for the VTable if we >>>> won't >>>> + // be referencing any RTTI data. >>>> + // The GlobalVariable will end up being an appropriate definition >>>> of the >>>> + // VFTable. >>>> + VFTable = VTable; >>>> } >>>> + if (C) >>>> + VTable->setComdat(C); >>>> >>>> + if (RD->hasAttr<DLLImportAttr>()) >>>> + >>>> VFTable->setDLLStorageClass(llvm::GlobalValue::DLLImportStorageClass); >>>> + else if (RD->hasAttr<DLLExportAttr>()) >>>> + >>>> VFTable->setDLLStorageClass(llvm::GlobalValue::DLLExportStorageClass); >>>> + >>>> + VFTablesMap[ID] = VFTable; >>>> return VTable; >>>> } >>>> >>>> @@ -1811,9 +1806,6 @@ void MicrosoftCXXABI::emitVBTableDefinit >>>> llvm::ArrayType::get(CGM.IntTy, Offsets.size()); >>>> llvm::Constant *Init = llvm::ConstantArray::get(VBTableType, >>>> Offsets); >>>> GV->setInitializer(Init); >>>> - >>>> - // Set the right visibility. >>>> - CGM.setGlobalVisibility(GV, RD); >>>> } >>>> >>>> llvm::Value *MicrosoftCXXABI::performThisAdjustment(CodeGenFunction >>>> &CGF, >>>> >>>> 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=232680&r1=232679&r2=232680&view=diff >>>> >>>> ============================================================================== >>>> --- cfe/trunk/test/CodeGenCXX/microsoft-abi-vbtables.cpp (original) >>>> +++ cfe/trunk/test/CodeGenCXX/microsoft-abi-vbtables.cpp Wed Mar 18 >>>> 17:04:43 2015 >>>> @@ -528,3 +528,14 @@ D d; >>>> >>>> // CHECK-DAG: @"\01??_8D@Test29@@7BB@1@@" = linkonce_odr unnamed_addr >>>> constant [2 x i32] zeroinitializer >>>> } >>>> + >>>> +namespace Test30 { >>>> +struct A {}; >>>> +template <class> struct B : virtual A { >>>> + B() {} >>>> +}; >>>> + >>>> +extern template class B<int>; >>>> +template B<int>::B(); >>>> +// CHECK-DAG: @"\01??_8?$B@H@Test30@@7B@" = external unnamed_addr >>>> constant [2 x i32]{{$}} >>>> +} >>>> >>>> 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=232680&r1=232679&r2=232680&view=diff >>>> >>>> ============================================================================== >>>> --- cfe/trunk/test/CodeGenCXX/microsoft-abi-vftables.cpp (original) >>>> +++ cfe/trunk/test/CodeGenCXX/microsoft-abi-vftables.cpp Wed Mar 18 >>>> 17:04:43 2015 >>>> @@ -3,7 +3,6 @@ >>>> >>>> // RTTI-DAG: $"\01??_7S@@6B@" = comdat largest >>>> // RTTI-DAG: $"\01??_7V@@6B@" = comdat largest >>>> -// RTTI-DAG: $"\01??_7W@?A@@6B@" = comdat largest >>>> >>>> struct S { >>>> virtual ~S(); >>>> @@ -36,7 +35,18 @@ struct 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*)], comdat($"\01??_7W@?A@@6B@") >>>> -// RTTI-DAG: @"\01??_7W@?A@@6B@" = internal unnamed_addr alias >>>> getelementptr inbounds ([2 x i8*], [2 x i8*]* @2, i32 0, i32 1) >>>> +// 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*)] >>>> +// RTTI-DAG: @"\01??_7W@?A@@6B@" = internal unnamed_addr alias >>>> getelementptr inbounds ([2 x i8*], [2 x i8*]* [[VTABLE_W]], i32 0, i32 1) >>>> >>>> // NO-RTTI-DAG: @"\01??_7W@?A@@6B@" = internal unnamed_addr constant >>>> [1 x i8*] [i8* bitcast ({{.*}} @"\01??_GW@?A@@UAEPAXI@Z" to i8*)] >>>> + >>>> +struct X {}; >>>> +template <class> struct Y : virtual X { >>>> + Y() {} >>>> + virtual ~Y(); >>>> +}; >>>> + >>>> +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*] >>>> >>>> >>>> _______________________________________________ >>>> cfe-commits mailing list >>>> cfe-comm...@cs.uiuc.edu >>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>>> >>>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
