This is awesome. A nice step towards fixing pr16187 :-)
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
