Re: [LTO][PATCH] Converting inline failure reason strings to symbol constants

[Jan Hubicka <jh at suse dot cz>]

> 
> I will change to use a .def file.

Thanks, the change would be OK for mainline too once it reopens.
> 
> > I like the change in general. For LTO we however should not stream out
> > this, since it contains information that is bogus anyway - i.e. not
> > inlining because body is not available is not true anymore at WHOPR
> > stage since bodies are there.
> 
> > Once whole cgraph is built, this should do:
> >
> >  if (!edge->callee->analyzed)
> >    edge->inline_failed = N_("function body not available");
> >  else if (callee->local.redefined_extern_inline)
> >    edge->inline_failed = N_("redefined extern inline functions are not "
> >                             "considered for inlining");
> >  else if (edge->callee->local.inlinable)
> >    edge->inline_failed = N_("function not considered for inlining");
> >  else
> >    edge->inline_failed = N_("function not inlinable");
> 
> I ended up doing something similar in another sandbox, which will be
> checked in later as another patch.
> 
> > Note that redefined extern inline functions are gross hack and we must
> > handle them better for LTO to be useful.
> 
> What are redefined extern inline functions for?

GCC allows something like this:

extern inline
function ()
{
  some body;
}

function ()
{
  some completely different body;
}

(in single compilation unit)

The intended semantic probably is that for inlining we would use extern
inline version, while for not inlining we would use the second.

This does not work in practice, since frontend does declaration merging
and kills first declaration including body when parsing the second.
Originally  in non-unit-at-a-time compiler this sort of worked: you
defined the inline version in header and used it and at the end you
defined the fallback offline version.  But in current unit-at-a-time
this would result in never inlining the function.

This is worse with --combine (and I guess with LTO too) as we every
extern inline function is considered public (that is first problem, they
are effectively function local functions for purposes of visibility) so
as soon as we define some extern inline function twice (that happens for
about every program including same header twice), we never inline the
function as the fallback above is in effect.

I think the correct way to handle this is to make all copies of the
functions around making extern inline functions effectivly static inline
with the change that if inlining fails the call will be redirected to
the offline variant of function (that is public)
> 
> BTW, how big does loop_nest need to be in struct cgraph_edge? Can I
> steal 5 bits there to store the inline failed reason?  For WHOPR, we
> want to minimize the memory foot-print as much as possible.

loop nest is just number of loops the statement is in.  THere is no
official limitation, but the value is usually not large.
You definitly can steal another few bits and you can make
initializationof the value to saturate to the maximal value when the
nest is greater to make degenerated testcases not ICE.

Honza
> 
> Thanks for your comments.
> 
> -Doug