Re: [WPA PATCH 3/5] Benefit analysis.

[Martin Jambor <mjambor at suse dot cz>]

Hi,

thanks for looking into this.

On Thu, Aug 06, 2009 at 08:46:38PM +0200, Jan Hubicka wrote:
> Hi,
> 
> > +/* Whether the obstack below has been initialized.  */
> > +static bool bbbs_obstack_initialized;
> 
> Testing for NULL would probably works too.

If I understand it correctly, bitmap_obstack is a structure, not a
pointer.

> > +
> > +  if (dir)
> > +    expr = fold_build2 (gimple_cond_code (stmt), boolean_type_node,
> > +			invar, expr);
> > +  else
> > +    expr = fold_build2 (gimple_cond_code (stmt), boolean_type_node,
> > +			expr, invar);
> > +
> > +  if (!expr || TREE_CODE_CLASS (TREE_CODE (expr)) != tcc_comparison)
> > +    return;
> 
> Should not be there some logic testing if same conditonal appears
> multiple times in thefunction?

Different condition statements can make different BBs disappear so I
don't think so.  Or did you mean something else?

> 
> Otherwise it seems fine.  DO you have how much it works in practice?

It  is rather  elaborate and  the reason  is exactly  to make  it work
rather well.  All testcases I  constructed were analyzed as I expected
them to.  The later patches remove call graph edges based on this code
and does it well, I got bugs from call graph code when it was trying o
reconstruct them to  functions that were otherwise all  inlined and so
on.  So I think it does work just as expected.

> I would like to merge this with the inliner heruistics.  That is make it
> use inliner size/time estimates (that do ignore some specific
> instructions) and combine it with the param benefits.

Great.

> 
> I guess all I need to do on this route is to add new size/time vectors
> computed by inliner and use the bitmaps after propagation arguments same
> way as we do for indirect inlining, right?
> 

I think so, yes.  

Thanks,

Martin