[PATCH] Fix ICEs in vect_finish_stmt_generation (PR tree-optimization/50133)

[PATCH] Fix ICEs in vect_finish_stmt_generation (PR tree-optimization/50133)

[Jakub Jelinek]

Hi!

The following testcase ICEs, because gsi_end_p (*gsi) and thus
there is no stmt after it from which to copy over the location.
As can be seen in the PR, we could do ugly hacks to retrieve locus
from previous stmt (non-debug of course) instead, but I'm probably missing
something obvious why we shouldn't take location from stmt itself
instead.  We've been doing that before, just PR37482 patch changed that
without an explanation.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2011-08-22  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/50133
	* tree-vect-stmts.c (vect_finish_stmt_generation): Copy location
	from stmt instead of some statement around gsi.

	* gcc.dg/pr50133.c: New test.

--- gcc/tree-vect-stmts.c.jj	2011-08-22 08:17:08.000000000 +0200
+++ gcc/tree-vect-stmts.c	2011-08-22 11:26:27.000000000 +0200
@@ -1419,7 +1419,6 @@ vect_finish_stmt_generation (gimple stmt
   stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
   loop_vec_info loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_info);
   bb_vec_info bb_vinfo = STMT_VINFO_BB_VINFO (stmt_info);
-  gimple_stmt_iterator si;
 
   gcc_assert (gimple_code (stmt) != GIMPLE_LABEL);
 
@@ -1434,13 +1433,7 @@ vect_finish_stmt_generation (gimple stmt
       print_gimple_stmt (vect_dump, vec_stmt, 0, TDF_SLIM);
     }
 
-  si = *gsi;
-  if (is_gimple_debug (gsi_stmt (si)))
-    {
-      gsi_next_nondebug (&si);
-      gcc_assert (!gsi_end_p (si));
-    }
-  gimple_set_location (vec_stmt, gimple_location (gsi_stmt (si)));
+  gimple_set_location (vec_stmt, gimple_location (stmt));
 }
 
 /* Checks if CALL can be vectorized in type VECTYPE.  Returns
--- gcc/testsuite/gcc.dg/pr50133.c.jj	2011-08-22 11:27:15.000000000 +0200
+++ gcc/testsuite/gcc.dg/pr50133.c	2011-08-22 11:12:04.000000000 +0200
@@ -0,0 +1,18 @@
+/* PR tree-optimization/50133 */
+/* { dg-do compile } */
+/* { dg-options "-O -ftree-vectorize -fno-tree-loop-im" } */
+
+extern int A[], B[];
+
+void
+foo (int z)
+{
+  int j, i;
+  for (j = 0; j < 32; j++)
+    {
+      int a = A[0];
+      for (i = 0; i < 16; i++)
+	a = A[i] ? a : z;
+      B[j] = a;
+    }
+}

	Jakub

Re: [PATCH] Fix ICEs in vect_finish_stmt_generation (PR tree-optimization/50133)

[Richard Guenther]

On Mon, Aug 22, 2011 at 4:22 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> The following testcase ICEs, because gsi_end_p (*gsi) and thus
> there is no stmt after it from which to copy over the location.
> As can be seen in the PR, we could do ugly hacks to retrieve locus
> from previous stmt (non-debug of course) instead, but I'm probably missing
> something obvious why we shouldn't take location from stmt itself
> instead.  We've been doing that before, just PR37482 patch changed that
> without an explanation.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

> 2011-08-22  Jakub Jelinek  <jakub@redhat.com>
>
>        PR tree-optimization/50133
>        * tree-vect-stmts.c (vect_finish_stmt_generation): Copy location
>        from stmt instead of some statement around gsi.
>
>        * gcc.dg/pr50133.c: New test.
>
> --- gcc/tree-vect-stmts.c.jj    2011-08-22 08:17:08.000000000 +0200
> +++ gcc/tree-vect-stmts.c       2011-08-22 11:26:27.000000000 +0200
> @@ -1419,7 +1419,6 @@ vect_finish_stmt_generation (gimple stmt
>   stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
>   loop_vec_info loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_info);
>   bb_vec_info bb_vinfo = STMT_VINFO_BB_VINFO (stmt_info);
> -  gimple_stmt_iterator si;
>
>   gcc_assert (gimple_code (stmt) != GIMPLE_LABEL);
>
> @@ -1434,13 +1433,7 @@ vect_finish_stmt_generation (gimple stmt
>       print_gimple_stmt (vect_dump, vec_stmt, 0, TDF_SLIM);
>     }
>
> -  si = *gsi;
> -  if (is_gimple_debug (gsi_stmt (si)))
> -    {
> -      gsi_next_nondebug (&si);
> -      gcc_assert (!gsi_end_p (si));
> -    }
> -  gimple_set_location (vec_stmt, gimple_location (gsi_stmt (si)));
> +  gimple_set_location (vec_stmt, gimple_location (stmt));
>  }
>
>  /* Checks if CALL can be vectorized in type VECTYPE.  Returns
> --- gcc/testsuite/gcc.dg/pr50133.c.jj   2011-08-22 11:27:15.000000000 +0200
> +++ gcc/testsuite/gcc.dg/pr50133.c      2011-08-22 11:12:04.000000000 +0200
> @@ -0,0 +1,18 @@
> +/* PR tree-optimization/50133 */
> +/* { dg-do compile } */
> +/* { dg-options "-O -ftree-vectorize -fno-tree-loop-im" } */
> +
> +extern int A[], B[];
> +
> +void
> +foo (int z)
> +{
> +  int j, i;
> +  for (j = 0; j < 32; j++)
> +    {
> +      int a = A[0];
> +      for (i = 0; i < 16; i++)
> +       a = A[i] ? a : z;
> +      B[j] = a;
> +    }
> +}
>
>        Jakub
>

Re: [PATCH] Fix ICEs in vect_finish_stmt_generation (PR tree-optimization/50133)

[Ira Rosen]

Jakub Jelinek <jakub@redhat.com> wrote on 22/08/2011 05:22:59 PM:

> Hi!
>
> The following testcase ICEs, because gsi_end_p (*gsi) and thus
> there is no stmt after it from which to copy over the location.
> As can be seen in the PR, we could do ugly hacks to retrieve locus
> from previous stmt (non-debug of course) instead, but I'm probably
missing
> something obvious why we shouldn't take location from stmt itself
> instead.

We insert vector statement before GSI, which may be different from location
of STMT.

Ira

> We've been doing that before, just PR37482 patch changed that
> without an explanation.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2011-08-22  Jakub Jelinek  <jakub@redhat.com>
>
>    PR tree-optimization/50133
>    * tree-vect-stmts.c (vect_finish_stmt_generation): Copy location
>    from stmt instead of some statement around gsi.
>
>    * gcc.dg/pr50133.c: New test.
>
> --- gcc/tree-vect-stmts.c.jj   2011-08-22 08:17:08.000000000 +0200
> +++ gcc/tree-vect-stmts.c   2011-08-22 11:26:27.000000000 +0200
> @@ -1419,7 +1419,6 @@ vect_finish_stmt_generation (gimple stmt
>    stmt_vec_info stmt_info = vinfo_for_stmt (stmt);
>    loop_vec_info loop_vinfo = STMT_VINFO_LOOP_VINFO (stmt_info);
>    bb_vec_info bb_vinfo = STMT_VINFO_BB_VINFO (stmt_info);
> -  gimple_stmt_iterator si;
>
>    gcc_assert (gimple_code (stmt) != GIMPLE_LABEL);
>
> @@ -1434,13 +1433,7 @@ vect_finish_stmt_generation (gimple stmt
>        print_gimple_stmt (vect_dump, vec_stmt, 0, TDF_SLIM);
>      }
>
> -  si = *gsi;
> -  if (is_gimple_debug (gsi_stmt (si)))
> -    {
> -      gsi_next_nondebug (&si);
> -      gcc_assert (!gsi_end_p (si));
> -    }
> -  gimple_set_location (vec_stmt, gimple_location (gsi_stmt (si)));
> +  gimple_set_location (vec_stmt, gimple_location (stmt));
>  }
>
>  /* Checks if CALL can be vectorized in type VECTYPE.  Returns
> --- gcc/testsuite/gcc.dg/pr50133.c.jj   2011-08-22 11:27:15.000000000
+0200
> +++ gcc/testsuite/gcc.dg/pr50133.c   2011-08-22 11:12:04.000000000 +0200
> @@ -0,0 +1,18 @@
> +/* PR tree-optimization/50133 */
> +/* { dg-do compile } */
> +/* { dg-options "-O -ftree-vectorize -fno-tree-loop-im" } */
> +
> +extern int A[], B[];
> +
> +void
> +foo (int z)
> +{
> +  int j, i;
> +  for (j = 0; j < 32; j++)
> +    {
> +      int a = A[0];
> +      for (i = 0; i < 16; i++)
> +   a = A[i] ? a : z;
> +      B[j] = a;
> +    }
> +}
>
>    Jakub