On 28/04/2025 16:33, Alex Coplan wrote: > On 15/04/2025 19:48, Jan Hubicka wrote: > > Hi, > > > gcc/ChangeLog: > > > > > > PR tree-optimization/117790 > > > * tree-vect-loop.cc (scale_profile_for_vect_loop): Use > > > scale_loop_profile_hold_exit_counts instead of scale_loop_profile. Drop > > > the exit edge parameter, since the code now handles multiple exits. > > > Adjust the caller ... > > > (vect_transform_loop): ... here. > > > > > >gcc/testsuite/ChangeLog: > > > > > > PR tree-optimization/117790 > > > * gcc.dg/vect/vect-early-break-profile-2.c: New test. > > > > > > > > >- if (entry_count.nonzero_p ()) > > >- set_edge_probability_and_rescale_others > > >- (exit_e, > > >- entry_count.probability_in (loop->header->count / vf)); > > >- /* Avoid producing very large exit probability when we do not have > > >- sensible profile. */ > > >- else if (exit_e->probability < profile_probability::always () / (vf * > > >2)) > > > > This is handling relatively common case wehre we decide to vectorize > > loop with, say, factor of 32 and have no profile-feedback. > > In this case if the loop trip count is unknown at early loop, we will > > esitmate it to iterate few times (approx 3-5 as that is average > > iteration count of random loop based on some measurements). > > > > The fact that we want to vecotirze by factor 32 implies that vectorizer > > does not take this info seriously and its heuristics thinks better. > > In this case we do not wan to drop the loop to 0 iteraitons as that > > would result of poor code layout and regalloc. > > > > I don't think you kept this logic in the new code? > > To be honest, I didn't really follow the logic here. Thinking about the > single-exit case (which the current code is designed to handle), both > the body of the if and the else if logically implement the same update: > > P'(exit) = vf * P(exit) > > I suppose the code in the body of the if is just a more numerically stable way > of doing this? > > Also, AFAICT, the condition of the else if is trying to make sure that: > > vf * P(exit) < 1/2 > i.e. > P'(exit) < 1/2 > > so it ensures we don't inflate the exit probably above 1/2 (equivalently > we don't reduce the expected niters below 2). But I don't understand > why this guard isn't applied to the whole if-else chain. Why should we > allow inflating the exit probability above 1/2 if entry_count.nonzero_p (), > but otherwise not? > > I dropped this logic in the 4/4 patch since it seemed > inconsistent, and in general the updating of exit probabilities > is already handled by scale_loop_profile_hold_exit_counts. > > Can you explain the intent behind the current logic, so I know what > behaviour we want to preserve? A concrete testcase would be useful, > too. FWIW, I tried doing a testsuite run on aarch64 with the following patch: > > diff --git a/gcc/tree-vect-loop.cc b/gcc/tree-vect-loop.cc > index 3de1ea646b4..71787301e6d 100644 > --- a/gcc/tree-vect-loop.cc > +++ b/gcc/tree-vect-loop.cc > @@ -12092,6 +12092,9 @@ scale_profile_for_vect_loop (class loop *loop, edge > exit_e, unsigned vf, bool fl > sensible profile. */ > else if (exit_e->probability < profile_probability::always () / (vf * 2)) > set_edge_probability_and_rescale_others (exit_e, exit_e->probability * > vf); > + else > + gcc_unreachable (); > + > > on top of current trunk, and the unreachable wasn't hit once.
Ping on this? I think this discussion being unresolved is the main thing blocking the series here: https://gcc.gnu.org/pipermail/gcc-patches/2025-May/684930.html https://gcc.gnu.org/pipermail/gcc-patches/2025-May/684931.html https://gcc.gnu.org/pipermail/gcc-patches/2025-May/684932.html Thanks, Alex > > Thanks, > Alex > > > > > Honza > > >- set_edge_probability_and_rescale_others (exit_e, exit_e->probability > > >* vf); > > >- loop->latch->count = single_pred_edge (loop->latch)->count (); > > >- > > >- scale_loop_profile (loop, profile_probability::always () / vf, > > >- get_likely_max_loop_iterations_int (loop)); > > >+ const auto likely_max_niters = get_likely_max_loop_iterations_int > > >(loop); > > >+ scale_loop_profile_hold_exit_counts (loop, > > >+ profile_probability::always () / vf, > > >+ likely_max_niters); > >
