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

[Marek Polacek <polacek at redhat dot com>]

Ping.

On Thu, Feb 19, 2015 at 09:03:51PM +0100, Marek Polacek wrote:
> 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

	Marek