diff mbox series

Disable loop distribution for loops with estimated iterations 0

Message ID ZMylmGqvOtxn7pEf@kam.mff.cuni.cz
State New
Headers show
Series Disable loop distribution for loops with estimated iterations 0 | expand

Commit Message

Jan Hubicka Aug. 4, 2023, 7:15 a.m. UTC
Hi,
this prevents useless loop distribiton produced in hmmer.  With FDO we now
correctly work out that the loop created for last iteraiton is not going to
iterate however loop distribution still produces a verioned loop that has no
chance to survive loop vectorizer since we only keep distributed loops
when loop vectorization suceeds and it requires number of (header) iterations
to exceed the vectorization factor.

Bootstrapped/regtested x86_64-linux, OK?

gcc/ChangeLog:

	* tree-loop-distribution.cc (loop_distribution::execute): Disable
	distribution for loops with estimated iterations 0.

Comments

Richard Biener Aug. 4, 2023, 8:19 a.m. UTC | #1
On Fri, Aug 4, 2023 at 9:16 AM Jan Hubicka via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Hi,
> this prevents useless loop distribiton produced in hmmer.  With FDO we now
> correctly work out that the loop created for last iteraiton is not going to
> iterate however loop distribution still produces a verioned loop that has no
> chance to survive loop vectorizer since we only keep distributed loops
> when loop vectorization suceeds and it requires number of (header) iterations
> to exceed the vectorization factor.
>
> Bootstrapped/regtested x86_64-linux, OK?

OK.

But why does optimize_loop_for_speed_p () return true when the loop
isn't expected to iterate?  Wouldn't that be a better place to fix this
and similar issues in other places then?

Thanks,
Richard.

> gcc/ChangeLog:
>
>         * tree-loop-distribution.cc (loop_distribution::execute): Disable
>         distribution for loops with estimated iterations 0.
>
> diff --git a/gcc/tree-loop-distribution.cc b/gcc/tree-loop-distribution.cc
> index cf7c197aaf7..8ff2108f284 100644
> --- a/gcc/tree-loop-distribution.cc
> +++ b/gcc/tree-loop-distribution.cc
> @@ -3871,10 +3871,20 @@ loop_distribution::execute (function *fun)
>
>           bool destroy_p;
>           int nb_generated_loops, nb_generated_calls;
> +         bool only_patterns = !optimize_loop_for_speed_p (loop)
> +                              || !flag_tree_loop_distribution;
> +         /* do not try to distribute loops that are not expected to iterate.  */
> +         if (!only_patterns)
> +           {
> +             HOST_WIDE_INT iterations = estimated_loop_iterations_int (loop);
> +             if (iterations < 0)
> +               iterations = likely_max_loop_iterations_int (loop);
> +             if (!iterations)
> +               only_patterns = true;
> +           }
>           nb_generated_loops
>             = distribute_loop (loop, work_list, cd, &nb_generated_calls,
> -                              &destroy_p, (!optimize_loop_for_speed_p (loop)
> -                                           || !flag_tree_loop_distribution));
> +                              &destroy_p, only_patterns);
>           if (destroy_p)
>             loops_to_be_destroyed.safe_push (loop);
>
Jan Hubicka Aug. 4, 2023, 8:58 a.m. UTC | #2
> On Fri, Aug 4, 2023 at 9:16 AM Jan Hubicka via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
> >
> > Hi,
> > this prevents useless loop distribiton produced in hmmer.  With FDO we now
> > correctly work out that the loop created for last iteraiton is not going to
> > iterate however loop distribution still produces a verioned loop that has no
> > chance to survive loop vectorizer since we only keep distributed loops
> > when loop vectorization suceeds and it requires number of (header) iterations
> > to exceed the vectorization factor.
> >
> > Bootstrapped/regtested x86_64-linux, OK?
> 
> OK.
> 
> But why does optimize_loop_for_speed_p () return true when the loop
> isn't expected to iterate?  Wouldn't that be a better place to fix this
> and similar issues in other places then?

optimize_loop_for_speed_p checks whether the loop header is considered
hot so we want to get it running fast.  I think it is up to each loop
transform to decide whether it helps loops with low iteration counts or
hight iteration counts or both.  Loop peeling and copy header are passes
that does helps low iteration count loops.  I think we have more.

For example I wondered if I should also disable splitting but I think
moving the conditional out of loop will likely help even if loop has
small trip count.

I briefly looked what passes already have cost model based on iteration
estimate. I guess we should also tame down invariant motion and perhaps
others.

Honza
> 
> Thanks,
> Richard.
> 
> > gcc/ChangeLog:
> >
> >         * tree-loop-distribution.cc (loop_distribution::execute): Disable
> >         distribution for loops with estimated iterations 0.
> >
> > diff --git a/gcc/tree-loop-distribution.cc b/gcc/tree-loop-distribution.cc
> > index cf7c197aaf7..8ff2108f284 100644
> > --- a/gcc/tree-loop-distribution.cc
> > +++ b/gcc/tree-loop-distribution.cc
> > @@ -3871,10 +3871,20 @@ loop_distribution::execute (function *fun)
> >
> >           bool destroy_p;
> >           int nb_generated_loops, nb_generated_calls;
> > +         bool only_patterns = !optimize_loop_for_speed_p (loop)
> > +                              || !flag_tree_loop_distribution;
> > +         /* do not try to distribute loops that are not expected to iterate.  */
> > +         if (!only_patterns)
> > +           {
> > +             HOST_WIDE_INT iterations = estimated_loop_iterations_int (loop);
> > +             if (iterations < 0)
> > +               iterations = likely_max_loop_iterations_int (loop);
> > +             if (!iterations)
> > +               only_patterns = true;
> > +           }
> >           nb_generated_loops
> >             = distribute_loop (loop, work_list, cd, &nb_generated_calls,
> > -                              &destroy_p, (!optimize_loop_for_speed_p (loop)
> > -                                           || !flag_tree_loop_distribution));
> > +                              &destroy_p, only_patterns);
> >           if (destroy_p)
> >             loops_to_be_destroyed.safe_push (loop);
> >
diff mbox series

Patch

diff --git a/gcc/tree-loop-distribution.cc b/gcc/tree-loop-distribution.cc
index cf7c197aaf7..8ff2108f284 100644
--- a/gcc/tree-loop-distribution.cc
+++ b/gcc/tree-loop-distribution.cc
@@ -3871,10 +3871,20 @@  loop_distribution::execute (function *fun)
 
 	  bool destroy_p;
 	  int nb_generated_loops, nb_generated_calls;
+	  bool only_patterns = !optimize_loop_for_speed_p (loop)
+			       || !flag_tree_loop_distribution;
+	  /* do not try to distribute loops that are not expected to iterate.  */
+	  if (!only_patterns)
+	    {
+	      HOST_WIDE_INT iterations = estimated_loop_iterations_int (loop);
+	      if (iterations < 0)
+		iterations = likely_max_loop_iterations_int (loop);
+	      if (!iterations)
+		only_patterns = true;
+	    }
 	  nb_generated_loops
 	    = distribute_loop (loop, work_list, cd, &nb_generated_calls,
-			       &destroy_p, (!optimize_loop_for_speed_p (loop)
-					    || !flag_tree_loop_distribution));
+			       &destroy_p, only_patterns);
 	  if (destroy_p)
 	    loops_to_be_destroyed.safe_push (loop);