Jakub Jelinek <ja...@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 <ja...@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
