[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-dev
Subject:    Re: [cfe-dev] [RFC] AlwaysInline codegen
From:       Evgenii Stepanov via cfe-dev <cfe-dev () lists ! llvm ! org>
Date:       2015-08-28 17:55:54
Message-ID: CABMLtrihO55NEkms8_hi6xpmSAV_9PvEw0vMOVJeYAyRRqPk7g () mail ! gmail ! com
[Download RAW message or body]

Are there any concerns remaining?
I'd like to go ahead with http://reviews.llvm.org/D12087

On Fri, Aug 21, 2015 at 3:40 PM, Richard Smith <richard@metafoo.co.uk> wrote:
> On Fri, Aug 21, 2015 at 1:23 PM, John McCall <rjmccall@apple.com> wrote:
>>
>> On Aug 20, 2015, at 7:36 PM, Richard Smith <richard@metafoo.co.uk> wrote:
>>
>> On Thu, Aug 20, 2015 at 7:17 PM, John McCall <rjmccall@apple.com> wrote:
>>>
>>> > On Aug 20, 2015, at 5:19 PM, Evgenii Stepanov via cfe-dev
>>> > <cfe-dev@lists.llvm.org> wrote:
>>> > Hi,
>>> >
>>> > There is a problem with the handling of alwaysinline functions in
>>> > Clang: they are not always inlined. AFAIK, this may only happen when
>>> > the caller is in the dead code, but then we don't always successfully
>>> > remove all dead code.
>>> >
>>> > Because of this, we may end up emitting an undefined reference for an
>>> > "inline __attribute__((always_inline))" function. Libc++ relies on the
>>> > compiler never doing that - it has lots of functions in the headers
>>> > marked this way and does _not_ export them from libc++.so.
>>> >
>>> > Current implementation in clang emits alwaysinline+inline functions as
>>> > available_externally definitions. The inliner is an SCC pass, and as
>>> > such it does not process unreachable functions at all. This means that
>>> > AlwaysInliner may leave some alwaysinline functions not inlined. If
>>> > such function has an available_externally linkage, it is not emitted
>>> > into the binary, and all calls to it are emitted as undefined symbol
>>> > references.
>>> >
>>> > Some time ago I've made an attempt to add a DCE pass before the
>>> > AlwaysInliner pass to fix this. Thst
>>> > (a) caused a big churn in the existing tests
>>> > (b) must be done at -O0 as well, which is probably undesirable and
>>> > could inflate compilation time
>>> > (c) feels like patching over a bigger problem.
>>> >
>>> > The following, better, idea was suggested by Chandler Carruth and
>>> > Richard Smith.
>>> >
>>> > Instead of emitting an available_externally definition for an
>>> > alwaysinline function, we emit a pair of
>>> > 1. internal alwaysinline definition (let's call it F.inlinefunction -
>>> > it demangles nicely)
>>> > 2a. A stub F() { musttail call F.inlinefunction }
>>> >  -- or --
>>> > 2b. A declaration of F.
>>>
>>> I have no idea why always_inline function definitions are being marked as
>>> available_externally.  There is zero reason to think that they're
>>> actually
>>> available externally, and there's zero benefit to the optimizer from this
>>> information because inlining is forced anyway, so this lie is both
>>> terrible
>>> and pointless.
>>
>>
>> The interesting case is a declaration like this:
>>
>>   extern inline __attribute__((gnu_inline, always_inline)) void f() {
>> /*...*/ }
>>
>> These are common in the glibc headers, and they mean:
>>
>> 1) This is a body just for inlining, and there is a strong definition
>> provided elsewhere (gnu_inline + extern inline), probably with an entirely
>> different body.
>> 2) If you see a direct call to this function, you must inline it.
>>
>> The intention appears to be to create a function definition that is only
>> used for inlining, and is never itself emitted. Taking the address of the
>> function should give the address of the strong definition from elsewhere,
>> not the address of this local definition of f.
>>
>> So, if we only want a single IR-level function for this case, it must be
>> available_externally (for (1)), and must be always_inline (for (2)). But
>> that results in the problem that Evgeniy reported.
>>
>>
>> How so?  You have a call that doesn't get inlined for whatever reason, and
>> it resolves against an external symbol that's guaranteed to exist.
>> Sub-optimal, but easily fixable by DCE'ing the dead call.  I don't see how
>> this fails to link unless, what, the program is using the libc headers and
>> not linking against libc?  Do we really need to fall over ourselves to
>> support that?
>>
>> Evgeniy seemed to be describing a situation where the external symbol
>> *wasn't* guaranteed to exist; but in that case, gnu_inline is a clear lie.
>
>
> The combination of flags above implies that if all calls are direct calls,
> then no external definition is required. Some headers rely on that, such as
> clang's own <htmxlintrin.h>.
_______________________________________________
cfe-dev mailing list
cfe-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev

[prev in list] [next in list] [prev in thread] [next in thread]