On Tue, Dec 16, 2025 at 12:33 AM Richard Biener
<[email protected]> wrote:
>
> On Tue, Dec 16, 2025 at 6:36 AM Andrew Pinski
> <[email protected]> wrote:
> >
> > After r16-6104-gb5c64db0a49d46, we try to duplicate bb's
> > that contain loop exit that have "never exit" but we check
> > against the propability of the exit to very_unlikely. If we
> > have PGO, then a loop exit might be very unlikely to be taken
> > if we interate the loop more than 2000 times. So we should take
> > into account if the probability is reliable or not; this basically
> > says if the probability is based on profile data (or corrected profile data)
> > or based on heurstics.
> >
> > This should fix PR 123126 but I don't have a full SPEC setup with PGO
> > to be able to test it.
> >
> > Bootstrapped and tested on x86_64-linux-gnu.
> >
> > gcc/ChangeLog:
> >
> > * tree-ssa-loop-ch.cc (should_duplicate_loop_header_p): Check
> > the reliable_p of the edge's probability.
> >
> > Signed-off-by: Andrew Pinski <[email protected]>
> > ---
> > gcc/tree-ssa-loop-ch.cc | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/gcc/tree-ssa-loop-ch.cc b/gcc/tree-ssa-loop-ch.cc
> > index feecf91cf70..293720fbd04 100644
> > --- a/gcc/tree-ssa-loop-ch.cc
> > +++ b/gcc/tree-ssa-loop-ch.cc
> > @@ -480,7 +480,8 @@ should_duplicate_loop_header_p (basic_block header,
> > class loop *loop,
> > or combining.
> > FIXME: once we have better quality tracking,
> > make this more robust. */
> > - || e->probability <= profile_probability::very_unlikely ()))
> > + || (!e->probability.reliable_p ()
> > + && e->probability <= profile_probability::very_unlikely
> > ())))
>
> Why !e->probability.reliable_p ()? That looks backward.
`reliable_p ()` says if we are using the profile data from an executed
run rather than the guessed values via heuristics.
`probably_never_executed_edge_p ()` checks the probability that the
edge is never executed (exactly 0%). With profiling data, then the
noreturn edge will be reliable but sometimes with the heuristics the
noreturn/unreachable edges will have a probability less than
very_unlikely.
So in the case with profile data (probability.reliable_p () being
true), we want to only see if it is exactly 0% probability. Rather
than the fuzzy amount of very_unlikely. Because a branch that is taken
less than 1 out of 2000 times (for an example loop with a trip count
over 2000), the code originally would duplicate the header and the
next block too. That is wrong.
Thanks,
Andrew
>
> Hopefully probably_never_executed_edge_p will DTRT here? I'll note
> there's also edge_probability_reliable_p () which uses .probably_reliable_p
> ().
>
> The profile API is too convoluted IMO :/ Possibly full of legacy stuff.
>
> Honza?
>
> > {
> > hasone = true;
> > if (dump_file && (dump_flags & TDF_DETAILS))
> > --
> > 2.43.0
> >
