<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Fri, Jan 27, 2017 at 2:51 PM Mehdi Amini <<a href="mailto:mehdi.amini@apple.com">mehdi.amini@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg">On Jan 27, 2017, at 2:43 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" class="gmail_msg" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br class="m_79804867233477206Apple-interchange-newline gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg"><br class="gmail_msg"><br class="gmail_msg"><div class="gmail_quote gmail_msg"><div dir="ltr" class="gmail_msg">On Fri, Jan 27, 2017 at 2:13 PM Mehdi Amini <<a href="mailto:mehdi.amini@apple.com" class="gmail_msg" target="_blank">mehdi.amini@apple.com</a>> wrote:<br class="gmail_msg"></div><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">CC Hans.<br class="gmail_msg">
<br class="gmail_msg">
This is not a regression (AFAICT), but this is a quality improvement, so may be worth considering in the 4.0 branch?<br class="gmail_msg"></blockquote><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">Perhaps - I'd generally err on the "if it's not a regression, don't hold the boat”.</div></div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg">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?</div></div></div></blockquote><div><br></div><div>*nod* fair point - though usually there's some amount of risk to any change & I tend to err on the "unless there's a known/likely need, choose not to take that risk". In this case we've all been using this setup for years without incident so while it's possibly a problem it seems somewhat abstract.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg">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?<br></div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><br class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><div class="gmail_msg"> 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 (<a href="https://llvm.org/svn/llvm-project/cfe/trunk@207451" class="gmail_msg" target="_blank">https://llvm.org/svn/llvm-project/cfe/trunk@207451</a>, <a href="http://reviews.llvm.org/D3515" class="gmail_msg" target="_blank">http://reviews.llvm.org/D3515</a>) & no one's noticed). But no big deal either way if other's feel like it ought to go in.</div></div></div></div></blockquote><div class="gmail_msg"><br class="gmail_msg"></div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg">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.</div><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">So up to the release manager :)</div></div></div></blockquote><div><br></div><div>*nod* sure sure - I'm not fussed either way :)</div><div><br></div><div>- Dave</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg"><br class="gmail_msg"></div><div class="gmail_msg">— </div></div></div><div style="word-wrap:break-word" class="gmail_msg"><div class="gmail_msg"><div class="gmail_msg">Mehdi</div><div class="gmail_msg"><br class="gmail_msg"></div><br class="gmail_msg"><blockquote type="cite" class="gmail_msg"><div class="gmail_msg"><div dir="ltr" class="gmail_msg"><div class="gmail_quote gmail_msg"><blockquote class="gmail_quote gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
> On Jan 27, 2017, at 2:04 PM, David Blaikie via Phabricator <<a href="mailto:reviews@reviews.llvm.org" class="gmail_msg" target="_blank">reviews@reviews.llvm.org</a>> wrote:<br class="gmail_msg">
><br class="gmail_msg">
> dblaikie created this revision.<br class="gmail_msg">
><br class="gmail_msg">
> As Mehdi put it, entities should either be<br class="gmail_msg">
> available_externally+weak_odr, or linkonce_odr+linkonce_odr. While some<br class="gmail_msg">
> functions are emitted a_e/weak, their local variables were emitted<br class="gmail_msg">
> a_e/linkonce_odr.<br class="gmail_msg">
><br class="gmail_msg">
> While it might be nice to emit them a_e/weak, the Itanium ABI (& best<br class="gmail_msg">
> guess at MSVC's behavior as well) requires the local to be<br class="gmail_msg">
> linkonce/linkonce.<br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
> <a href="https://reviews.llvm.org/D29233" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D29233</a><br class="gmail_msg">
><br class="gmail_msg">
> Files:<br class="gmail_msg">
> lib/AST/ASTContext.cpp<br class="gmail_msg">
> test/CodeGenCXX/explicit-instantiation.cpp<br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
> Index: test/CodeGenCXX/explicit-instantiation.cpp<br class="gmail_msg">
> ===================================================================<br class="gmail_msg">
> --- test/CodeGenCXX/explicit-instantiation.cpp<br class="gmail_msg">
> +++ test/CodeGenCXX/explicit-instantiation.cpp<br class="gmail_msg">
> @@ -5,6 +5,9 @@<br class="gmail_msg">
> // This check logically is attached to 'template int S<int>::i;' below.<br class="gmail_msg">
> // CHECK: @_ZN1SIiE1iE = weak_odr global i32<br class="gmail_msg">
><br class="gmail_msg">
> +// This check is logically attached to 'template int ExportedStaticLocal::f<int>()' below.<br class="gmail_msg">
> +// CHECK-OPT: @_ZZN19ExportedStaticLocal1fIiEEvvE1i = linkonce_odr global<br class="gmail_msg">
> +<br class="gmail_msg">
> template<typename T, typename U, typename Result><br class="gmail_msg">
> struct plus {<br class="gmail_msg">
> Result operator()(const T& t, const U& u) const;<br class="gmail_msg">
> @@ -153,3 +156,17 @@<br class="gmail_msg">
> template <typename T> void S<T>::g() {}<br class="gmail_msg">
> template <typename T> int S<T>::i;<br class="gmail_msg">
> template <typename T> void S<T>::S2::h() {}<br class="gmail_msg">
> +<br class="gmail_msg">
> +namespace ExportedStaticLocal {<br class="gmail_msg">
> +void sink(int&);<br class="gmail_msg">
> +template <typename T><br class="gmail_msg">
> +inline void f() {<br class="gmail_msg">
> + static int i;<br class="gmail_msg">
> + sink(i);<br class="gmail_msg">
> +}<br class="gmail_msg">
> +// See the check line at the top of the file.<br class="gmail_msg">
> +extern template void f<int>();<br class="gmail_msg">
> +void use() {<br class="gmail_msg">
> + f<int>();<br class="gmail_msg">
> +}<br class="gmail_msg">
> +}<br class="gmail_msg">
> Index: lib/AST/ASTContext.cpp<br class="gmail_msg">
> ===================================================================<br class="gmail_msg">
> --- lib/AST/ASTContext.cpp<br class="gmail_msg">
> +++ lib/AST/ASTContext.cpp<br class="gmail_msg">
> @@ -8892,22 +8892,27 @@<br class="gmail_msg">
> return GVA_Internal;<br class="gmail_msg">
><br class="gmail_msg">
> if (VD->isStaticLocal()) {<br class="gmail_msg">
> - GVALinkage StaticLocalLinkage = GVA_DiscardableODR;<br class="gmail_msg">
> const DeclContext *LexicalContext = VD->getParentFunctionOrMethod();<br class="gmail_msg">
> while (LexicalContext && !isa<FunctionDecl>(LexicalContext))<br class="gmail_msg">
> LexicalContext = LexicalContext->getLexicalParent();<br class="gmail_msg">
><br class="gmail_msg">
> - // Let the static local variable inherit its linkage from the nearest<br class="gmail_msg">
> - // enclosing function.<br class="gmail_msg">
> - if (LexicalContext)<br class="gmail_msg">
> - StaticLocalLinkage =<br class="gmail_msg">
> - Context.GetGVALinkageForFunction(cast<FunctionDecl>(LexicalContext));<br class="gmail_msg">
> -<br class="gmail_msg">
> - // GVA_StrongODR function linkage is stronger than what we need,<br class="gmail_msg">
> - // downgrade to GVA_DiscardableODR.<br class="gmail_msg">
> - // This allows us to discard the variable if we never end up needing it.<br class="gmail_msg">
> - return StaticLocalLinkage == GVA_StrongODR ? GVA_DiscardableODR<br class="gmail_msg">
> - : StaticLocalLinkage;<br class="gmail_msg">
> + // ObjC Blocks can create local variables that don't have a FunctionDecl<br class="gmail_msg">
> + // LexicalContext.<br class="gmail_msg">
> + if (!LexicalContext)<br class="gmail_msg">
> + return GVA_DiscardableODR;<br class="gmail_msg">
> +<br class="gmail_msg">
> + // Otherwise, let the static local variable inherit its linkage from the<br class="gmail_msg">
> + // nearest enclosing function.<br class="gmail_msg">
> + auto StaticLocalLinkage =<br class="gmail_msg">
> + Context.GetGVALinkageForFunction(cast<FunctionDecl>(LexicalContext));<br class="gmail_msg">
> +<br class="gmail_msg">
> + // Itanium ABI (& MSVC seems to do similarly) requires static locals in<br class="gmail_msg">
> + // inline functions to be emitted anywhere they're needed, even if the<br class="gmail_msg">
> + // function they are local to is emitted StrongODR/AvailableExternally.<br class="gmail_msg">
> + if (StaticLocalLinkage == GVA_StrongODR ||<br class="gmail_msg">
> + StaticLocalLinkage == GVA_AvailableExternally)<br class="gmail_msg">
> + return GVA_DiscardableODR;<br class="gmail_msg">
> + return StaticLocalLinkage;<br class="gmail_msg">
> }<br class="gmail_msg">
><br class="gmail_msg">
> // MSVC treats in-class initialized static data members as definitions.<br class="gmail_msg">
><br class="gmail_msg">
><br class="gmail_msg">
> <D29233.86118.patch><br class="gmail_msg">
<br class="gmail_msg">
</blockquote></div></div>
</div></blockquote></div></div></blockquote></div></div>