> On Jan 27, 2017, at 2:43 PM, David Blaikie <dblai...@gmail.com> wrote: > > > > On Fri, Jan 27, 2017 at 2:13 PM Mehdi Amini <mehdi.am...@apple.com > <mailto:mehdi.am...@apple.com>> wrote: > CC Hans. > > This is not a regression (AFAICT), but this is a quality improvement, so may > be worth considering in the 4.0 branch? > > Perhaps - I'd generally err on the "if it's not a regression, don't hold the > boat”.
I agree with “don’t hold the boat”, but I usually understand by that “don’t delay the release or change the schedule”, do you mean something else? It is likely that the bar should get higher as we get closer to the release, I don’t know if the branching point should be considered the point where we stop taking changes that aren’t regression fixes? > LLVM's been this way for a while (good question as to how long - looks like > at the latest it was introduced in April 2014 > (https://llvm.org/svn/llvm-project/cfe/trunk@207451 > <https://llvm.org/svn/llvm-project/cfe/trunk@207451>, > http://reviews.llvm.org/D3515 <http://reviews.llvm.org/D3515>) & no one's > noticed). But no big deal either way if other's feel like it ought to go in. I’m (very) biased by the way we (Apple) organize our release: we will cherry-pick any bug fixes (and other “quality improvements”), regression or not, for months after the branching point. So up to the release manager :) — Mehdi > > On Jan 27, 2017, at 2:04 PM, David Blaikie via Phabricator > > <revi...@reviews.llvm.org <mailto:revi...@reviews.llvm.org>> wrote: > > > > dblaikie created this revision. > > > > As Mehdi put it, entities should either be > > available_externally+weak_odr, or linkonce_odr+linkonce_odr. While some > > functions are emitted a_e/weak, their local variables were emitted > > a_e/linkonce_odr. > > > > While it might be nice to emit them a_e/weak, the Itanium ABI (& best > > guess at MSVC's behavior as well) requires the local to be > > linkonce/linkonce. > > > > > > https://reviews.llvm.org/D29233 <https://reviews.llvm.org/D29233> > > > > Files: > > lib/AST/ASTContext.cpp > > test/CodeGenCXX/explicit-instantiation.cpp > > > > > > Index: test/CodeGenCXX/explicit-instantiation.cpp > > =================================================================== > > --- test/CodeGenCXX/explicit-instantiation.cpp > > +++ test/CodeGenCXX/explicit-instantiation.cpp > > @@ -5,6 +5,9 @@ > > // This check logically is attached to 'template int S<int>::i;' below. > > // CHECK: @_ZN1SIiE1iE = weak_odr global i32 > > > > +// This check is logically attached to 'template int > > ExportedStaticLocal::f<int>()' below. > > +// CHECK-OPT: @_ZZN19ExportedStaticLocal1fIiEEvvE1i = linkonce_odr global > > + > > template<typename T, typename U, typename Result> > > struct plus { > > Result operator()(const T& t, const U& u) const; > > @@ -153,3 +156,17 @@ > > template <typename T> void S<T>::g() {} > > template <typename T> int S<T>::i; > > template <typename T> void S<T>::S2::h() {} > > + > > +namespace ExportedStaticLocal { > > +void sink(int&); > > +template <typename T> > > +inline void f() { > > + static int i; > > + sink(i); > > +} > > +// See the check line at the top of the file. > > +extern template void f<int>(); > > +void use() { > > + f<int>(); > > +} > > +} > > Index: lib/AST/ASTContext.cpp > > =================================================================== > > --- lib/AST/ASTContext.cpp > > +++ lib/AST/ASTContext.cpp > > @@ -8892,22 +8892,27 @@ > > return GVA_Internal; > > > > if (VD->isStaticLocal()) { > > - GVALinkage StaticLocalLinkage = GVA_DiscardableODR; > > const DeclContext *LexicalContext = VD->getParentFunctionOrMethod(); > > while (LexicalContext && !isa<FunctionDecl>(LexicalContext)) > > LexicalContext = LexicalContext->getLexicalParent(); > > > > - // Let the static local variable inherit its linkage from the nearest > > - // enclosing function. > > - if (LexicalContext) > > - StaticLocalLinkage = > > - > > Context.GetGVALinkageForFunction(cast<FunctionDecl>(LexicalContext)); > > - > > - // GVA_StrongODR function linkage is stronger than what we need, > > - // downgrade to GVA_DiscardableODR. > > - // This allows us to discard the variable if we never end up needing > > it. > > - return StaticLocalLinkage == GVA_StrongODR ? GVA_DiscardableODR > > - : StaticLocalLinkage; > > + // ObjC Blocks can create local variables that don't have a > > FunctionDecl > > + // LexicalContext. > > + if (!LexicalContext) > > + return GVA_DiscardableODR; > > + > > + // Otherwise, let the static local variable inherit its linkage from > > the > > + // nearest enclosing function. > > + auto StaticLocalLinkage = > > + > > Context.GetGVALinkageForFunction(cast<FunctionDecl>(LexicalContext)); > > + > > + // Itanium ABI (& MSVC seems to do similarly) requires static locals in > > + // inline functions to be emitted anywhere they're needed, even if the > > + // function they are local to is emitted StrongODR/AvailableExternally. > > + if (StaticLocalLinkage == GVA_StrongODR || > > + StaticLocalLinkage == GVA_AvailableExternally) > > + return GVA_DiscardableODR; > > + return StaticLocalLinkage; > > } > > > > // MSVC treats in-class initialized static data members as definitions. > > > > > > <D29233.86118.patch> >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
