r225570 - Don't emit implicit template instantiations eagerly (PR21718)
Hans Wennborg
hans at chromium.org
Mon Jan 12 13:50:13 PST 2015
On Mon, Jan 12, 2015 at 11:40 AM, Rafael Espíndola
<rafael.espindola at gmail.com> wrote:
> This is awesome.
Thanks!
> A nice step towards fixing pr16187 :-)
I'm not sure I fully understand the details of that bug. Are you
thinking we could solve it if we delay more kinds of declarations so
the visibility gets computed at the end?
> On 9 January 2015 at 20:19, Hans Wennborg <hans at hanshq.net> wrote:
>> Author: hans
>> Date: Fri Jan 9 19:19:48 2015
>> New Revision: 225570
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=225570&view=rev
>> Log:
>> Don't emit implicit template instantiations eagerly (PR21718)
>>
>> Their linkage can change if they are later explicitly instantiated. We would
>> previously emit such functions eagerly (as opposed to lazily on first use) if
>> they have a 'dllexport' or 'used' attribute, and fail an assert when hitting the
>> explicit instantiation.
>>
>> This is achieved by replacing the old CodeGenModule::MayDeferGeneration() method
>> with two new ones: MustBeEmitted() and MayBeEmittedEagerly().
>>
>> Differential Revision: http://reviews.llvm.org/D6674
>>
>> Modified:
>> cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>> cfe/trunk/lib/CodeGen/CodeGenModule.h
>> cfe/trunk/test/CodeGenCXX/dllexport.cpp
>> cfe/trunk/test/CodeGenCXX/explicit-instantiation.cpp
>>
>> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.cpp?rev=225570&r1=225569&r2=225570&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CodeGenModule.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/CodeGenModule.cpp Fri Jan 9 19:19:48 2015
>> @@ -1102,14 +1102,18 @@ void CodeGenModule::EmitDeferred() {
>> llvm::GlobalValue *GV = G.GV;
>> DeferredDeclsToEmit.pop_back();
>>
>> - assert(GV == GetGlobalValue(getMangledName(D)));
>> + assert(!GV || GV == GetGlobalValue(getMangledName(D)));
>> + if (!GV)
>> + GV = GetGlobalValue(getMangledName(D));
>> +
>> +
>> // Check to see if we've already emitted this. This is necessary
>> // for a couple of reasons: first, decls can end up in the
>> // deferred-decls queue multiple times, and second, decls can end
>> // up with definitions in unusual ways (e.g. by an extern inline
>> // function acquiring a strong function redefinition). Just
>> // ignore these cases.
>> - if(!GV->isDeclaration())
>> + if (GV && !GV->isDeclaration())
>> continue;
>>
>> // Otherwise, emit the definition and move on to the next one.
>> @@ -1234,12 +1238,22 @@ bool CodeGenModule::isInSanitizerBlackli
>> return false;
>> }
>>
>> -bool CodeGenModule::MayDeferGeneration(const ValueDecl *Global) {
>> +bool CodeGenModule::MustBeEmitted(const ValueDecl *Global) {
>> // Never defer when EmitAllDecls is specified.
>> if (LangOpts.EmitAllDecls)
>> - return false;
>> + return true;
>> +
>> + return getContext().DeclMustBeEmitted(Global);
>> +}
>>
>> - return !getContext().DeclMustBeEmitted(Global);
>> +bool CodeGenModule::MayBeEmittedEagerly(const ValueDecl *Global) {
>> + if (const auto *FD = dyn_cast<FunctionDecl>(Global))
>> + if (FD->getTemplateSpecializationKind() == TSK_ImplicitInstantiation)
>> + // Implicit template instantiations may change linkage if they are later
>> + // explicitly instantiated, so they should not be emitted eagerly.
>> + return false;
>> +
>> + return true;
>> }
>>
>> llvm::Constant *CodeGenModule::GetAddrOfUuidDescriptor(
>> @@ -1348,9 +1362,10 @@ void CodeGenModule::EmitGlobal(GlobalDec
>> return;
>> }
>>
>> - // Defer code generation when possible if this is a static definition, inline
>> - // function etc. These we only want to emit if they are used.
>> - if (!MayDeferGeneration(Global)) {
>> + // Defer code generation to first use when possible, e.g. if this is an inline
>> + // function. If the global must always be emitted, do it eagerly if possible
>> + // to benefit from cache locality.
>> + if (MustBeEmitted(Global) && MayBeEmittedEagerly(Global)) {
>> // Emit the definition if it can't be deferred.
>> EmitGlobalDefinition(GD);
>> return;
>> @@ -1363,13 +1378,16 @@ void CodeGenModule::EmitGlobal(GlobalDec
>> DelayedCXXInitPosition[Global] = CXXGlobalInits.size();
>> CXXGlobalInits.push_back(nullptr);
>> }
>> -
>> - // If the value has already been used, add it directly to the
>> - // DeferredDeclsToEmit list.
>> +
>> StringRef MangledName = getMangledName(GD);
>> - if (llvm::GlobalValue *GV = GetGlobalValue(MangledName))
>> + if (llvm::GlobalValue *GV = GetGlobalValue(MangledName)) {
>> + // The value has already been used and should therefore be emitted.
>> addDeferredDeclToEmit(GV, GD);
>> - else {
>> + } else if (MustBeEmitted(Global)) {
>> + // The value must be emitted, but cannot be emitted eagerly.
>> + assert(!MayBeEmittedEagerly(Global));
>> + addDeferredDeclToEmit(/*GV=*/nullptr, GD);
>> + } else {
>> // Otherwise, remember that we saw a deferred decl with this name. The
>> // first use of the mangled name will cause it to move into
>> // DeferredDeclsToEmit.
>> @@ -1843,7 +1861,7 @@ CodeGenModule::CreateRuntimeVariable(llv
>> void CodeGenModule::EmitTentativeDefinition(const VarDecl *D) {
>> assert(!D->getInit() && "Cannot emit definite definitions here!");
>>
>> - if (MayDeferGeneration(D)) {
>> + if (!MustBeEmitted(D)) {
>> // If we have not seen a reference to this variable yet, place it
>> // into the deferred declarations table to be emitted if needed
>> // later.
>>
>> Modified: cfe/trunk/lib/CodeGen/CodeGenModule.h
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenModule.h?rev=225570&r1=225569&r2=225570&view=diff
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/CodeGenModule.h (original)
>> +++ cfe/trunk/lib/CodeGen/CodeGenModule.h Fri Jan 9 19:19:48 2015
>> @@ -1200,9 +1200,15 @@ private:
>> /// Emits the initializer for a uuidof string.
>> llvm::Constant *EmitUuidofInitializer(StringRef uuidstr);
>>
>> - /// Determine if the given decl can be emitted lazily; this is only relevant
>> - /// for definitions. The given decl must be either a function or var decl.
>> - bool MayDeferGeneration(const ValueDecl *D);
>> + /// Determine whether the definition must be emitted; if this returns \c
>> + /// false, the definition can be emitted lazily if it's used.
>> + bool MustBeEmitted(const ValueDecl *D);
>> +
>> + /// Determine whether the definition can be emitted eagerly, or should be
>> + /// delayed until the end of the translation unit. This is relevant for
>> + /// definitions whose linkage can change, e.g. implicit function instantions
>> + /// which may later be explicitly instantiated.
>> + bool MayBeEmittedEagerly(const ValueDecl *D);
>>
>> /// Check whether we can use a "simpler", more core exceptions personality
>> /// function.
>>
>> Modified: cfe/trunk/test/CodeGenCXX/dllexport.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllexport.cpp?rev=225570&r1=225569&r2=225570&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/CodeGenCXX/dllexport.cpp (original)
>> +++ cfe/trunk/test/CodeGenCXX/dllexport.cpp Fri Jan 9 19:19:48 2015
>> @@ -615,6 +615,21 @@ NonExportedBaseClass::~NonExportedBaseCl
>> struct __declspec(dllexport) ExportedDerivedClass : NonExportedBaseClass {};
>> // M32-DAG: weak_odr dllexport x86_thiscallcc void @"\01??1ExportedDerivedClass@@UAE at XZ"
>>
>> +// Do not assert about generating code for constexpr functions twice during explicit instantiation (PR21718).
>> +template <typename T> struct ExplicitInstConstexprMembers {
>> + // Copy assignment operator
>> + // M32-DAG: define weak_odr dllexport x86_thiscallcc dereferenceable(1) %struct.ExplicitInstConstexprMembers* @"\01??4?$ExplicitInstConstexprMembers at X@@QAEAAU0 at ABU0@@Z"
>> +
>> + constexpr ExplicitInstConstexprMembers() {}
>> + // M32-DAG: define weak_odr dllexport x86_thiscallcc %struct.ExplicitInstConstexprMembers* @"\01??0?$ExplicitInstConstexprMembers at X@@QAE at XZ"
>> +
>> + ExplicitInstConstexprMembers(const ExplicitInstConstexprMembers&) = default;
>> + // M32-DAG: define weak_odr dllexport x86_thiscallcc %struct.ExplicitInstConstexprMembers* @"\01??0?$ExplicitInstConstexprMembers at X@@QAE at ABU0@@Z"
>> +
>> + constexpr int f() const { return 42; }
>> + // M32-DAG: define weak_odr dllexport x86_thiscallcc i32 @"\01?f@?$ExplicitInstConstexprMembers at X@@QBEHXZ"
>> +};
>> +template struct __declspec(dllexport) ExplicitInstConstexprMembers<void>;
>>
>> //===----------------------------------------------------------------------===//
>> // Classes with template base classes
>>
>> Modified: cfe/trunk/test/CodeGenCXX/explicit-instantiation.cpp
>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/explicit-instantiation.cpp?rev=225570&r1=225569&r2=225570&view=diff
>> ==============================================================================
>> --- cfe/trunk/test/CodeGenCXX/explicit-instantiation.cpp (original)
>> +++ cfe/trunk/test/CodeGenCXX/explicit-instantiation.cpp Fri Jan 9 19:19:48 2015
>> @@ -90,6 +90,19 @@ namespace LateInstantiation {
>> // CHECK-OPT: define available_externally i32 @_ZN17LateInstantiation1fIiEEiv(
>> }
>>
>> +namespace PR21718 {
>> +// The linkage of a used constexpr member function can change from linkonce_odr
>> +// to weak_odr after explicit instantiation without errors about defining the
>> +// same function twice.
>> +template <typename T>
>> +struct S {
>> +// CHECK-LABEL: define weak_odr i32 @_ZN7PR217181SIiE1fEv
>> + __attribute__((used)) constexpr int f() { return 0; }
>> +};
>> +int g() { return S<int>().f(); }
>> +template struct S<int>;
>> +}
>> +
>> // Check that we emit definitions from explicit instantiations even when they
>> // occur prior to the definition itself.
>> template <typename T> struct S {
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> _______________________________________________
> 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