Re: [PATCH] PR/66760, ipa-inline-analysis.c compile-time hog

[Martin Jambor <mjambor at suse dot cz>]

Hi,

On Mon, Jul 13, 2015 at 02:13:59PM +0200, Paolo Bonzini wrote:
> 
> 
> On 13/07/2015 13:55, Martin Jambor wrote:
> > I can't approve it, but FWIW, I'm generally fine with the patch.
> > Although the original idea was to share one func_body_info in between
> > ipa-cp and ipa-inline analyses, this is certainly better than what we
> > have now and perhaps even good enough generally.
> 
> Ah, so you'd add a pointer to cgraph_node?

No, that does not seem right, given that this data is only needed
during very limited time.  You might want to use Martin's shiny new
function_summary class in symbol-summary.c.  That is a mechanism
specifically designed to append to a cgraph_node information specific
to an optimization pass (or two, as ipa-cp and ipa-inline already both
use a few of them).  Unfortunately, the class is not very well
documented but you should be able to figure out how to use it from
other code using them.

If you then always deallocate everything there at the end of
ipa-inline analysis, you'll get exactly the right life-time for the
data.

> I only have a question
> then---does cgraph_node have a "destructor" where I can free the
> func_body_info?  Or is there no such thing?

No, not really, but a summary would give you that.

> 
> > The only semi-issue I have is the name of func_body_info.  If it is
> > going to be exposed in a header file, perhaps it should get an ipa_
> > prefix.
> 
> That's a patch as large as this one.  I can do the rename later, and
> maybe have it preapproved to convince me to get the new public key in
> place. :)

Yeah, I think that is good enough (but remember I can't approve or
preapprove anything).

> 
> > I also think that its initialization should be put into a
> > common function, but that is somethig I can do as a followup, if need
> > be.
> 
> Tried doing that now...  it seems better to do it together with adding a
> func_body_info* to cgraph_node*, so that the initialization is done
> lazily in cgraph_node::get_func_body_info.
> 

Well, see above, we're actually trying to  pull information like this
out of cgraph_node.

Martin