Re: Inlining and estimate_num_insns

[Jan Hubicka <jh at suse dot cz>]

> Steven Bosscher wrote:
> >On Sunday 27 February 2005 10:57, Richard Guenther wrote:
> >
> >>Steven Bosscher wrote:
> >>
> >>>On Feb 27, 2005 02:04 AM, Richard Guenther 
> >
> ><rguenth@tat.physik.uni-tuebingen.de> wrote:
> >
> >>>>In the end we surely want to watch CiSBE and SPEC testers.
> >>>
> >>>Maybe so, but your timings already show this is pretty unacceptable.
> >>
> >>Well, the compile time regressions are not caused by my patch but only
> >>exposed by them.  Previously we were saying "hey, we are as fast as
> >>3.4", but really comparing apples and oranges, as we regress badly in
> >>performance by just doing less work due to less inlining.  If you want
> >>to throttle that back, you can as well, as a followup patch, reduce
> >>inlining limits.  That of course doesn't remove the conceptual
> >>improvement of estimating the size of an inlined indirection the same
> >>as the actual function.
> >>
> >>How do you suppose we fix the three-fold run-time performance
> >>regressions I and other people see for their scientific code?
> >
> >
> >I'm not sure, but 1) I have large C++ codes and don't see such
> >slowdowns, and 2) I see no reason to kill compile time for everyone
> >just to work around the (ahem) lameness of heavily templatized code
> >in a relatively small number of C++ applications.  I understand it
> 
> I take it as a lame property of our current inlining heuristics
> and function size estimate that for
> 
>   inline int foo { return 0; }
>   int bar { return foo(); }
> 
> the size estimate of foo is 2, that of bar is initially 13 and 5
> after inlining.  3.4 did better in that it has 1 for foo, 12 for bar
> before and 3 after inlining.  Though one could equally argue that
> 3.4 had an abstraction penalty of 3.0 while 4.0 now has 2.5.
> 
> It is not hard to believe that for C code this does not matter much,
> but for C++ it effectively lowers our inlining limits as at least
> things like accessor methods are very common, and if you think of
> template metaprograms, with the old scheme we're just loosing.
> 
> >is a problem for a few people, but if you look at the performance
> >numbers of things like SPEC, CSiBE, and MySQL, there doesn't appear
> >to be any performance problem.  But your patch will probably still
> >blow compile time through the roof for those applications as well.
> 
> I don't know.  But I also think our inlining limits are way too high,
> at least unnecessary high with the patch applied.  I'll try do find
> some more representative packages to test - any hints?  In the mean
> time would it be ok to apply the patch to mainline to have the automatic
> testers do some tests and then afterwards maybe revert it, if we do
> not like it?  I'll take the previous approval of Jan as taken back
> for now.

Well, I still think that the metric without artificial assignemnts is
more realistic so the path of using your patch in combination of
re-benchmarking the inlining limits is proper way to go.  I would not
expect drastic slowdown in testcases that don't suffer from abstraction
penalty.  I will test it on SPEC tonight and send you a results
tomorrow so lets see how intuition works here.

The basic idea is that we need to recognize "forwarder" functions as
being free to get sane performance on funny C++ template codes.  I was
even thinking about some kind of pattern matching for common cases (ie
functions just passing their arguments to other functions and similar)
and forcing their costs to be CALL_INSN_COST, but didn't get across that
yet.  So we should slow down compilation of testcases like tramp3d to
get sane performance and not affect too much lower level code, like
SPEC.

Honza
> 
> Richard.