[PATCH] D22782: Added 'inline' attribute to __init to inline the basic_string's constructor

[David Blaikie via cfe-commits]

On Wed, Aug 10, 2016 at 5:39 PM Eric Fiselier <eric at efcs.ca> wrote:

> On Mon, Aug 8, 2016 at 9:32 AM, David Blaikie via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> 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).
>>
>
> Normally I would agree that this is an optimizer bug, but std::string is
> externally instantiated.
> It seems reasonable for Clang *not* to inline externally instantiated
> templates unless requested.
>

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)


> Simply removing the extern template declaration will resolve these
> inlining issues.
>
> Here is the standalone example you requested: https://godbolt.org/g/aVwEMR
>

Simplifying the test case a bit (while still preserving the behavior):

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>());
}

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.

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.

& 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)

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.

(Richard?)

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)

>
>
>>
>> But up to you folks maintaining libc++ whether this goes in of course,
>> just my 2c.
>>
>> On Wed, Aug 3, 2016 at 5:23 AM Aditya Kumar via cfe-commits <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>> hiraditya added a comment.
>>>
>>> In https://reviews.llvm.org/D22782#504416, @EricWF wrote:
>>>
>>> > The change itself LGTM, although we probably want to inline the
>>> forward/input iterator __init's as well.
>>> >
>>> > However I would like to see a small benchmark that demonstrates the
>>> performance change. Please try and write the benchmark using Google
>>> Benchmark.
>>> >  Some helpful links:
>>> >
>>> > - http://libcxx.llvm.org/docs/TestingLibcxx.html#building-benchmarks
>>> > - http://github.com/google/benchmark
>>>
>>>
>>> Sure,
>>> We'll come up with a synthetic benchmark to expose performance
>>> improvements.
>>>
>>> Thanks,
>>>
>>>
>>> https://reviews.llvm.org/D22782
>>>
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160811/66cb41fa/attachment.html>