Avoid ICE for nested inductions (PR 83914)

Message ID 87po6641x0.fsf@linaro.org
State New
Headers show
Series
  • Avoid ICE for nested inductions (PR 83914)
Related show

Commit Message

Richard Sandiford Jan. 19, 2018, 9:45 a.m.
This testcase ICEd because we converted the initial value of an
induction to the vector element type even for nested inductions.
This isn't necessary because the initial expression is vectorised
normally, and it meant that init_expr was no longer the original
statement operand by the time we called vect_get_vec_def_for_operand.

Also, adding the conversion code here made the existing SLP conversion
redundant.

It looks like something went wrong when rebasing the peeling via masks
work on top of the support for SLP reductions, sorry about that.

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

Richard


2018-01-19  Richard Sandiford  <richard.sandiford@linaro.org>

gcc/
	PR tree-optimization/83914
	* tree-vect-loop.c (vectorizable_induction): Don't convert
	init_expr or apply the peeling adjustment for inductions
	that are nested within the vectorized loop.  Remove redundant
	conversion code.

gcc/testsuite/
	PR tree-optimization/83914
	* gcc.dg/vect/pr83914.c: New test.

Comments

Richard Biener Jan. 19, 2018, 11:53 a.m. | #1
On Fri, Jan 19, 2018 at 10:45 AM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:
> This testcase ICEd because we converted the initial value of an

> induction to the vector element type even for nested inductions.

> This isn't necessary because the initial expression is vectorised

> normally, and it meant that init_expr was no longer the original

> statement operand by the time we called vect_get_vec_def_for_operand.

>

> Also, adding the conversion code here made the existing SLP conversion

> redundant.

>

> It looks like something went wrong when rebasing the peeling via masks

> work on top of the support for SLP reductions, sorry about that.

>

> Tested on aarch64-linux-gnu, x86_64-linux-gnu and powerpc64le-linux-gnu.

> OK to install?


Ok.

Richard.

> Richard

>

>

> 2018-01-19  Richard Sandiford  <richard.sandiford@linaro.org>

>

> gcc/

>         PR tree-optimization/83914

>         * tree-vect-loop.c (vectorizable_induction): Don't convert

>         init_expr or apply the peeling adjustment for inductions

>         that are nested within the vectorized loop.  Remove redundant

>         conversion code.

>

> gcc/testsuite/

>         PR tree-optimization/83914

>         * gcc.dg/vect/pr83914.c: New test.

>

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

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

> --- gcc/tree-vect-loop.c        2018-01-16 15:13:24.938622242 +0000

> +++ gcc/tree-vect-loop.c        2018-01-19 09:36:33.409191362 +0000

> @@ -7678,28 +7678,33 @@ vectorizable_induction (gimple *phi,

>    init_expr = PHI_ARG_DEF_FROM_EDGE (phi,

>                                      loop_preheader_edge (iv_loop));

>

> -  /* Convert the initial value and step to the desired type.  */

>    stmts = NULL;

> -  init_expr = gimple_convert (&stmts, TREE_TYPE (vectype), init_expr);

> -  step_expr = gimple_convert (&stmts, TREE_TYPE (vectype), step_expr);

> -

> -  /* If we are using the loop mask to "peel" for alignment then we need

> -     to adjust the start value here.  */

> -  tree skip_niters = LOOP_VINFO_MASK_SKIP_NITERS (loop_vinfo);

> -  if (skip_niters != NULL_TREE)

> +  if (!nested_in_vect_loop)

>      {

> -      if (FLOAT_TYPE_P (vectype))

> -       skip_niters = gimple_build (&stmts, FLOAT_EXPR, TREE_TYPE (vectype),

> -                                   skip_niters);

> -      else

> -       skip_niters = gimple_convert (&stmts, TREE_TYPE (vectype),

> -                                     skip_niters);

> -      tree skip_step = gimple_build (&stmts, MULT_EXPR, TREE_TYPE (vectype),

> -                                    skip_niters, step_expr);

> -      init_expr = gimple_build (&stmts, MINUS_EXPR, TREE_TYPE (vectype),

> -                               init_expr, skip_step);

> +      /* Convert the initial value to the desired type.  */

> +      tree new_type = TREE_TYPE (vectype);

> +      init_expr = gimple_convert (&stmts, new_type, init_expr);

> +

> +      /* If we are using the loop mask to "peel" for alignment then we need

> +        to adjust the start value here.  */

> +      tree skip_niters = LOOP_VINFO_MASK_SKIP_NITERS (loop_vinfo);

> +      if (skip_niters != NULL_TREE)

> +       {

> +         if (FLOAT_TYPE_P (vectype))

> +           skip_niters = gimple_build (&stmts, FLOAT_EXPR, new_type,

> +                                       skip_niters);

> +         else

> +           skip_niters = gimple_convert (&stmts, new_type, skip_niters);

> +         tree skip_step = gimple_build (&stmts, MULT_EXPR, new_type,

> +                                        skip_niters, step_expr);

> +         init_expr = gimple_build (&stmts, MINUS_EXPR, new_type,

> +                                   init_expr, skip_step);

> +       }

>      }

>

> +  /* Convert the step to the desired type.  */

> +  step_expr = gimple_convert (&stmts, TREE_TYPE (vectype), step_expr);

> +

>    if (stmts)

>      {

>        new_bb = gsi_insert_seq_on_edge_immediate (pe, stmts);

> @@ -7718,15 +7723,6 @@ vectorizable_induction (gimple *phi,

>        /* Enforced above.  */

>        unsigned int const_nunits = nunits.to_constant ();

>

> -      /* Convert the init to the desired type.  */

> -      stmts = NULL;

> -      init_expr = gimple_convert (&stmts, TREE_TYPE (vectype), init_expr);

> -      if (stmts)

> -       {

> -         new_bb = gsi_insert_seq_on_edge_immediate (pe, stmts);

> -         gcc_assert (!new_bb);

> -       }

> -

>        /* Generate [VF*S, VF*S, ... ].  */

>        if (SCALAR_FLOAT_TYPE_P (TREE_TYPE (step_expr)))

>         {

> Index: gcc/testsuite/gcc.dg/vect/pr83914.c

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

> --- /dev/null   2018-01-19 09:30:49.543814408 +0000

> +++ gcc/testsuite/gcc.dg/vect/pr83914.c 2018-01-19 09:36:33.405191141 +0000

> @@ -0,0 +1,15 @@

> +/* { dg-do compile } */

> +/* { dg-additional-options "-O3" } */

> +

> +struct s { struct s *ptrs[16]; } *a, *b;

> +int c;

> +void

> +foo (int n)

> +{

> +  for (; n; a = b, n--)

> +    {

> +      b = a + 1;

> +      for (c = 8; c; c--)

> +       a->ptrs[c] = b;

> +    }

> +}

Patch

Index: gcc/tree-vect-loop.c
===================================================================
--- gcc/tree-vect-loop.c	2018-01-16 15:13:24.938622242 +0000
+++ gcc/tree-vect-loop.c	2018-01-19 09:36:33.409191362 +0000
@@ -7678,28 +7678,33 @@  vectorizable_induction (gimple *phi,
   init_expr = PHI_ARG_DEF_FROM_EDGE (phi,
 				     loop_preheader_edge (iv_loop));
 
-  /* Convert the initial value and step to the desired type.  */
   stmts = NULL;
-  init_expr = gimple_convert (&stmts, TREE_TYPE (vectype), init_expr);
-  step_expr = gimple_convert (&stmts, TREE_TYPE (vectype), step_expr);
-
-  /* If we are using the loop mask to "peel" for alignment then we need
-     to adjust the start value here.  */
-  tree skip_niters = LOOP_VINFO_MASK_SKIP_NITERS (loop_vinfo);
-  if (skip_niters != NULL_TREE)
+  if (!nested_in_vect_loop)
     {
-      if (FLOAT_TYPE_P (vectype))
-	skip_niters = gimple_build (&stmts, FLOAT_EXPR, TREE_TYPE (vectype),
-				    skip_niters);
-      else
-	skip_niters = gimple_convert (&stmts, TREE_TYPE (vectype),
-				      skip_niters);
-      tree skip_step = gimple_build (&stmts, MULT_EXPR, TREE_TYPE (vectype),
-				     skip_niters, step_expr);
-      init_expr = gimple_build (&stmts, MINUS_EXPR, TREE_TYPE (vectype),
-				init_expr, skip_step);
+      /* Convert the initial value to the desired type.  */
+      tree new_type = TREE_TYPE (vectype);
+      init_expr = gimple_convert (&stmts, new_type, init_expr);
+
+      /* If we are using the loop mask to "peel" for alignment then we need
+	 to adjust the start value here.  */
+      tree skip_niters = LOOP_VINFO_MASK_SKIP_NITERS (loop_vinfo);
+      if (skip_niters != NULL_TREE)
+	{
+	  if (FLOAT_TYPE_P (vectype))
+	    skip_niters = gimple_build (&stmts, FLOAT_EXPR, new_type,
+					skip_niters);
+	  else
+	    skip_niters = gimple_convert (&stmts, new_type, skip_niters);
+	  tree skip_step = gimple_build (&stmts, MULT_EXPR, new_type,
+					 skip_niters, step_expr);
+	  init_expr = gimple_build (&stmts, MINUS_EXPR, new_type,
+				    init_expr, skip_step);
+	}
     }
 
+  /* Convert the step to the desired type.  */
+  step_expr = gimple_convert (&stmts, TREE_TYPE (vectype), step_expr);
+
   if (stmts)
     {
       new_bb = gsi_insert_seq_on_edge_immediate (pe, stmts);
@@ -7718,15 +7723,6 @@  vectorizable_induction (gimple *phi,
       /* Enforced above.  */
       unsigned int const_nunits = nunits.to_constant ();
 
-      /* Convert the init to the desired type.  */
-      stmts = NULL;
-      init_expr = gimple_convert (&stmts, TREE_TYPE (vectype), init_expr);
-      if (stmts)
-	{
-	  new_bb = gsi_insert_seq_on_edge_immediate (pe, stmts);
-	  gcc_assert (!new_bb);
-	}
-
       /* Generate [VF*S, VF*S, ... ].  */
       if (SCALAR_FLOAT_TYPE_P (TREE_TYPE (step_expr)))
 	{
Index: gcc/testsuite/gcc.dg/vect/pr83914.c
===================================================================
--- /dev/null	2018-01-19 09:30:49.543814408 +0000
+++ gcc/testsuite/gcc.dg/vect/pr83914.c	2018-01-19 09:36:33.405191141 +0000
@@ -0,0 +1,15 @@ 
+/* { dg-do compile } */
+/* { dg-additional-options "-O3" } */
+
+struct s { struct s *ptrs[16]; } *a, *b;
+int c;
+void
+foo (int n)
+{
+  for (; n; a = b, n--)
+    {
+      b = a + 1;
+      for (c = 8; c; c--)
+	a->ptrs[c] = b;
+    }
+}