This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
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