[llvm-commits] [cfe-commits] [PATCH] Potential fix for PR9614

[Peter Collingbourne]

On Tue, Oct 11, 2011 at 01:32:14PM -0700, Eli Friedman wrote:
> On Tue, Oct 11, 2011 at 1:28 PM, Eli Friedman <eli.friedman at gmail.com> wrote:
> > On Sun, Oct 9, 2011 at 12:11 PM, Peter Collingbourne <peter at pcc.me.uk> wrote:
> >> Hi,
> >>
> >> These patches (for Clang and LLVM) together fix PR9614.
> >>
> >> The Clang patch causes it to mark self calls to available_externally
> >> functions via an asm label as noinline, and the LLVM patch inhibits
> >> certain optimizations, including tail call elimination, for noinline
> >> calls to available_externally functions, forcing a call to the external
> >> version of the function.
> >>
> >> Any thoughts on this approach?  Is this OK to commit?
> >
> > I really don't like overloading the semantics of noinline in this
> > way... noinline is supposed to prevent inlining, not stop every
> > optimization that might examine the contents of the function.  If you
> > need to represent something new, use a new call attribute, or
> > something like that.

OK, I like that better.  What about adding a call attribute 'external'
which resides before 'tail' in the grammar of the call instruction?

> >  (Granted, that's still a bit messy, but I can't
> > think of a particularly better solution... the only other thing I can
> > think of is abusing aliases somehow to get the desired semantics,
> > using a special kind of linkage.  Possibly more localized, but aliases
> > can get messy.)

I considered using aliases for this, but aliases don't really capture
the semantics of the call as well as a call attribute would do, and
the attribute seems to be simpler for frontends for other languages
which need this same functionality to emit.  So, less messy IMHO.

> Oh, also, if you want to get something into 3.0 quickly, just hacking
> clang so it doesn't emit the definition at all in situations like
> PR9614 would be more obviously acceptable.

Given that most of the optimisations we'd need to inhibit have already
been identified, I'd prefer to do this properly.  I'd like this to
be in 3.0 but I'm not too picky about it.

Thanks,
-- 
Peter