This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix RTL DSE (PR rtl-optimization/68955)


Hi!

The following testcase is miscompiled on i686-linux at -O3.
The bug is in DSE record_store, which for group_id < 0 uses mem_addr
set to result of get_addr (base->val_rtx) (plus optional offset),
which is fine for canon_true_dependence with other MEMs in that function,
but we also store that address in store_info.  The problem is if later on
e.g. some read uses the same e.g. hard register as get_addr returned, but
that register contains at that later point a different value. 
canon_true_dependence then happily returns the read does not alias the
store, although it might.
The fix is to store the VALUE (plus optional offset) into
store_info->mem_addr instead, then at some later insn when get_addr is
called on it it will either return the same register or expression (if it
has not changed), or some different one otherwise.

Bootstrapped/regtested on x86_64-linux and i686-linux, I've additionally
performed instrumented x86_64-linux and i686-linux bootstraps/regtests,
which recorded number of locally_deleted and globally_deleted from each
function, once without this patch, once with it.  Across both 64-bit and
32-bit bootstrap/regtest, both unpatched and patched had the same number
151999 of globally_deleted, and unpatched had 74828 locally_deleted, while
patched 74849 locally_deleted.  Thus the patch should in addition to
actually fixing a real bug not really decrease number of DSEd stores
significant way, but sometimes even improve it.

Ok for trunk?

2016-01-15  Jakub Jelinek  <jakub@redhat.com>

	PR rtl-optimization/68955
	* dse.c (record_store): For group_id < 0, ensure mem_addr is not
	result of get_addr.

	* gcc.dg/torture/pr68955.c: New test.

--- gcc/dse.c.jj	2016-01-04 14:55:51.000000000 +0100
+++ gcc/dse.c	2016-01-15 19:25:31.323384174 +0100
@@ -1653,6 +1653,15 @@ record_store (rtx body, bb_info_t bb_inf
   insn_info->store_rec = store_info;
   store_info->mem = mem;
   store_info->alias_set = spill_alias_set;
+  if (spill_alias_set == 0 && group_id < 0)
+    {
+      /* Ensure we store address with VALUE in it, instead of result of
+	 get_addr on it, otherwise if the registers in get_addr change,
+	 we will not notice possible aliasing.  */
+      mem_addr = base->val_rtx;
+      if (offset)
+	mem_addr = plus_constant (get_address_mode (mem), mem_addr, offset);
+    }
   store_info->mem_addr = mem_addr;
   store_info->cse_base = base;
   if (width > HOST_BITS_PER_WIDE_INT)
--- gcc/testsuite/gcc.dg/torture/pr68955.c.jj	2016-01-15 19:32:00.347979523 +0100
+++ gcc/testsuite/gcc.dg/torture/pr68955.c	2016-01-15 19:31:39.000000000 +0100
@@ -0,0 +1,41 @@
+/* PR rtl-optimization/68955 */
+/* { dg-do run } */
+/* { dg-output "ONE1ONE" } */
+
+int a, b, c, d, g, m;
+int i[7][7][5] = { { { 5 } }, { { 5 } },
+		   { { 5 }, { 5 }, { 5 }, { 5 }, { 5 }, { -1 } } };
+static int j = 11;
+short e, f, h, k, l;
+
+static void
+foo ()
+{
+  for (; e < 5; e++)
+    for (h = 3; h; h--)
+      {
+	for (g = 1; g < 6; g++)
+	  {
+	    m = c == 0 ? b : b / c;
+	    i[e][1][e] = i[1][1][1] | (m & l) && f;
+	  }
+	for (k = 0; k < 6; k++)
+	  {
+	    for (d = 0; d < 6; d++)
+	      i[1][e][h] = i[h][k][e] >= l;
+	    i[e + 2][h + 3][e] = 6 & l;
+	    i[2][1][2] = a;
+	    for (; j < 5;)
+	      for (;;)
+		;
+	  }
+      }
+}
+
+int
+main ()
+{
+  foo ();
+  __builtin_printf ("ONE%dONE\n", i[1][0][2]);
+  return 0;
+}

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]