diff mbox

[COMMITTED] fixed pr59651 & new test case

Message ID B71DF1153024A14EABB94E39368E44A604262537@SJEXCHMB13.corp.ad.broadcom.com
State New
Headers show

Commit Message

Bingfeng Mei Jan. 3, 2014, 3:42 p.m. UTC 
Jakub, thanks. Committed with suggested changes.

Bingfeng

-----Original Message-----
From: Jakub Jelinek [mailto:jakub@redhat.com] 
Sent: 03 January 2014 14:26
To: Bingfeng Mei
Cc: gcc-patches@gcc.gnu.org; tbelagod@arm.com
Subject: Re: [PING] [PATCH] fixed pr59651 & new test case

On Fri, Jan 03, 2014 at 02:07:03PM +0000, Bingfeng Mei wrote:
> This patch fixes pr59651.  The original regression for pr52943 only
> appears on AArch64 target.  I constructed a similar test that also exposes
> bug on x86-64.  The problem is that calculation of address range in alias
> versioning for loops with negative step is wrong during vectorization. 
> For example, for a loop that access int a[3] -> a[1], the old calculated
> address range is [a, a+12).  It should be [a+4, a+16) instead.

+extern void abort (void);
+int a[] = { 0, 0, 0, 0, 0, 0, 0, 6 };
+
+int b;
+int
+main ()
+{
+  for (;;)
+    {
+      b = 7;
+      for (; b; b -= 1)

Sounds like C-reduce weirdness, just write for (b = 7; b; --b)

+	a[b] = a[7] > 1;
+      break;
+    }
+  if (a[1] != 0)
+    abort ();
+  return 0;
+}

Ok with those changes.

	Jakub
diff mbox

Patch

--- tree-vect-loop-manip.c	(revision 206279)
+++ tree-vect-loop-manip.c	(working copy)
@@ -2241,12 +2241,26 @@  vect_create_cond_for_alias_checks (loop_
       tree seg_a_min = addr_base_a;
       tree seg_a_max = fold_build_pointer_plus (addr_base_a, segment_length_a);
       if (tree_int_cst_compare (DR_STEP (dr_a.dr), size_zero_node) < 0)
-	seg_a_min = seg_a_max, seg_a_max = addr_base_a;
+	{
+	  seg_a_min = 
+	    fold_build_pointer_plus (seg_a_max,
+				     TYPE_SIZE_UNIT (TREE_TYPE (DR_REF (dr_a.dr))));
+	  seg_a_max =
+	    fold_build_pointer_plus (addr_base_a, 
+				     TYPE_SIZE_UNIT (TREE_TYPE (DR_REF (dr_a.dr))));
+	}

Too long lines, can you create a temporary tree var for TYPE_SIZE_UNIT (...); value?
Can you add a comment why you do this?

--- testsuite/gcc.dg/torture/pr59651.c	(revision 0)
+++ testsuite/gcc.dg/torture/pr59651.c	(revision 0)

Perhaps better would be to put this test into gcc.dg/vect/pr59651.c
(with the required /* { dg-final { cleanup-tree-dump "vect" } } */ ),
so that it is also tested say on i686-linux with -msse2 etc.
Or duplicate between those two places (the body of the test can be
included in one of those two from the other place using relative path).

@@ -0,0 +1,20 @@ 
+/* { dg-do run } */
+