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 <pola...@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
