Re: [patch] Update profile in loop versioning and unrolling

[Jan Hubicka <jh at suse dot cz>]

Hi,

> I have added some tests for the testsuite; unfortunately, I do not know
> about a reliable way how to test whether probability/frequency of some
> object is set "sanely", since the structure of cfg and the exact values of
> frequencies may change easily by introducing changes to other
> optimizers.

This is always unfortunate problem.  For now I was always able to get around
with grepping for "invalid sum" messages in tree dumps for simple testcases.
    /* Fix frequencies.  */
!   if (redirect_all_edges)

I don't quite understand why the redirect_all_edges is becoming neccsary here,
but I guess it is within your maintenance rights.

+ /* Sets probability and count of edge E to zero.  The probability and count
+    is redistributed evenly to the remaining edges coming from E->src.  */

Perhas this can be sane thing to do within cleanup_cfg when eliminating
constant conditionals?  Still not perfect but perhaps better than what we do now,
at least if

if (blah)
  fooo

is folded away, we get right profile, while we will currently break consistently
everywhere.  If this function is generally useful, I would like it to move to predict.c.

+       if (orig
+ 	  /* Do not risk overflows.  The probability of the exit is usually
+ 	     small, anyway.  */
+ 	  && REG_BR_PROB_BASE - orig->probability >= REG_BR_PROB_BASE / 5)

Will this overflow for anything else than REG_BR_PROB_BASE == orign->prbability?
+   /* Avoid overflows.  */
+   if (freq_e * est_niter < 65536
+       && freq_e * est_niter <= 100 * freq_h)
+     scale_loop_frequencies (loop, freq_e * est_niter, freq_h);

Similarly here, it would be nice to come with some solution without magic
numbers spread around the sources.  What is the scenario we run into overflow
here?

Otherwise the patch looks fine. And thanks for looking into the updating here!

Honza