Fix slpeel_update_phi_nodes_for_guard1 ICE (PR tree-optimization/59519)

Hi!

Since the r205959 SCEV changes for peeled chrec, apparently we can end up
with multiple PHIs on the to be vectorized loop that have the same arguments
(both on preheader and latch edges).  slpeel_update_phi_nodes_for_guard1
doesn't like that, it asserts that for the argument from the latch edge
we set_current_def only once, when it is shared by more than one PHI,
we would actually be trying to set it more than once.

What we create is just multiple PHIs on the *new_exit_bb that look like
_NN = PHI <loop_arg(X)>
but as they necessarily have the same value, IMHO it shouldn't matter
which one of those we record through set_current_def.

I've tried to trace where we'd call get_current_def on the loop_arg
besides the get_current_def call changed in the patch, but saw none even
on
struct S { int f0; } d;
int a[8] = { 0 }, b, c, e, f;

void
foo (void)
{
  for (; e < 1; e++)
    {
      for (b = 0; b < 7; b++)
	{
	  c |= (a[b + 1] != 0);
	  if (d.f0)
	    break;
	}
      f += b;
    }
}
where the value is actually used after the loop, in all cases the generated
IL looks sane.

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

2014-01-03  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/59519
	* tree-vect-loop-manip.c (slpeel_update_phi_nodes_for_guard1): Don't
	ICE if get_current_def (current_new_name) is already non-NULL, as long
	as it is a phi result of some other phi in *new_exit_bb that has
	the same argument.

	* gcc.dg/vect/pr59519-1.c: New test.
	* gcc.dg/vect/pr59519-2.c: New test.


	Jakub

Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>Since the r205959 SCEV changes for peeled chrec, apparently we can end
>up
>with multiple PHIs on the to be vectorized loop that have the same
>arguments
>(both on preheader and latch edges). 
>slpeel_update_phi_nodes_for_guard1
>doesn't like that, it asserts that for the argument from the latch edge
>we set_current_def only once, when it is shared by more than one PHI,
>we would actually be trying to set it more than once.
>
>What we create is just multiple PHIs on the *new_exit_bb that look like
>_NN = PHI <loop_arg(X)>
>but as they necessarily have the same value, IMHO it shouldn't matter
>which one of those we record through set_current_def.
>
>I've tried to trace where we'd call get_current_def on the loop_arg
>besides the get_current_def call changed in the patch, but saw none
>even
>on
>struct S { int f0; } d;
>int a[8] = { 0 }, b, c, e, f;
>
>void
>foo (void)
>{
>  for (; e < 1; e++)
>    {
>      for (b = 0; b < 7; b++)
>	{
>	  c |= (a[b + 1] != 0);
>	  if (d.f0)
>	    break;
>	}
>      f += b;
>    }
>}
>where the value is actually used after the loop, in all cases the
>generated
>IL looks sane.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok.

Thanks,
Richard.

>2014-01-03  Jakub Jelinek  <jakub@redhat.com>
>
>	PR tree-optimization/59519
>	* tree-vect-loop-manip.c (slpeel_update_phi_nodes_for_guard1): Don't
>	ICE if get_current_def (current_new_name) is already non-NULL, as long
>	as it is a phi result of some other phi in *new_exit_bb that has
>	the same argument.
>
>	* gcc.dg/vect/pr59519-1.c: New test.
>	* gcc.dg/vect/pr59519-2.c: New test.
>
>--- gcc/tree-vect-loop-manip.c.jj	2013-12-10 12:43:21.000000000 +0100
>+++ gcc/tree-vect-loop-manip.c	2014-01-02 16:29:21.983645769 +0100
>@@ -483,7 +483,18 @@ slpeel_update_phi_nodes_for_guard1 (edge
> 	  if (!current_new_name)
> 	    continue;
>         }
>-      gcc_assert (get_current_def (current_new_name) == NULL_TREE);
>+      tree new_name = get_current_def (current_new_name);
>+      /* Because of peeled_chrec optimization it is possible that we
>have
>+	 set this earlier.  Verify the PHI has the same value.  */
>+      if (new_name)
>+	{
>+	  gimple phi = SSA_NAME_DEF_STMT (new_name);
>+	  gcc_assert (gimple_code (phi) == GIMPLE_PHI
>+		      && gimple_bb (phi) == *new_exit_bb
>+		      && (PHI_ARG_DEF_FROM_EDGE (phi, single_exit (loop))
>+			  == loop_arg));
>+	  continue;
>+	}
> 
>       set_current_def (current_new_name, PHI_RESULT (new_phi));
>     }
>--- gcc/testsuite/gcc.dg/vect/pr59519-1.c.jj	2014-01-02
>16:39:07.743647669 +0100
>+++ gcc/testsuite/gcc.dg/vect/pr59519-1.c	2014-01-02 16:40:22.414276947
>+0100
>@@ -0,0 +1,19 @@
>+/* PR tree-optimization/59519 */
>+/* { dg-do compile } */
>+/* { dg-additional-options "-O3" } */
>+
>+int a, b, c, d;
>+
>+void
>+foo (void)
>+{
>+  for (; d; d++)
>+    for (b = 0; b < 14; b++)
>+      {
>+	c |= 1;
>+	if (a)
>+	  break;
>+      }
>+}
>+
>+/* { dg-final { cleanup-tree-dump "vect" } } */
>--- gcc/testsuite/gcc.dg/vect/pr59519-2.c.jj	2014-01-02
>16:39:10.726629480 +0100
>+++ gcc/testsuite/gcc.dg/vect/pr59519-2.c	2014-01-02 16:40:26.213249520
>+0100
>@@ -0,0 +1,20 @@
>+/* PR tree-optimization/59519 */
>+/* { dg-do compile } */
>+/* { dg-additional-options "-O3" } */
>+
>+struct S { int f0; } d;
>+int a[8] = { 0 }, b, c, e;
>+
>+void
>+foo (void)
>+{
>+  for (; e < 1; e++)
>+    for (b = 0; b < 7; b++)
>+      {
>+	c |= (a[b + 1] != 0);
>+	if (d.f0)
>+	  break;
>+      }
>+}
>+
>+/* { dg-final { cleanup-tree-dump "vect" } } */
>
>	Jakub