Re: [Patch] Handle profile counts truncated to 0 in coldness test

[Jan Hubicka <hubicka at ucw dot cz>]

> The second part ensures that frequencies are correctly updated after
> inlining. The problem is that after inlining the frequencies were
> being recomputed directly from the corresponding bb counts in
> rebuild_frequencies. If the counts had been truncated to 0, then the
> recomputed frequencies would become 0 as well (often leading to
> inconsistencies in the frequencies between blocks). Then the above
> change to probably_never_executed would not help identify these blocks
> as non-cold from the frequencies. The fix was to use the existing
> logic used during static profile rebuilding to also
> recompute/propagate the frequencies from the branch probabilities in
> the profile feedback case. I also renamed a number of estimate_*
> routines to compute_* and updated the comments to reflect the fact
> that these routines are not doing estimation (from a static profile),
> but in fact recomputing/propagating frequencies from the existing
> (either guessed or profile-feedback-based) probabilities.

I see where your plan - you want frequencies to be computed purely on
probabilities that are there.  This however will lead to unnecesary loss of
precisions for functions with large counts: probabilities are truncated in
precision and the propagation won't give correct answers and has serious
truncating issues.

So perhaps we want to execute this only if maximal count in function is low?
(i.e. less than REG_BR_PROB_BASE or even REG_BR_PROB_BASE/10 - if counts
are close enough the roundoff errors of count updating should be less terrible
than errors introduced by the propagation).

> -/* Estimate probabilities of loopback edges in loops at same nest level.  */
> +/* Compute frequencies in loops at same nest level.  */
> 
>  static void
> -estimate_loops_at_level (struct loop *first_loop)
> +compute_loops_at_level (struct loop *first_loop)

I would keep "estimate".  The whole logic is not safe and it will lead to
mistakes for irreducible loops and many other cases...

> @@ -3027,18 +3042,16 @@ void
>  rebuild_frequencies (void)
>  {
>    timevar_push (TV_REBUILD_FREQUENCIES);
> -  if (profile_status == PROFILE_GUESSED)
> +  if (profile_status == PROFILE_GUESSED || profile_status == PROFILE_READ)
>      {
>        loop_optimizer_init (0);
>        add_noreturn_fake_exit_edges ();
>        mark_irreducible_loops ();
>        connect_infinite_loops_to_exit ();
> -      estimate_bb_frequencies ();
> +      compute_bb_frequencies (true);

Here please add the check about maximal count in functiona and commnet
(especially for case we will remember to remove this hack if we switch to sreal
based counts/frequencies/probabilities)

OK, with those changes (if it fixes the problem)

Thanks,
Honza