Re: Fix one of problems in profile updating code

[Jan Hubicka <jh at suse dot cz>]

> On Fri, 2005-07-15 at 15:12 -0700, Mark Mitchell wrote:
> > Jan Hubicka wrote:
> > > Jeff,
> > > with one of updates you killed code computing counts of duplicated block
> > > in thread updating code.  Please try to mind profile when modifying the
> > > code further as it is getting bit dificult to figure out what jump
> > > threading does and how to fix it and the code broke several times in the
> > > past.  This bug was responsible for quite large regression of art
> > > benchmark and there still seems to be other problem described in
> > > PR22401.
> > 
> > Jeff, Honza --
> > 
> > Is there something that can be done here to make it harder for people to 
> > make mistakes with profiling?  This is an important feature, but one 
> > that not everyone uses.  Like, is it possible to create a test case for 

Well, almost everyone uses the guessed profile at -O2.  Missupdating it
can cause similar problems as missupdating the real profile...

I was trying to simplify the testcase to something that can be thrown
into testsuite and grepped for Invalid sum, but the testcase ended up
pretty complex.  Jeff, it is probably trivial for you to create testcase
that will reliably force jump threading to invent new duplicated basic
block I can use as a skeleton to build something?

> > the bug you just fixed so it doesn't happen again?
> Or at least cobble together (and point everyone to) documentation
> on what kinds of transformations will muck up the profiling
> information and how to fix it.

Well, about every transformation on CFG via BB creation/edge redirection
needs updating code.  I guess I can come up with something on GCC
wiki...

One idea I run across is to force BB/edge creation routines to
initialize profile into invalid values so verify_flow_info would abort
when user don't do something about it.  But I am not quite sure if it is
not too painful as people would probably want to play with CFG for a
while without worrying about updating code...
It would not catch the edge redirection problems but might make people
more aware that profile is there.
> 
> Basically all the profiling stuff is a black box to me (and
> possibly others).  If I know what to look out for and how to
> fix it, then, well, I'll do that.

The incorrectly updated profiles should show up as "Invalid sum"
messages in the dump files.  Unforutnately for counts you need real
profile to be read in to see them and sometimes profile can not be
updated correctly or roundoff errors cummulate so the Invalid sums are
unavoidable.  (thus the dificulty to maintain updating code and my
reservations to throw nontrivial testcases into testsuite as they might
eventually break for such a "valid" reasons)

Another plan I have is to teach our memory testing script to watch
patches that increase number of Invalid sums and ping author.  This is
pretty weak but might turn out to be working well in practice, don't
know yet.

The profile is stored via count fields in edges/basic block that
represent number of execution (so, for instance, if you redirect edge
via threading, you need to compensate count of basic block and edge you
threaded across) and frequencies/probabilities for guessed profile.
Frequencies are just as counts just reduced into range of
0...REG_BR_PROB_BASE so you don't get overflows when manipulating with
these.  Probabilities represent probability that basic block will be
leaved by given edge (so it's frequency is computed as
e->succ->probability * e->frequency / REG_BR_PROB_BASE, as done by
EDGE_FREQUENCY macro).

HTH
Honza
> 
> Jeff
>