Re: [PATCH] Fix ICE in early inliner (PR ipa/65008)

[Marek Polacek <polacek at redhat dot com>]

On Thu, Feb 19, 2015 at 12:06:40PM +0100, Richard Biener wrote:
> On Thu, 19 Feb 2015, Marek Polacek wrote:
> 
> > The problem exposed by this PR is (IIUC) that we hadn't gotten around to
> > recomputing the inline parameters in the case when optimize_inline_calls
> > introduces new statements.  That results in ICEing later on because in
> > estimate_edge_growth we assert that estimated size of a statement is not
> > 0.
> > 
> > This happens since r220359 - with this change, we started to perform early
> > inlining even in always_inline functions.  So in the following testcase,
> > we have in A::A() at the start of early inlining:
> > 
> >   call_foo (this_2(D));
> > 
> > Since call_foo is always_inline, we inline it and apply the changes via
> > a call to optimize_inline_calls.  That turns the above statement into:
> > 
> >   A::foo (this_2(D));
> > 
> > This statement is new and we don't have the inline params for it computed,
> > because when estimate_function_body_sizes walked the IL, the stmt wasn't
> > there.
> > 
> > So fixed by doing what we do in early_inliner in a block below, that is,
> > recomputing the inline parameters.  I didn't copied the if that calls
> > gimple_check_call_matching_types and sets edge->call_stmt_cannot_inline_p,
> > I'm not sure if it's needed.
> > 
> > Does that make sense?
> > 
> > Bootstrapped/regtested on x86_64-linux.
> > 
> > 2015-02-19  Marek Polacek  <polacek@redhat.com>
> > 
> > 	PR ipa/65008
> > 	* ipa-inline.c (early_inliner): Recompute inline parameters.
> > 
> > 	* g++.dg/ipa/pr65008.C: New test.
> > 
> > diff --git gcc/ipa-inline.c gcc/ipa-inline.c
> > index 025f7fc..c445f0a 100644
> > --- gcc/ipa-inline.c
> > +++ gcc/ipa-inline.c
> > @@ -2559,6 +2559,19 @@ early_inliner (function *fun)
> >  	{
> >  	  timevar_push (TV_INTEGRATION);
> >  	  todo |= optimize_inline_calls (current_function_decl);
> > +	  /* optimize_inline_calls call above might have introduced new
> > +	     statements that don't have inline parameters computed.  */
> > +	  for (edge = node->callees; edge; edge = edge->next_callee)
> 
> Are cgraph edges up-to-date here?  I'd doubt that...  if so, why not
> do this update in the inliner itself where it updates the cgraph edges?

I've tried to move this hunk into tree-inliner.c, but I think that is
not possible.  E.g. inline_edge_summary is ipa-inline thing only.
Also all my attemps to update the inline metrics somewhere in copy_bb or
e.g. expand_call_inline failed, so I'm afraid I don't have anything
better than the original patch ;).  Honza, any opinion?

	Marek