<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Wed, Aug 10, 2016 at 5:39 PM Eric Fiselier <<a href="mailto:eric@efcs.ca">eric@efcs.ca</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Mon, Aug 8, 2016 at 9:32 AM, David Blaikie via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr">I'm still (as per another similar thread) a bit concerned this is working around a compiler optimizer bug - I'd love to see a standalone example of this behavior (that adding the inline keyword to an already inline/available definition is what's causing the inlining) so we can look at what the compiler might be doing wrong (& consider fixing that as the root cause instead of having to change every instance of this problem) - preferably an example without std::string/headers/etc (so, preprocessed and reduced).<br></div></blockquote><div><br></div></div></div></div><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Normally I would agree that this is an optimizer bug, but std::string is externally instantiated.</div><div>It seems reasonable for Clang *not* to inline externally instantiated templates unless requested.</div></div></div></div></blockquote><div><br></div><div>Sort of - but it does have the ability to create what it describes as "available externally" definitions that can be used for inlining (& other interprocedural analysis), but don't need to be emitted into the final object if a call to them remains (ie: that call can be resolved by an external definition)<br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>Simply removing the extern template declaration will resolve these inlining issues.</div><div><br></div><div>Here is the standalone example you requested: <a href="https://godbolt.org/g/aVwEMR" target="_blank">https://godbolt.org/g/aVwEMR</a></div></div></div></div></blockquote><div><br></div><div>Simplifying the test case a bit (while still preserving the behavior):<br><br><pre style="top: -99px;"><span style="line-height:normal">template <class T>
struct string {
~string();
};
template <class T>
string<T>::~string() {}
// Remove this line to see the difference
#ifdef EXTERN
extern template struct string<char>;
#endif
template <class T>
void sink(string<T>);
int main() {
sink(string<char>());
}</span></pre></div><div>I still see the big difference (the use of exception handling to ensure the dtor is called, etc) in this example when EXTERN is defined.<br><br>If I move the definition of ~string into the string class definition, the difference goes away - the dtor call and associated exception handling is optimized away.<br><br>& yes, adding 'inline' to the out-of-line definition produces the same behavior. And in both cases Clang does the right thing of using an available_externally definition, not a comdat/linkonce_odr definition. (this is why I was confused by the original bug report - the existence or absence of a definition of the function in the resulting IR or object file isn't proof that the compiler didn't have the information necessary to optimize the code)<br><br>Perhaps this is a language limitation that the out of line definition can't be implicitly instantiated in this situation? Or we try not to do so for some reason.<br><br>(Richard?)<br><br>I take it the always_inline part of this is necessary for the whole libc++ ABI strategy? (something I've never quite wrapped my head around the details of)</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div dir="ltr"><br>But up to you folks maintaining libc++ whether this goes in of course, just my 2c.</div><br><div class="gmail_quote"><div><div class="m_4403945042718145257h5"><div dir="ltr">On Wed, Aug 3, 2016 at 5:23 AM Aditya Kumar via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>> wrote:<br></div></div></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div><div class="m_4403945042718145257h5">hiraditya added a comment.<br>
<br>
In <a href="https://reviews.llvm.org/D22782#504416" rel="noreferrer" target="_blank">https://reviews.llvm.org/D22782#504416</a>, @EricWF wrote:<br>
<br>
> The change itself LGTM, although we probably want to inline the forward/input iterator __init's as well.<br>
><br>
> However I would like to see a small benchmark that demonstrates the performance change. Please try and write the benchmark using Google Benchmark.<br>
> Some helpful links:<br>
><br>
> - <a href="http://libcxx.llvm.org/docs/TestingLibcxx.html#building-benchmarks" rel="noreferrer" target="_blank">http://libcxx.llvm.org/docs/TestingLibcxx.html#building-benchmarks</a><br>
> - <a href="http://github.com/google/benchmark" rel="noreferrer" target="_blank">http://github.com/google/benchmark</a><br>
<br>
<br>
Sure,<br>
We'll come up with a synthetic benchmark to expose performance improvements.<br>
<br>
Thanks,<br>
<br>
<br>
<a href="https://reviews.llvm.org/D22782" rel="noreferrer" target="_blank">https://reviews.llvm.org/D22782</a><br>
<br>
<br>
<br></div></div><span>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</span></blockquote></div>
<br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div></div></div></blockquote></div></div>