On Tue, Dec 16, 2025 at 4:14 AM Richard Biener
<[email protected]> wrote:
>
> On Tue, Dec 16, 2025 at 9:48 AM Andrew Pinski
> <[email protected]> wrote:
> >
> > 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.
>
> Yes, which is why I'm suggesting probably_never_executed_edge_p ()
> as we should predict edges to abort() and the like as executing never.
There is a check on `probably_never_executed_edge_p ()` right above
already. But I have found (in one of the testcases I added,
`gcc.dg/tree-ssa/copy-headers-{10,11,12}.c`),
probably_never_executed_edge_p sometimes returns false for edges to
abort/trap/unreachable. I didn't look into why though. This is why
there is an extra check added here.
Let me look into the reason why it is not predicted as never then.
Thanks,
Andrew
>
> But I'll leave this to Honza.
>
> Richard.
>
> >
> > 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
> > > >
