[Bug target/24378] New: gcc.dg/vect/pr24300.c (test for excess errors) fails

[Bug target/24378] New: gcc.dg/vect/pr24300.c (test for excess errors) fails

[jsm28 at gcc dot gnu dot org]

FAIL: gcc.dg/vect/pr24300.c (test for excess errors)
appeared on mainline on ia64-hp-hpux11.23 between 20051010 and 20051013.  This
is a new test added in that time.  The failure is for both -milp32 and -mlp64.

/scratch/gcc/nightly-2005-10-13-mainline/src/gcc-mainline/gcc/testsuite/gcc.dg/vect/pr24300.c:
In function 'bar':
/scratch/gcc/nightly-2005-10-13-mainline/src/gcc-mainline/gcc/testsuite/gcc.dg/vect/pr24300.c:7:
internal compiler error: Segmentation fault


-- 
           Summary: gcc.dg/vect/pr24300.c (test for excess errors) fails
           Product: gcc
           Version: 4.1.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: jsm28 at gcc dot gnu dot org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24378

[Bug target/24378] gcc.dg/vect/pr24300.c (test for excess errors) fails

[phython at gcc dot gnu dot org]

------- Comment #1 from phython at gcc dot gnu dot org  2005-10-16 05:47 -------
I'm also seeing this on ia64-linux.


-- 

phython at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
 GCC target triplet|                            |ia64-*-*
   Last reconfirmed|0000-00-00 00:00:00         |2005-10-16 05:47:12
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24378

[Bug target/24378] [4.1 Regression] gcc.dg/vect/pr24300.c (test for excess errors) fails

[phython at gcc dot gnu dot org]

------- Comment #2 from phython at gcc dot gnu dot org  2005-11-20 22:18 -------
 This testcase fails when we don't have optabs for REDUC trees.


-- 

phython at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
      Known to fail|                            |4.1.0
      Known to work|                            |4.0.2
            Summary|gcc.dg/vect/pr24300.c (test |[4.1 Regression]
                   |for excess errors) fails    |gcc.dg/vect/pr24300.c (test
                   |                            |for excess errors) fails


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24378

[Bug target/24378] [4.1 Regression] gcc.dg/vect/pr24300.c (test for excess errors) fails

[ebotcazou at gcc dot gnu dot org]

------- Comment #3 from ebotcazou at gcc dot gnu dot org  2005-12-05 08:52 -------
Explicitly confirmed on SPARC if that matters.  We should not segfault though.


-- 

ebotcazou at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot gnu dot
                   |                            |org
   Last reconfirmed|2005-11-18 05:59:12         |2005-12-05 08:52:19
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24378

[Bug target/24378] [4.1/4.2 Regression] gcc.dg/vect/pr24300.c (test for excess errors) fails

[ebotcazou at gcc dot gnu dot org]

------- Comment #4 from ebotcazou at gcc dot gnu dot org  2005-12-06 09:21 -------
Dorit, this is a bad interaction between versioning and reduction.

The compiler segfaults in tree-ssa-dce.c because a PHI node declared as
having 2 operands has only 1.  It is created by the vectorizer, namely
vect_create_epilog_for_reduction here:

  /*** 2. Create epilog code ***/

  /* 2.1 Create new loop-exit-phi to preserve loop-closed form:
        v_out1 = phi <v_loop>  */

  exit_bb = loop->single_exit->dest;
  new_phi = create_phi_node (SSA_NAME_VAR (vect_def), exit_bb);
  SET_PHI_ARG_DEF (new_phi, loop->single_exit->dest_idx, vect_def);

It turns out that exit_bb has 2 predecessors while the code implicitly
assumes it has only 1.  This is so because versioning has kicked in:

<L32>:;
  vect_p.37_65 = nd.1_55;
  addr2int0.38_67 = (int) vect_p.37_65;
  andmask.39_69 = addr2int0.38_67 & 7;
  if (andmask.39_69 == 0) goto <L33>; else goto <L34>;

<L33>:;

<L35>:;
  vect_p.43_87 = nd.1_55;
  vect_p.41_79 = (vector int *) vect_p.43_87;
  vect_cst_.46_104 = { 0, 0 };

  # ivtmp.48_110 = PHI <ivtmp.48_111(2), 0(19)>;
  # vect_var_.45_75 = PHI <vect_var_.45_5(2), vect_cst_.46_104(19)>;
  # ivtmp.44_78 = PHI <ivtmp.44_77(2), vect_p.41_79(19)>;
  # ivtmp.35_61 = PHI <ivtmp.35_63(2), 4(19)>;
  # n_18 = PHI <n_58(2), 0(19)>;
  # s_19 = PHI <s_59(2), 0(19)>;
<L0>:;
  s.0_51 = (unsigned int) s_19;
  D.1314_52 = s.0_51 * 4;
  D.1315_53 = (int *) D.1314_52;
  D.1317_56 = D.1315_53 + nd.1_55;
  vect_var_.40_76 = *ivtmp.44_78;
  D.1318_57 = *D.1317_56;
  vect_var_.45_5 = vect_var_.45_75 + vect_var_.40_76;
  n_58 = D.1318_57 + n_18;
  s_59 = s_19 + 1;
  ivtmp.35_63 = ivtmp.35_61 - 1;
  ivtmp.44_77 = ivtmp.44_78 + 8B;
  ivtmp.48_111 = ivtmp.48_110 + 1;
  if (ivtmp.48_111 < 2) goto <L19>; else goto <L7>;

<L19>:;
  goto <bb 1> (<L0>);

<L34>:;

  # ivtmp.35_2 = PHI <4(18), ivtmp.35_88(15)>;
  # n_60 = PHI <0(18), n_90(15)>;
  # s_62 = PHI <0(18), s_89(15)>;
<L31>:;
  s.0_64 = (unsigned int) s_62;
  D.1314_66 = s.0_64 * 4;
  D.1315_68 = (int *) D.1314_66;
  D.1317_92 = D.1315_68 + nd.1_55;
  D.1318_91 = *D.1317_92;
  n_90 = n_60 + D.1318_91;
  s_89 = s_62 + 1;
  ivtmp.35_88 = ivtmp.35_2 - 1;
  if (ivtmp.35_88 != 0) goto <L30>; else goto <L7>;

<L30>:;
  goto <bb 14> (<L31>);

  # vect_var_.45_105 = PHI <vect_var_.45_5(1), (14)>;
  # n_6 = PHI <n_58(1), n_90(14)>;
<L7>:;

The bogus PHI node is the last but one.

What's the best approach to fixing this?  Punting in vectorizable_reduction
if we know beforehand that the loop will be versioned?

  /* Same if the loop is to be versioned.  */
  if (VEC_length (tree, LOOP_VINFO_MAY_MISALIGN_STMTS (loop_vinfo)))
    return false;

Or can vect_create_epilog_for_reduction be enhanced to cope with versioning?

Thanks in advance.


-- 

ebotcazou at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |dorit at il dot ibm dot com


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24378

[Bug target/24378] [4.1/4.2 Regression] gcc.dg/vect/pr24300.c (test for excess errors) fails

[dorit at il dot ibm dot com]

------- Comment #5 from dorit at il dot ibm dot com  2005-12-07 16:35 -------
> What's the best approach to fixing this?  Punting in vectorizable_reduction
> if we know beforehand that the loop will be versioned?
>   /* Same if the loop is to be versioned.  */
>   if (VEC_length (tree, LOOP_VINFO_MAY_MISALIGN_STMTS (loop_vinfo)))
>     return false;

maybe just as a temporary work around, but this shouldn't be a reason to block
vectorization of reductions

> Or can vect_create_epilog_for_reduction be enhanced to cope with versioning?
> Thanks in advance.

I think that vect_create_epilog_for_reduction should be fixed as follows:
create a new basic block at the exit edge, put the reduction epilog code there,
and then properly update the phi node at the merge-block (exit_bb). i.e. we
start with:

==============================
L32: 
if (cond) goto vectorized_loop (L33) 
else      goto scalar_loop (L34)

L34:
L31:
  loop body
  n_90 += ...
  if (...) goto L30
  else     goto L7
L30: 
  goto L31

L33:
L35:
L0:
  loop body
  n_58 += ...
  if (...) goto L19
  else     goto L7
L19: 
  goto L0

L7:
n = phi (n_58, n_90)
==============================

Transform that into:

==============================
L32: 
if (cond) goto vectorized_loop (L33) 
else      goto scalar_loop (L34)

L34:
L31:
  loop body
  n_90 += ...
  if (...) goto L30
  else     goto L7
L30: 
  goto L31

L33:
L35:
L0:
  loop body
  vect_var_5 += ...             # new
  n_58 += ...   
  if (...) goto L19
  else     goto new_bb
L19: 
  goto L0

new_bb:
v_out = phi <vect_var_5>        # new
s_out = epilog_code <v_out>     # new
goto L7

L7:
n = phi (s_out, n_90)           # updated
==============================

i.e., the new scalar result s_out that we compute in the reduction epilog will
replace the old value n_58 in the phi node of the merge-block L7. 

One other issue to be careful about is that new_bb should also have all the
necessary loop-exit-phis in order to maintain loop-closed-form. Assuming that
after loop-versioning the code is in loop-closed-form, then all the necessary
loop-exit-phis are in L7. We'd probably need to scan the phis of L7, and create
a phi-node in new_bb for each phi-node in L7. e.g., if we had:

==============================
L7:
x_2 = phi (x_0, x_1)
n = phi (n_58, n_90)
==============================

We'd probably want to create the following:

==============================
new_bb:
x_100 = phi (x_0)                # "copied"     from L7
n_101 = phi (n_58)       # "copied"     from L7 (this is redundant, will get
DCE'ed)
v_out = phi <vect_var_5> # new phi
s_out = epilog_code <v_out>
goto L7

L7:
x_2 = phi (x_100, x_1)  # updated
n = phi (s_out, n_90)   # updated
==============================


Another option is to make sure that the code transformation routines in the
vectorizer could continue to make the assumption that the loop-exit-bb has a
single predecessor. One option is to fix loop-versioning to do that (create a
new_bb at the exit-edge of each loop version before the merging point), or by
fixing it up in the vectorizer immediately after loop versioning takes place. 
I think maybe this option is safer. It's basically doing the same as above but
earlier in the vectorizer, and independently of whether reduction takes place.
i.e. create this: 

==============================
new_bb:
x_100 = phi (x_0)                # "copied"     from L7
n_101 = phi (n_58)       # "copied"     from L7
goto L7

L7:
x_2 = phi (x_100, x_1)  # updated
n = phi (n_101, n_90)   # updated
==============================

This way the reduction-epilog-code creation can continue to work exactly as it
does now.

You can assign this to me, but I won't be able to get to this for the next
week, so maybe I better have it assigned to me when I actually have time to
work on this (in about a week, if it's not fixed by then). Either way is fine
with me. 


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24378

[Bug target/24378] [4.1/4.2 Regression] gcc.dg/vect/pr24300.c (test for excess errors) fails

[ebotcazou at gcc dot gnu dot org]

------- Comment #6 from ebotcazou at gcc dot gnu dot org  2005-12-07 18:18 -------
[Thanks for the very detailed plan.]

> You can assign this to me, but I won't be able to get to this for the next
> week, so maybe I better have it assigned to me when I actually have time to
> work on this (in about a week, if it's not fixed by then). Either way is fine
> with me. 

I think I can try to implement what you have sketched, but only for mainline,
as I'll first need to familiarize myself further with Tree-SSA and the
vectorizer, and this is low priority for the time being.  I probably won't be
able to do so in time for the 4.1 release.

So it's up to you to decide what to do for the 4.1 branch.  I personally think
that punting would be just fine there, at least until we have a real fix.  So
I'd lean towards installing the workaround on the 4.1 branch only and leaving
the PR open until you or I get a chance to really tackling the problem.

What do you think?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24378

[Bug target/24378] [4.1/4.2 Regression] gcc.dg/vect/pr24300.c (test for excess errors) fails

[dorit at il dot ibm dot com]

------- Comment #7 from dorit at il dot ibm dot com  2005-12-07 20:49 -------
(In reply to comment #6)
Can you test the attached patch?
Unfortunrately it's relative to autovect-branch, but hopefully it would easily
apply to mainline/4.1. Unbootstrapped, hardly tested...


Index: tree-vect-transform.c
===================================================================
--- tree-vect-transform.c       (revision 108060)
+++ tree-vect-transform.c       (working copy)
@@ -5052,12 +5069,44 @@
       tree cond_expr_stmt_list = NULL_TREE;
       basic_block condition_bb;
       block_stmt_iterator cond_exp_bsi;
+      basic_block merge_bb;
+      basic_block new_exit_bb;
+      edge new_exit_e, e;
+      tree orig_phi, new_phi, arg;

       cond_expr = vect_create_cond_for_align_checks (loop_vinfo,
                                                      &cond_expr_stmt_list);
       initialize_original_copy_tables ();
       nloop = loop_version (loops, loop, cond_expr, &condition_bb, true);
       free_original_copy_tables();
+
+      /** Loop versioning violets an assumption we try to maintain during 
+        vectorization - that the loop exit block has a single predecessor.
+        After versioning, the exit block of both loop versions is the same
+        basic block (i.e. it has two predecessors). Just in order to simplify
+        following transformations in the vectorizer, we fix this situation
+        here by adding a new (empty) block on the exit-edge of the loop,
+        with the proper loop-exit phis to maintain loop-closed-form.  **/
+      
+      merge_bb = loop->single_exit->dest;
+      gcc_assert (EDGE_COUNT (merge_bb->preds) == 2);
+      new_exit_bb = split_edge (loop->single_exit);
+      add_bb_to_loop (new_exit_bb, loop->outer);
+      new_exit_e = loop->single_exit;
+      e = EDGE_SUCC (new_exit_bb, 0);
+
+      for (orig_phi = phi_nodes (merge_bb); orig_phi; 
+          orig_phi = PHI_CHAIN (orig_phi))
+       {
+          new_phi = create_phi_node (SSA_NAME_VAR (PHI_RESULT (orig_phi)),
+                                    new_exit_bb);
+          arg = PHI_ARG_DEF_FROM_EDGE (orig_phi, e);
+          add_phi_arg (new_phi, arg, new_exit_e);
+         SET_PHI_ARG_DEF (orig_phi, e->dest_idx, PHI_RESULT (new_phi));
+       } 
+
+      /** end loop-exit-fixes after versioning  **/
+
       update_ssa (TODO_update_ssa);
       cond_exp_bsi = bsi_last (condition_bb);
       bsi_insert_before (&cond_exp_bsi, cond_expr_stmt_list, BSI_SAME_STMT);


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24378

[Bug target/24378] [4.1/4.2 Regression] gcc.dg/vect/pr24300.c (test for excess errors) fails

[ebotcazou at gcc dot gnu dot org]

------- Comment #8 from ebotcazou at gcc dot gnu dot org  2005-12-08 08:32 -------
> Can you test the attached patch?

This kind of trick almost always works...

> Unfortunrately it's relative to autovect-branch, but hopefully it would easily
> apply to mainline/4.1. Unbootstrapped, hardly tested...

...and it works fine here.:-)  And AFAICS the loop is correctly vectorized. 

Quickstrapped and vector-regtested on SPARC 32-bit and 64-bit, the vectorizer
testsuite is now clean.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24378

[Bug target/24378] [4.1/4.2 Regression] gcc.dg/vect/pr24300.c (test for excess errors) fails

[dorit at il dot ibm dot com]

------- Comment #9 from dorit at il dot ibm dot com  2005-12-14 15:38 -------
Thanks for testing the patch. I finally submitted it:
http://gcc.gnu.org/ml/gcc-patches/2005-12/msg01071.html


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24378

[Bug tree-optimization/24378] [4.1/4.2 Regression] gcc.dg/vect/pr24300.c (test for excess errors) fails

[ebotcazou at gcc dot gnu dot org]

------- Comment #10 from ebotcazou at gcc dot gnu dot org  2005-12-14 16:38 -------
Recategorizing and assigning to Dorit.


-- 

ebotcazou at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |dorit at gcc dot gnu dot org
                   |dot org                     |
             Status|NEW                         |ASSIGNED
          Component|target                      |tree-optimization


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24378

[Bug tree-optimization/24378] [4.1/4.2 Regression] gcc.dg/vect/pr24300.c (test for excess errors) fails

[dorit at gcc dot gnu dot org]

------- Comment #11 from dorit at gcc dot gnu dot org  2005-12-18 08:46 -------
Subject: Bug 24378

Author: dorit
Date: Sun Dec 18 08:46:30 2005
New Revision: 108746

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=108746
Log:
        PR tree-optimization/24378
        * tree-vect-transform.c (vect_transform_loop): Create
single-predecessor
        basic-block after loop-versioning.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/tree-vect-transform.c


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24378

[Bug tree-optimization/24378] [4.1/4.2 Regression] gcc.dg/vect/pr24300.c (test for excess errors) fails

[dorit at gcc dot gnu dot org]

------- Comment #12 from dorit at gcc dot gnu dot org  2005-12-18 11:20 -------
Subject: Bug 24378

Author: dorit
Date: Sun Dec 18 11:20:17 2005
New Revision: 108750

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=108750
Log:
        PR tree-optimization/24378
        * tree-vect-transform.c (vect_transform_loop): Create
single-predecessor
        basic-block after loop-versioning.


Modified:
    branches/gcc-4_1-branch/gcc/ChangeLog
    branches/gcc-4_1-branch/gcc/tree-vect-transform.c


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24378

[Bug tree-optimization/24378] [4.1/4.2 Regression] gcc.dg/vect/pr24300.c (test for excess errors) fails

[ebotcazou at gcc dot gnu dot org]

------- Comment #13 from ebotcazou at gcc dot gnu dot org  2005-12-22 13:05 -------
http://gcc.gnu.org/ml/gcc-patches/2005-12/msg01071.html


-- 

ebotcazou at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.1.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=24378