Message ID | 20160116002658.GH3017@tucnak.redhat.com |
---|---|
State | New |
Headers | show |
> 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. I presume that the origin of the bug is: /* get_addr can only handle VALUE but cannot handle expr like: VALUE + OFFSET, so call get_addr to get original addr for mem_addr before plus_constant. */ mem_addr = get_addr (mem_addr); if (offset) mem_addr = plus_constant (get_address_mode (mem), mem_addr, offset); both in record_store and check_mem_read_rtx, so I wonder if we shouldn't bite the bullet and try enhancing get_addr since it's a mainline-only regression.
--- 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; +}