On Fri, 7 Aug 2026, Tobias Burnus wrote:
> 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?
So I think the invariant we know is that the BB counts in
the original
do { if (outer) if (inner) <true-count>; else <false-count>; } while ();
should be preserved even when the inner if is moved and so we have
if (inner)
do { if (outer) <true-count>; } while ();
else
do { if (outer) <false-count>; } while ();
the task is to compute a reasonable count setup and from that adjust
probabilities. I'll note that the probabilities of the if (outer)
is not fixed either.
> ?
>
> * * *
>
> > - 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.
It depends whether you update then/else scale or the edge
probabilties reliably (IIRC the edges are only created downstream,
so you have to update the scales).
> 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
>
>
--
Richard Biener <[email protected]>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Jochen Jaser, Andrew McDonald, Abhinav Puri; (HRB 36809, AG Nuernberg)
