[cfe-commits] r116186 - in /cfe/trunk: lib/CodeGen/CGRTTI.cpp lib/CodeGen/CGVTables.cpp lib/CodeGen/CGVTables.h test/CodeGenCXX/vtable-linkage.cpp
Argyrios Kyrtzidis
kyrtzidis at apple.com
Mon Oct 11 14:03:18 PDT 2010
On 11 Oct 2010, at 13:42, Douglas Gregor wrote:
>
> On Oct 11, 2010, at 11:22 AM, Argyrios Kyrtzidis wrote:
>
>> $ cat t.cpp
>> template <typename T>
>> struct S {
>> virtual void m();
>> };
>>
>> void f() {
>> S<int> *s = new S<int>;
>> }
>>
>> $ clang -S -emit-llvm t.cpp -o -
>> [...]
>> @_ZTV1SIiE = external constant [3 x i8*]
>> [...]
>>
>> $ llvm-gcc -S -emit-llvm t.cpp -o -
>> [...]
>> %struct.__class_type_info_pseudo = type { %struct.__type_info_pseudo }
>> %struct.__type_info_pseudo = type { i8*, i8* }
>>
>> @_ZTV1SIiE = weak_odr constant [3 x i32 (...)*] [i32 (...)* null, i32 (...)* bitcast (%struct.__class_type_info_pseudo* @_ZTI1SIiE to i32 (...)*), i32 (...)* bitcast (void (%"struct.S<int>"*)* @_ZN1SIiE1mEv to i32 (...)*)], align 16 ; <[3 x i32 (...)*]*> [#uses=1]
>> @_ZTI1SIiE = weak_odr constant %struct.__class_type_info_pseudo { %struct.__type_info_pseudo { i8* inttoptr (i64 add (i64 ptrtoint ([0 x i32 (...)*]* @_ZTVN10__cxxabiv117__class_type_infoE to i64), i64 16) to i8*), i8* getelementptr inbounds ([6 x i8]* @_ZTS1SIiE, i64 0, i64 0) } }, align 16 ; <%struct.__class_type_info_pseudo*> [#uses=1]
>> @_ZTVN10__cxxabiv117__class_type_infoE = external constant [0 x i32 (...)*] ; <[0 x i32 (...)*]*> [#uses=1]
>> @_ZTS1SIiE = weak_odr constant [6 x i8] c"1SIiE\00" ; <[6 x i8]*> [#uses=2]
>> [..]
>>
>>
>> This resulted in an unresolved symbol linker error in Qt.
>
> This should only happen if the method S<int>::m is not instantiated anywhere else in Qt. Is that really the case?
To be more clear, when using clang, the specific executable being linked fails with linker error because the VTable symbol is not defined in the object files being linked.
With gcc, the VTable is emitted (as in the test case I posted) and the key function is resolved dynamically when the executable is run.
>
>> "C++ ABI 5.2.6 Instantiated Templates" says that template instantiations emit the virtual table in the object where instantiated, it doesn't mention the key function.
>
> Hrm, strange. I guess one could interpret this paragraph as overriding the general rule for emitting vtables, but that's rather unfortunate: it'll mean that we end up emitting vtables and RTTI in many more translation units than we do.
I don't think it will make that much difference in real code, since in the majority of cases the key function of templates has a body.
-Argiris
>
> - Doug
>
>
>> -Argyrios
>>
>> On Oct 11, 2010, at 11:10 AM, Douglas Gregor wrote:
>>
>>>
>>> On Oct 10, 2010, at 8:25 PM, Argyrios Kyrtzidis wrote:
>>>
>>>> Author: akirtzidis
>>>> Date: Sun Oct 10 22:25:57 2010
>>>> New Revision: 116186
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=116186&view=rev
>>>> Log:
>>>> Make sure the VTables for template instantiations are emitted even if the key function doesn't have a body.
>>>
>>> This doesn't seem right. The vtables should come with the key function, according to the C++ ABI, whenever/wherever it is instantiated. What prompted this change? I suggest reverting it until we understand the issue that you were seeing.
>>>
>>> - Doug
>>>
>>>> Modified:
>>>> cfe/trunk/lib/CodeGen/CGRTTI.cpp
>>>> cfe/trunk/lib/CodeGen/CGVTables.cpp
>>>> cfe/trunk/lib/CodeGen/CGVTables.h
>>>> cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp
>>>>
>>>> Modified: cfe/trunk/lib/CodeGen/CGRTTI.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGRTTI.cpp?rev=116186&r1=116185&r2=116186&view=diff
>>>> ==============================================================================
>>>> --- cfe/trunk/lib/CodeGen/CGRTTI.cpp (original)
>>>> +++ cfe/trunk/lib/CodeGen/CGRTTI.cpp Sun Oct 10 22:25:57 2010
>>>> @@ -270,8 +270,9 @@
>>>> /// the given type exists somewhere else, and that we should not emit the type
>>>> /// information in this translation unit. Assumes that it is not a
>>>> /// standard-library type.
>>>> -static bool ShouldUseExternalRTTIDescriptor(ASTContext &Context,
>>>> - QualType Ty) {
>>>> +static bool ShouldUseExternalRTTIDescriptor(CodeGenModule &CGM, QualType Ty) {
>>>> + ASTContext &Context = CGM.getContext();
>>>> +
>>>> // If RTTI is disabled, don't consider key functions.
>>>> if (!Context.getLangOptions().RTTI) return false;
>>>>
>>>> @@ -283,13 +284,7 @@
>>>> if (!RD->isDynamicClass())
>>>> return false;
>>>>
>>>> - // Get the key function.
>>>> - const CXXMethodDecl *KeyFunction = Context.getKeyFunction(RD);
>>>> - if (KeyFunction && !KeyFunction->hasBody()) {
>>>> - // The class has a key function, but it is not defined in this translation
>>>> - // unit, so we should use the external descriptor for it.
>>>> - return true;
>>>> - }
>>>> + return !CGM.getVTables().ShouldEmitVTableInThisTU(RD);
>>>> }
>>>>
>>>> return false;
>>>> @@ -528,8 +523,7 @@
>>>>
>>>> // Check if there is already an external RTTI descriptor for this type.
>>>> bool IsStdLib = IsStandardLibraryRTTIDescriptor(Ty);
>>>> - if (!Force &&
>>>> - (IsStdLib || ShouldUseExternalRTTIDescriptor(CGM.getContext(), Ty)))
>>>> + if (!Force && (IsStdLib || ShouldUseExternalRTTIDescriptor(CGM, Ty)))
>>>> return GetAddrOfExternalRTTIDescriptor(Ty);
>>>>
>>>> // Emit the standard library with external linkage.
>>>>
>>>> Modified: cfe/trunk/lib/CodeGen/CGVTables.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.cpp?rev=116186&r1=116185&r2=116186&view=diff
>>>> ==============================================================================
>>>> --- cfe/trunk/lib/CodeGen/CGVTables.cpp (original)
>>>> +++ cfe/trunk/lib/CodeGen/CGVTables.cpp Sun Oct 10 22:25:57 2010
>>>> @@ -2336,6 +2336,27 @@
>>>> NumVirtualFunctionPointers[RD] = CurrentIndex;
>>>> }
>>>>
>>>> +bool CodeGenVTables::ShouldEmitVTableInThisTU(const CXXRecordDecl *RD) {
>>>> + assert(RD->isDynamicClass() && "Non dynamic classes have no VTable.");
>>>> +
>>>> + TemplateSpecializationKind TSK = RD->getTemplateSpecializationKind();
>>>> + if (TSK == TSK_ExplicitInstantiationDeclaration)
>>>> + return false;
>>>> +
>>>> + const CXXMethodDecl *KeyFunction = CGM.getContext().getKeyFunction(RD);
>>>> + if (!KeyFunction)
>>>> + return true;
>>>> +
>>>> + // Itanium C++ ABI, 5.2.6 Instantiated Templates:
>>>> + // An instantiation of a class template requires:
>>>> + // - In the object where instantiated, the virtual table...
>>>> + if (TSK == TSK_ImplicitInstantiation ||
>>>> + TSK == TSK_ExplicitInstantiationDefinition)
>>>> + return true;
>>>> +
>>>> + return KeyFunction->hasBody();
>>>> +}
>>>> +
>>>> uint64_t CodeGenVTables::getNumVirtualFunctionPointers(const CXXRecordDecl *RD) {
>>>> llvm::DenseMap<const CXXRecordDecl *, uint64_t>::iterator I =
>>>> NumVirtualFunctionPointers.find(RD);
>>>> @@ -2703,9 +2724,7 @@
>>>>
>>>> // We may need to generate a definition for this vtable.
>>>> if (RequireVTable && !Entry.getInt()) {
>>>> - if (!isKeyFunctionInAnotherTU(CGM.getContext(), RD) &&
>>>> - RD->getTemplateSpecializationKind()
>>>> - != TSK_ExplicitInstantiationDeclaration)
>>>> + if (ShouldEmitVTableInThisTU(RD))
>>>> CGM.DeferredVTables.push_back(RD);
>>>>
>>>> Entry.setInt(true);
>>>>
>>>> Modified: cfe/trunk/lib/CodeGen/CGVTables.h
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGVTables.h?rev=116186&r1=116185&r2=116186&view=diff
>>>> ==============================================================================
>>>> --- cfe/trunk/lib/CodeGen/CGVTables.h (original)
>>>> +++ cfe/trunk/lib/CodeGen/CGVTables.h Sun Oct 10 22:25:57 2010
>>>> @@ -295,14 +295,9 @@
>>>> CodeGenVTables(CodeGenModule &CGM)
>>>> : CGM(CGM) { }
>>>>
>>>> - // isKeyFunctionInAnotherTU - True if this record has a key function and it is
>>>> - // in another translation unit.
>>>> - static bool isKeyFunctionInAnotherTU(ASTContext &Context,
>>>> - const CXXRecordDecl *RD) {
>>>> - assert (RD->isDynamicClass() && "Non dynamic classes have no key.");
>>>> - const CXXMethodDecl *KeyFunction = Context.getKeyFunction(RD);
>>>> - return KeyFunction && !KeyFunction->hasBody();
>>>> - }
>>>> + /// \brief True if the VTable of this record must be emitted in the
>>>> + /// translation unit.
>>>> + bool ShouldEmitVTableInThisTU(const CXXRecordDecl *RD);
>>>>
>>>> /// needsVTTParameter - Return whether the given global decl needs a VTT
>>>> /// parameter, which it does if it's a base constructor or destructor with
>>>>
>>>> Modified: cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp
>>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp?rev=116186&r1=116185&r2=116186&view=diff
>>>> ==============================================================================
>>>> --- cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp (original)
>>>> +++ cfe/trunk/test/CodeGenCXX/vtable-linkage.cpp Sun Oct 10 22:25:57 2010
>>>> @@ -197,3 +197,18 @@
>>>> template <typename T>
>>>> void G<T>::f0() {}
>>>> void G_f0() { new G<int>(); }
>>>> +
>>>> +// RUN: FileCheck --check-prefix=CHECK-H %s < %t
>>>> +
>>>> +// H<int> has a key function without a body but it's a template instantiation
>>>> +// so its VTable must be emmitted.
>>>> +// CHECK-H: @_ZTV1HIiE = weak_odr constant
>>>> +template <typename T>
>>>> +class H {
>>>> +public:
>>>> + virtual ~H();
>>>> +};
>>>> +
>>>> +void use_H() {
>>>> + H<int> h;
>>>> +}
>>>>
>>>>
>>>> _______________________________________________
>>>> cfe-commits mailing list
>>>> cfe-commits at cs.uiuc.edu
>>>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>>
>>
>
More information about the cfe-commits
mailing list