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 <ja...@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
