Re: [Patch][RFC?] Re-guess probability when unswitch hoists a condition before the loop [PR126664]

Richard Biener wrote:
>> Back to the attached patch:
>>
>> The patch is based on the assumption that the probability can be
>> different when moved outside of the loop - and just re-guesses it
>> makes sense, which yields the expected 41% and 59% edge probabilities.
>>
>> Is the patch OK – or at least like the right approach?
> unconditionally re-guessing seems overly conservative when the
> unswitched condition is always executed in the loop.

But the question is what to do otherwise?

(1) Only do so when any of the probabilities is 'never'? Namely:

     profile_probability prob_true = edge_true->probability;
     profile_probability prob_false = prob_true.invert ();
     if (prob_true.nonzero_p() && prob_false.nonzero_p())
       prob_true = prob_false = profile_probability::uninitialized ();

     This will fix the 0% / 100% case and yield here 41% / 59%;
     it might not fix other cases, but also has not much overhead.

(2) Use conditionally profile_probability::even () ?

     This will use 50% / 50% – which is not as good as the 41% / 59%
     but less expensive - and surely better than 0% / 100%. It might
     pessimize cases where the probability is finite but highly
     unbalanced and correctly guessed.

(3) do something else – but what?

     ?

* * *

> -  scale_loop_frequencies (loop, then_scale);
> -  scale_loop_frequencies (nloop, else_scale);
> +  if (then_scale.initialized_p () && else_scale.initialized_p ())
> +    {
> +      scale_loop_frequencies (loop, then_scale);
> +      scale_loop_frequencies (nloop, else_scale);
> +    }
> +  else
> +    {
> +      edge te = EDGE_SUCC (cond_bb, 0);
> +      edge ee = EDGE_SUCC (cond_bb, 1);
> +      scale_loop_frequencies (loop, te->probability);
> +      scale_loop_frequencies (nloop, ee->probability);
> +    }
>
> doesn't the 2nd hunk always work? (are you sure of the
> first/second edge going to the respective loops?)

Do you mean 'does the 2nd ...'? If so, see below. If you
really meant 'does not the 2nd ...' than I think I don't
understand the question.

In any case:

cond_bb = lv_adjust_loop_entry_edge (...)

and that function returns 'new_head'. That is created
(current GCC version for simplicity):

   new_head = split_edge (e);
   lv_add_condition_to_bb (first_head, second_head, new_head,
                           cond_expr);
   e = single_succ_edge (new_head);
   e1 = make_edge (new_head, first_head,
                   current_ir_type () == IR_GIMPLE ? EDGE_TRUE_VALUE : 0);
   e1->probability = then_prob;
   e->probability = else_prob;

where single_succ_edge returns EDGE_SUCC (bb, 0) and checks that
this is the only edge.

And as lv_adjust_loop_entry_edge states, it does:
    --- edge e ---- > [second_head]
    Split it and insert new conditional expression and adjust edges.
     --- edge e ---> [cond expr] ---> [first_head]
                         |
                         +---------> [second_head]

Thus, I think that code should be always fine (until the function
is largely modified).

Tobias