On Mon, Jan 12, 2015 at 11:40 AM, Rafael Espíndola <[email protected]> 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 <[email protected]> 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@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@X@@QAEAAU0@ABU0@@Z" >> + >> + constexpr ExplicitInstConstexprMembers() {} >> + // M32-DAG: define weak_odr dllexport x86_thiscallcc >> %struct.ExplicitInstConstexprMembers* >> @"\01??0?$ExplicitInstConstexprMembers@X@@QAE@XZ" >> + >> + ExplicitInstConstexprMembers(const ExplicitInstConstexprMembers&) = >> default; >> + // M32-DAG: define weak_odr dllexport x86_thiscallcc >> %struct.ExplicitInstConstexprMembers* >> @"\01??0?$ExplicitInstConstexprMembers@X@@QAE@ABU0@@Z" >> + >> + constexpr int f() const { return 42; } >> + // M32-DAG: define weak_odr dllexport x86_thiscallcc i32 >> @"\01?f@?$ExplicitInstConstexprMembers@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 >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
