r232680 - MS ABI: Don't try to emit VF/VB-Tables for extern class templates
Nico Weber via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 18 05:45:02 PST 2019
(-old cfe-commits, +new cfe-commits)
On Fri, Jan 18, 2019 at 8:21 AM Nico Weber <thakis at 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.majnemer at 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 <thakis at 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.majnemer at 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 at Test29@@7BB at 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 at 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 at 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 at 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 at 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 at H@@6B@" = external unnamed_addr constant [1
>>>> x i8*]
>>>> +// NO-RTTI-DAG: @"\01??_7?$Y at H@@6B@" = external unnamed_addr constant
>>>> [1 x i8*]
>>>>
>>>>
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190118/18fbe68a/attachment-0001.html>
More information about the cfe-commits
mailing list