[cfe-dev] [RFC] AlwaysInline codegen

[John McCall via cfe-dev]

> On Aug 20, 2015, at 7:36 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> 
> On Thu, Aug 20, 2015 at 7:17 PM, John McCall <rjmccall at apple.com <mailto:rjmccall at apple.com>> wrote:
> > On Aug 20, 2015, at 5:19 PM, Evgenii Stepanov via cfe-dev <cfe-dev at lists.llvm.org <mailto:cfe-dev at 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.

John.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20150821/7ba53218/attachment.html>