diff mbox series

Protect against min_profitable_iters going negative

Message ID 87a7zwwwa1.fsf@linaro.org
State New
Headers show
Series Protect against min_profitable_iters going negative | expand

Commit Message

Richard Sandiford Nov. 8, 2017, 4:49 p.m. UTC
We had:

      if (vec_outside_cost <= 0)
        min_profitable_iters = 0;
      else
        {
	  min_profitable_iters = ((vec_outside_cost - scalar_outside_cost)
				  * assumed_vf
				  - vec_inside_cost * peel_iters_prologue
				  - vec_inside_cost * peel_iters_epilogue)
				 / ((scalar_single_iter_cost * assumed_vf)
				    - vec_inside_cost);

which can lead to negative min_profitable_iters when the *_outside_costs
are the same and peel_iters_epilogue is nonzero (e.g. if we're peeling
for gaps).

This is tested as part of the patch that adds support for fully-predicated
loops.

Tested on aarch64-linux-gnu (both with and without SVE), x86_64-linux-gnu
and powerpc64le-linux-gnu.  OK to install?

Thanks,
Richard


2017-11-08  Richard Sandiford  <richard.sandiford@linaro.org>
	    Alan Hayward  <alan.hayward@arm.com>
	    David Sherwood  <david.sherwood@arm.com>

gcc/
	* tree-vect-loop.c (vect_estimate_min_profitable_iters): Make sure
	min_profitable_iters doesn't go negative.

Comments

Jeff Law Nov. 8, 2017, 10:52 p.m. UTC | #1
On 11/08/2017 09:49 AM, Richard Sandiford wrote:
> We had:
> 
>       if (vec_outside_cost <= 0)
>         min_profitable_iters = 0;
>       else
>         {
> 	  min_profitable_iters = ((vec_outside_cost - scalar_outside_cost)
> 				  * assumed_vf
> 				  - vec_inside_cost * peel_iters_prologue
> 				  - vec_inside_cost * peel_iters_epilogue)
> 				 / ((scalar_single_iter_cost * assumed_vf)
> 				    - vec_inside_cost);
> 
> which can lead to negative min_profitable_iters when the *_outside_costs
> are the same and peel_iters_epilogue is nonzero (e.g. if we're peeling
> for gaps).
> 
> This is tested as part of the patch that adds support for fully-predicated
> loops.
> 
> Tested on aarch64-linux-gnu (both with and without SVE), x86_64-linux-gnu
> and powerpc64le-linux-gnu.  OK to install?
> 
> Thanks,
> Richard
> 
> 
> 2017-11-08  Richard Sandiford  <richard.sandiford@linaro.org>
> 	    Alan Hayward  <alan.hayward@arm.com>
> 	    David Sherwood  <david.sherwood@arm.com>
> 
> gcc/
> 	* tree-vect-loop.c (vect_estimate_min_profitable_iters): Make sure
> 	min_profitable_iters doesn't go negative.
OK.
jeff
diff mbox series

Patch

Index: gcc/tree-vect-loop.c
===================================================================
--- gcc/tree-vect-loop.c	2017-11-08 16:35:04.770241799 +0000
+++ gcc/tree-vect-loop.c	2017-11-08 16:47:06.386264822 +0000
@@ -3651,23 +3651,24 @@  vect_estimate_min_profitable_iters (loop
 
   if ((scalar_single_iter_cost * assumed_vf) > (int) vec_inside_cost)
     {
-      if (vec_outside_cost <= 0)
+      min_profitable_iters = ((vec_outside_cost - scalar_outside_cost)
+			      * assumed_vf
+			      - vec_inside_cost * peel_iters_prologue
+			      - vec_inside_cost * peel_iters_epilogue);
+
+      if (min_profitable_iters <= 0)
         min_profitable_iters = 0;
       else
-        {
-	  min_profitable_iters = ((vec_outside_cost - scalar_outside_cost)
-				  * assumed_vf
-				  - vec_inside_cost * peel_iters_prologue
-				  - vec_inside_cost * peel_iters_epilogue)
-				 / ((scalar_single_iter_cost * assumed_vf)
-				    - vec_inside_cost);
+	{
+	  min_profitable_iters /= ((scalar_single_iter_cost * assumed_vf)
+				   - vec_inside_cost);
 
 	  if ((scalar_single_iter_cost * assumed_vf * min_profitable_iters)
 	      <= (((int) vec_inside_cost * min_profitable_iters)
 		  + (((int) vec_outside_cost - scalar_outside_cost)
 		     * assumed_vf)))
 	    min_profitable_iters++;
-        }
+	}
     }
   /* vector version will never be profitable.  */
   else