diff mbox

Fix PR49518

Message ID alpine.LNX.2.00.1107041336010.810@zhemvz.fhfr.qr
State New
Headers show

Commit Message

Richard Biener July 4, 2011, 11:38 a.m. UTC
Handling of negative steps broke one of the many asserts in
the vectorizer.  The following patch drops one that I can't
make sense of.  I think all asserts need comments - especially
this one would, as I can't see why using vf is correct to
test against and not nelements (and why <= vf and not < vf).

Well, ok?

Thanks,
Richard.

2011-07-04  Richard Guenther  <rguenther@suse.de>

	PR tree-optimization/49518
	* tree-vect-data-refs.c (vect_enhance_data_refs_alignment):
	Drop assert.

	* gcc.dg/torture/pr49518.c: New testcase.

Comments

Ira Rosen July 4, 2011, 12:18 p.m. UTC | #1
Richard Guenther <rguenther@suse.de> wrote on 04/07/2011 02:38:50 PM:

> Handling of negative steps broke one of the many asserts in
> the vectorizer.  The following patch drops one that I can't
> make sense of.  I think all asserts need comments - especially
> this one would, as I can't see why using vf is correct to
> test against and not nelements (and why <= vf and not < vf).

There is an explanation 10 rows above the assert. It doesn't make sense to
peel more than vf iterations (and not nelements, since for the case of
multiple types it may help to align more data-refs - see the comment in the
code). IIRC <= is for the case of aligned access, but I am not sure about
that, so maybe you are right.

I don't see how it is related to negative steps.

I think that the real reason for this failure is that the loads are
actually irrelevant (hence, vf=4 that doesn't take char loads into
account), but we don't check that when we analyze data-refs. So, in my
opinion, the proper fix will add such check.

Thanks,
Ira

>
> Well, ok?
>
> Thanks,
> Richard.
>
> 2011-07-04  Richard Guenther  <rguenther@suse.de>
>
>    PR tree-optimization/49518
>    * tree-vect-data-refs.c (vect_enhance_data_refs_alignment):
>    Drop assert.
>
>    * gcc.dg/torture/pr49518.c: New testcase.
>
> Index: gcc/tree-vect-data-refs.c
> ===================================================================
> --- gcc/tree-vect-data-refs.c   (revision 175800)
> +++ gcc/tree-vect-data-refs.c   (working copy)
> @@ -1552,7 +1552,6 @@ vect_enhance_data_refs_alignment (loop_v
>
>                for (j = 0; j < possible_npeel_number; j++)
>                  {
> -                  gcc_assert (npeel_tmp <= vf);
>                    vect_peeling_hash_insert (loop_vinfo, dr, npeel_tmp);
>                    npeel_tmp += nelements;
>                  }
> Index: gcc/testsuite/gcc.dg/torture/pr49518.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/torture/pr49518.c   (revision 0)
> +++ gcc/testsuite/gcc.dg/torture/pr49518.c   (revision 0)
> @@ -0,0 +1,19 @@
> +/* { dg-do compile } */
> +
> +int a, b;
> +struct S { unsigned int s, t, u; } c, d = { 0, 1, 0 };
> +
> +void
> +test (unsigned char z)
> +{
> +  char e[] = {0, 0, 0, 0, 1};
> +  for (c.s = 1; c.s; c.s++)
> +    {
> +      b = e[c.s];
> +      if (a)
> +   break;
> +      b = z >= c.u;
> +      if (d.t)
> +   break;
> +    }
> +}
diff mbox

Patch

Index: gcc/tree-vect-data-refs.c
===================================================================
--- gcc/tree-vect-data-refs.c	(revision 175800)
+++ gcc/tree-vect-data-refs.c	(working copy)
@@ -1552,7 +1552,6 @@  vect_enhance_data_refs_alignment (loop_v
 
               for (j = 0; j < possible_npeel_number; j++)
                 {
-                  gcc_assert (npeel_tmp <= vf);
                   vect_peeling_hash_insert (loop_vinfo, dr, npeel_tmp);
                   npeel_tmp += nelements;
                 }
Index: gcc/testsuite/gcc.dg/torture/pr49518.c
===================================================================
--- gcc/testsuite/gcc.dg/torture/pr49518.c	(revision 0)
+++ gcc/testsuite/gcc.dg/torture/pr49518.c	(revision 0)
@@ -0,0 +1,19 @@ 
+/* { dg-do compile } */
+
+int a, b;
+struct S { unsigned int s, t, u; } c, d = { 0, 1, 0 };
+
+void
+test (unsigned char z)
+{
+  char e[] = {0, 0, 0, 0, 1};
+  for (c.s = 1; c.s; c.s++)
+    {
+      b = e[c.s];
+      if (a)
+	break;
+      b = z >= c.u;
+      if (d.t)
+	break;
+    }
+}