This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] profile feedback: -fprofile-use= and -fprofile-correction, correctness fixes and option semantic changes.


On Sun, Mar 30, 2008 at 6:13 AM, Jan Hubicka <hubicka@ucw.cz> wrote:
> >
>  > I'm not sure it makes sense to do this for gcov - I'd think it's
>  > probably better off
>  > to leave gcov output as simply inconsistent.
>
>  Yes, inconsistent output is OK, but we need to add some switch to make
>  gcov to output it instead of erroring out IMO so the profiles can be
>  used for optimization and for gcoving.

I see. I'll add that.

>  > The reason I made the counters artificially consistent is because
>  > some of transformations that goes haywire when the flow consistency is
>  > not correct.
>  > There were enough of those that I didn't try to find all those places and
>  > make them tolerate inconsistencies (I may very well try to do that later,
>  > if smoothing, even with more sophisticated algorithms, doesn't work out well.
>
>  One problem is that compiler itself introduce inconsistencies as result
>  of optimization (for example inlining distributes profile evenly and if
>  one of inline copies gets conditionals optimized to true or false, its
>  local profile gets confused). None of optimization should go completely
>  out of control when seeing such problems.
>
>  The overall scheme is to try to avoid such inconsistencies by curefully
>  updating profile after CFG manipulations (there is warning output to
>  dump files when ones are seen) but if there is no way around, we should
>  just live with it.  I am trying to keep the code behaving sanely on
>  incosistent profiles, but of course it is easy to get things wrong here.
>
>  Other way would be to artifically push profile to consistent shape using
>  yours code or by re-doing the frequency propagation on guessed profile,
>  but it is bit expensive to do it after each cfgcleanup that might
>  obsolette profile this way.
>
>  > >  > +/* Make the block/edge counts of the entire CFG flow-consistent.
>  > >  > +   This is one super hack.  The problem is difficult to solve well
>  > >  > +   but we rely on compute_all_counts()
>  > >  > +   to leave no negative number and repeatedly apply the backward propagation
>  > >  > +   of counts, until everything's consistent.  */
>  > >
>  > >  We can either have this hack, or we can simply disable the optimization
>  > >  and emit counters on each edge as done by some compilers...  I guess
>  > >  experience will show how much off the result can be and how much we
>  > >  care.
>  >
>  > We can take that approach, but it at least requires more testing/code inspection
>  > to weed out all places in the compiler (e.g. I found some code, e.g. i386.c,
>  > that breaks if it's fed flow-inconsistent edge counts).
>
>  We need to fix those...
>
>  > Without this change, we can end up with a dangling histogram
>  > (because value histograms are kept in a separate hash table
>  > key'ed off of the statement pointer values themselves).
>  > The buggy case I encountered was when a memmove() call
>  > with a histogram attached is replaced with nop.
>  > The histogram checking later finds  the dangling histogram
>  > and leads to ICE.
>
>  I see, there is code to move the histograms, I think it is better to use
>  it.  It is used at same places as we move EH info.  If this place is
>  replacing statement without moving EH info and histograms, we ought to
>  fix both problems.
>
>  We do some work to replace one stringop by different and in that case
>  the histogram might be still useful.  Of course in case of replacing by
>  NOP it is best to kill it, but I think it is easier to just keep
>  shuffling the histograms around until they are finally ignored by
>  expansion instead of trying to figure out in what cases it might be
>  still useful (that can be probably done by calling back to value
>  profiling code)
>
>  Honza

Ok. I'll drop the hunk from this patch, and submit a separate patch
that will keep the histograms around in set_rhs.
Thanks!

Seongbae


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]