Bug 30858 - [4.3 Regression] ice for legal code with -O2 -ftree-vectorize
Summary: [4.3 Regression] ice for legal code with -O2 -ftree-vectorize
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 4.3.0
: P3 normal
Target Milestone: 4.3.0
Assignee: Not yet assigned to anyone
URL:
Keywords: ice-on-valid-code
Depends on:
Blocks:
 
Reported: 2007-02-19 11:59 UTC by David Binderman
Modified: 2007-06-18 05:46 UTC (History)
1 user (show)

See Also:
Host: x86_64-suse-linux
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2007-02-19 13:17:31


Attachments
C source code (51.98 KB, text/plain)
2007-02-19 11:59 UTC, David Binderman
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Binderman 2007-02-19 11:59:04 UTC
I just tried to compile Suse Linux package timidity-2.13.2-60
with the GNU C++ compiler version 4.3 snapshot 20070216.

The compiler said

ncurs_c.c:1196: internal compiler error: in get_initial_def_for_reduction, at tree-vect-transform.c:1036
Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://gcc.gnu.org/bugs.html> for instructions.

Preprocessed source code attached. Flags -O2 -ftree-vectorize required.

On the wider issue of the quality of the vectorizer, I
have thrown most of Suse Linux 10.3 at it and it has crashed
in a few places.

I suspect there would be considerable value at getting some
other distribution [ Debian ?], maybe on another type of 
machine [ PPC64 ?], and flushing out a few more bugs in the optimizer.

You would need to ensure that -ftree-vectorize was switched
on for every compile.

Just a suggestion.
Comment 1 David Binderman 2007-02-19 11:59:46 UTC
Created attachment 13069 [details]
C source code
Comment 2 Dorit Naishlos 2007-02-19 12:45:10 UTC
Reduced testcase:

int foo (int ko)
{
 int j,i;
  for (j = 0; j < ko; j++)
   i += (i > 10) ? -5 : 7;
 return i;
}

Looking into it...
Comment 3 Dorit Naishlos 2007-02-19 12:56:03 UTC
(In reply to comment #0)

Thanks for exercising the vectorizer and reporting these bugs!

> On the wider issue of the quality of the vectorizer, I
> have thrown most of Suse Linux 10.3 at it and it has crashed
> in a few places.

only a few? :-)

> I suspect there would be considerable value at getting some
> other distribution [ Debian ?], maybe on another type of 
> machine [ PPC64 ?], and flushing out a few more bugs in the optimizer.
> You would need to ensure that -ftree-vectorize was switched
> on for every compile.
> Just a suggestion.

I agree. We are working on a cost model these days to make the vectorizer less greedy, hopefully as a step towards enabling vectorization on by default - which would help in flushing bugs out.

(Just as a side comment - FYI - most of the vectorizer bugs you opened so far in the last few days (30771, 30795, 30843) are related to features that were added *very* recently).
Comment 4 Richard Biener 2007-02-19 13:17:31 UTC
Confirmed.
Comment 5 Dorit Naishlos 2007-02-19 14:12:14 UTC
Looks like I wasn't careful enough with my fix for PR30771. Here is a fix for that fix I'm now testing:

Index: tree-vect-analyze.c
===================================================================
--- tree-vect-analyze.c	(revision 122128)
+++ tree-vect-analyze.c	(working copy)
@@ -124,10 +124,11 @@

 	  /* Two cases of "relevant" phis: those that define an
 	     induction that is used in the loop, and those that
-	     define a reduction.  */
+	     directly define a reduction.  */
 	  if ((STMT_VINFO_RELEVANT (stmt_info) == vect_used_in_loop
 	       && STMT_VINFO_DEF_TYPE (stmt_info) == vect_induction_def)
-	      || (STMT_VINFO_RELEVANT (stmt_info) == vect_used_by_reduction
+	      || (STMT_VINFO_RELEVANT (stmt_info) ==
+					   vect_used_directly_by_reduction
 		  && STMT_VINFO_DEF_TYPE (stmt_info) == vect_reduction_def))
             {
 	      gcc_assert (!STMT_VINFO_VECTYPE (stmt_info));
@@ -328,8 +329,12 @@
 	    return false;
 	  }

-	  if (STMT_VINFO_RELEVANT (stmt_info) == vect_used_in_loop
-	      && STMT_VINFO_DEF_TYPE (stmt_info) != vect_induction_def)
+	  if ((STMT_VINFO_RELEVANT (stmt_info) == vect_used_in_loop
+	       && STMT_VINFO_DEF_TYPE (stmt_info) != vect_induction_def)
+	      || (STMT_VINFO_RELEVANT (stmt_info) >
+                                           vect_used_directly_by_reduction
+                  && STMT_VINFO_DEF_TYPE (stmt_info) == vect_reduction_def))
+
 	    {
 	      /* Most likely a reduction-like computation that is used
 		 in the loop.  */
@@ -2313,9 +2318,11 @@
       if (STMT_VINFO_DEF_TYPE (stmt_vinfo) == vect_reduction_def)
 	{
 	  gcc_assert (relevant == vect_unused_in_loop && live_p);
-	  relevant = vect_used_by_reduction;
+	  relevant = vect_used_directly_by_reduction;
 	  live_p = false;
 	}
+      else if (relevant == vect_used_directly_by_reduction)
+	relevant = vect_used_by_reduction;

       i = 0;
       FOR_EACH_SSA_TREE_OPERAND (use, stmt, iter, SSA_OP_USE)
Index: tree-vectorizer.h
===================================================================
--- tree-vectorizer.h	(revision 122128)
+++ tree-vectorizer.h	(working copy)
@@ -175,6 +175,7 @@
 /* Indicates whether/how a variable is used in the loop.  */
 enum vect_relevant {
   vect_unused_in_loop = 0,
+  vect_used_directly_by_reduction,
   vect_used_by_reduction,
   vect_used_in_loop
 };

Comment 6 Dorit Naishlos 2007-02-20 22:56:14 UTC
proposed patches - http://gcc.gnu.org/ml/gcc-patches/2007-02/msg01734.html

> I have thrown most of Suse Linux 10.3 at it and it has crashed
> in a few places.

would you mind giving these patches a try? (to see what's the next ICE...?)


Comment 7 David Binderman 2007-02-21 12:17:23 UTC
(In reply to comment #6)
> proposed patches - http://gcc.gnu.org/ml/gcc-patches/2007-02/msg01734.html

> would you mind giving these patches a try? (to see what's the next ICE...?)

If the patches have been applied to gcc mainstream,
then they will appear in the next 4.3 snapshot, due 23 Feb,
and so I will test them then.

Is this acceptable ?
Comment 8 Dorit Naishlos 2007-02-21 19:31:11 UTC
> Is this acceptable ?

sure, thanks
Comment 9 patchapp@dberlin.org 2007-02-22 01:01:12 UTC
Subject: Bug number PR30858

A patch for this bug has been added to the patch tracker.
The mailing list url for the patch is http://gcc.gnu.org/ml/gcc-patches/2007-02/msg01774.html
Comment 10 dorit 2007-02-22 08:16:29 UTC
Subject: Bug 30858

Author: dorit
Date: Thu Feb 22 08:16:18 2007
New Revision: 122220

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=122220
Log:
        PR tree-optimization/30858
        * tree-vectorizer.c (vect_is_simple_reduction): Check that the stmts
        in the reduction cycle have a single use in the loop.
        * tree-vectorizer.h (relevant): Add documentation.


Added:
    trunk/gcc/testsuite/gcc.dg/vect/pr30858.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/tree-vectorizer.c
    trunk/gcc/tree-vectorizer.h

Comment 11 Andrew Pinski 2007-06-18 05:46:57 UTC
Fixed.  This should have been closed a while back.