Don't try to vectorise COND_EXPR reduction chains (PR 84913)

Message ID 87605v0zq9.fsf@linaro.org
State New
Headers show
Series
  • Don't try to vectorise COND_EXPR reduction chains (PR 84913)
Related show

Commit Message

Richard Sandiford March 17, 2018, 10:21 a.m.
The testcase ICEd for both SVE and AVX512 because we were trying
to vectorise a chain of COND_EXPRs as a reduction and getting
confused by reduc_index == -1.

We normally handle reduction chains by vectorising all statements
except the last one normally, but that wouldn't work here for the
reasons explained in the comment.

Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?

Richard


2018-03-17  Richard Sandiford  <richard.sandiford@linaro.org>

gcc/
	PR tree-optimization/84913
	* tree-vect-loop.c (vectorizable_reduction): Don't try to
	vectorize chains of COND_EXPRs.

gcc/testsuite/
	PR tree-optimization/84913
	* gfortran.dg/vect/pr84913.f90: New test.

Comments

Richard Biener March 17, 2018, 12:47 p.m. | #1
On March 17, 2018 11:21:18 AM GMT+01:00, Richard Sandiford <richard.sandiford@linaro.org> wrote:
>The testcase ICEd for both SVE and AVX512 because we were trying

>to vectorise a chain of COND_EXPRs as a reduction and getting

>confused by reduc_index == -1.

>

>We normally handle reduction chains by vectorising all statements

>except the last one normally, but that wouldn't work here for the

>reasons explained in the comment.

>

>Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK to install?


OK. 

Richard. 

>Richard

>

>

>2018-03-17  Richard Sandiford  <richard.sandiford@linaro.org>

>

>gcc/

>	PR tree-optimization/84913

>	* tree-vect-loop.c (vectorizable_reduction): Don't try to

>	vectorize chains of COND_EXPRs.

>

>gcc/testsuite/

>	PR tree-optimization/84913

>	* gfortran.dg/vect/pr84913.f90: New test.

>

>Index: gcc/tree-vect-loop.c

>===================================================================

>--- gcc/tree-vect-loop.c	2018-03-01 08:20:43.850526185 +0000

>+++ gcc/tree-vect-loop.c	2018-03-17 10:19:19.289947549 +0000

>@@ -6788,6 +6788,30 @@ vectorizable_reduction (gimple *stmt, gi

>/* If we have a condition reduction, see if we can simplify it further.

> */

>   if (v_reduc_type == COND_REDUCTION)

>     {

>+      /* TODO: We can't yet handle reduction chains, since we need to

>treat

>+	 each COND_EXPR in the chain specially, not just the last one.

>+	 E.g. for:

>+

>+	    x_1 = PHI <x_3, ...>

>+	    x_2 = a_2 ? ... : x_1;

>+	    x_3 = a_3 ? ... : x_2;

>+

>+	 we're interested in the last element in x_3 for which a_2 || a_3

>+	 is true, whereas the current reduction chain handling would

>+	 vectorize x_2 as a normal VEC_COND_EXPR and only treat x_3

>+	 as a reduction operation.  */

>+      if (reduc_index == -1)

>+	{

>+	  if (dump_enabled_p ())

>+	    dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,

>+			     "conditional reduction chains not supported\n");

>+	  return false;

>+	}

>+

>+      /* vect_is_simple_reduction ensured that operand 2 is the

>+	 loop-carried operand.  */

>+      gcc_assert (reduc_index == 2);

>+

>       /* Loop peeling modifies initial value of reduction PHI, which

> 	 makes the reduction stmt to be transformed different to the

> 	 original stmt analyzed.  We need to record reduction code for

>Index: gcc/testsuite/gfortran.dg/vect/pr84913.f90

>===================================================================

>--- /dev/null	2018-03-17 08:19:33.716019995 +0000

>+++ gcc/testsuite/gfortran.dg/vect/pr84913.f90	2018-03-17

>10:19:19.286947657 +0000

>@@ -0,0 +1,13 @@

>+! { dg-do compile }

>+

>+function foo(a, b, c, n)

>+  integer :: a(n), b(n), c(n), n, i, foo

>+  foo = 0

>+  do i = 1, n

>+    if (a(i) .eq. b(i)) then

>+      foo = 1

>+    else if (a(i) .eq. c(i)) then

>+      foo = 2

>+    end if

>+  end do

>+end function foo

Patch

Index: gcc/tree-vect-loop.c
===================================================================
--- gcc/tree-vect-loop.c	2018-03-01 08:20:43.850526185 +0000
+++ gcc/tree-vect-loop.c	2018-03-17 10:19:19.289947549 +0000
@@ -6788,6 +6788,30 @@  vectorizable_reduction (gimple *stmt, gi
   /* If we have a condition reduction, see if we can simplify it further.  */
   if (v_reduc_type == COND_REDUCTION)
     {
+      /* TODO: We can't yet handle reduction chains, since we need to treat
+	 each COND_EXPR in the chain specially, not just the last one.
+	 E.g. for:
+
+	    x_1 = PHI <x_3, ...>
+	    x_2 = a_2 ? ... : x_1;
+	    x_3 = a_3 ? ... : x_2;
+
+	 we're interested in the last element in x_3 for which a_2 || a_3
+	 is true, whereas the current reduction chain handling would
+	 vectorize x_2 as a normal VEC_COND_EXPR and only treat x_3
+	 as a reduction operation.  */
+      if (reduc_index == -1)
+	{
+	  if (dump_enabled_p ())
+	    dump_printf_loc (MSG_MISSED_OPTIMIZATION, vect_location,
+			     "conditional reduction chains not supported\n");
+	  return false;
+	}
+
+      /* vect_is_simple_reduction ensured that operand 2 is the
+	 loop-carried operand.  */
+      gcc_assert (reduc_index == 2);
+
       /* Loop peeling modifies initial value of reduction PHI, which
 	 makes the reduction stmt to be transformed different to the
 	 original stmt analyzed.  We need to record reduction code for
Index: gcc/testsuite/gfortran.dg/vect/pr84913.f90
===================================================================
--- /dev/null	2018-03-17 08:19:33.716019995 +0000
+++ gcc/testsuite/gfortran.dg/vect/pr84913.f90	2018-03-17 10:19:19.286947657 +0000
@@ -0,0 +1,13 @@ 
+! { dg-do compile }
+
+function foo(a, b, c, n)
+  integer :: a(n), b(n), c(n), n, i, foo
+  foo = 0
+  do i = 1, n
+    if (a(i) .eq. b(i)) then
+      foo = 1
+    else if (a(i) .eq. c(i)) then
+      foo = 2
+    end if
+  end do
+end function foo