Summary: | [4.3/4.4/4.5 Regression] possible integer wrong code bug | ||
---|---|---|---|
Product: | gcc | Reporter: | John Regehr <regehr> |
Component: | rtl-optimization | Assignee: | Richard Biener <rguenth> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | chenyang, gcc-bugs, zadeck |
Priority: | P2 | Keywords: | wrong-code |
Version: | 4.5.0 | ||
Target Milestone: | 4.3.5 | ||
Host: | Target: | i?86-*-* x86_64-*-* | |
Build: | Known to work: | 4.3.5 4.4.4 4.5.0 | |
Known to fail: | 4.3.4 4.4.3 | Last reconfirmed: | 2010-02-04 10:11:02 |
Description
John Regehr
2010-02-04 05:45:22 UTC
Confirmed. Fails with -O -fno-tree-pta as well. extern void abort (void); static int g[1]; static int *p = &g[0]; static int *q = &g[0]; int main(void) { g[0] = 1; *p = 0; *p = *q; if (g[0] != 0) abort (); return 0; } dse1 deletes insn 7 in (insn 7 6 8 2 t.c:11 (set (mem:SI (reg/f:DI 58 [ p.0 ]) [0 S4 A32]) (const_int 0 [0x0])) 47 {*movsi_1} (nil)) (insn 8 7 9 2 t.c:12 (set (reg/f:DI 63 [ q ]) (mem/u/f/c/i:DI (symbol_ref:DI ("q") [flags 0x2] <var_decl 0x7ffff5ae7140 q>) [0 q+0 S8 A64])) 89 {*movdi_1_rex64} (nil)) (insn 9 8 10 2 t.c:12 (set (reg:SI 64) (mem:SI (reg/f:DI 63 [ q ]) [0 S4 A32])) 47 {*movsi_1} (expr_list:REG_DEAD (reg/f:DI 63 [ q ]) (nil))) (insn 10 9 11 2 t.c:12 (set (mem:SI (reg/f:DI 58 [ p.0 ]) [0 S4 A32]) (reg:SI 64)) 47 {*movsi_1} (expr_list:REG_DEAD (reg:SI 64) (expr_list:REG_DEAD (reg/f:DI 58 [ p.0 ]) (nil)))) The two canonical RTXen that are not detected as conflicting are (mem/u/f/c/i:DI (symbol_ref:DI ("q") [flags 0x2] <var_decl 0x7ffff5ae7140 q>) [0 q+0 S8 A64]) (mem/u/f/c/i:DI (symbol_ref:DI ("p") [flags 0x2] <var_decl 0x7ffff5ae70a0 p>) [0 p+0 S8 A64]) I will have a look. Well, dse puts (mem/u/f/c/i:DI (symbol_ref:DI ("q") [flags 0x2] <var_decl 0x7ffff5ae7140 q>) [0 q+0 S8 A64]) (mem/u/f/c/i:DI (symbol_ref:DI ("p") [flags 0x2] <var_decl 0x7ffff5ae70a0 p>) [0 p+0 S8 A64]) into different groups: **scanning insn=9 cselib value 2 0x10f8e58 (reg/f:DI 63 [ q ]) cselib lookup (reg/f:DI 63 [ q ]) => 2 mem: (reg/f:DI 63 [ q ]) after canon_rtx address: (mem/u/f/c/i:DI (symbol_ref:DI ("q") [flags 0x2] <var_decl 0x7ffff5ae7140 q>) [0 q+0 S8 A64]) gid=2 offset=0 processing const load gid=2[0..4) mems_found = 0, cannot_delete = true cselib lookup (mem:SI (reg/f:DI 63 [ q ]) [0 S4 A32]) => 0 **scanning insn=10 cselib lookup (reg/f:DI 58 [ p.0 ]) => 1 mem: (reg/f:DI 58 [ p.0 ]) after canon_rtx address: (mem/u/f/c/i:DI (symbol_ref:DI ("p") [flags 0x2] <var_decl 0x7ffff5ae70a0 p>) [0 p+0 S8 A64]) gid=1 offset=0 processing const base store gid=1[0..4) trying store in insn=7 gid=1[0..4) just because the addresses are MEM_READONLY_P. But that obviously does not mean they do not point to the same thing - no idea what implementor had in mind here. Kenny? The following fixes this for me: Index: dse.c =================================================================== --- dse.c (revision 156468) +++ dse.c (working copy) @@ -1015,9 +1015,6 @@ const_or_frame_p (rtx x) { switch (GET_CODE (x)) { - case MEM: - return MEM_READONLY_P (x); - case CONST: case CONST_INT: case CONST_DOUBLE: The only addresses treated as the dse "constant" kind should be symbol-refs. Or we need to lookup the constant initializer the constant mem refers to and use that (but I have no idea if that's easily possible on RTL). Richi, you are, of course, correct. kenny Re comment #4, there are two possibilities to fix this: 1) as you say, don't regard MEM addresses (i.e. used in double indirection) as const_or_frame_p, because that would put different (but runtime-same) bases into different groups, or 2) fix this marvel in check_mem_read_rtx: if (group_id == store_info->group_id) ... /* else The else case that is missing here is that the bases are constant but different. There is nothing to do here because there is no overlap. */ Clearly the comment is wrong, base addresses can be constant, different for rtx_equal purposes, but still be the same at runtime. When going the (2) way we would need to ask the oracle, which most of the time would probably say "don't know" anyway, and be slow. Hence not regarding such base addresses as interesting for the global problem seems to be the better fix. Also fails with 4.3.4 and 4.4.2 if you do -fno-tree-ccp -fno-tree-fre (I have a fix for the CCP optimization regression as well). Subject: Bug 42952 Author: rguenth Date: Thu Feb 4 16:14:17 2010 New Revision: 156494 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=156494 Log: 2010-02-04 Richard Guenther <rguenther@suse.de> PR rtl-optimization/42952 * dse.c (const_or_frame_p): Remove MEM handling. * gcc.dg/torture/pr42952.c: New testcase. Added: trunk/gcc/testsuite/gcc.dg/torture/pr42952.c Modified: trunk/gcc/ChangeLog trunk/gcc/dse.c trunk/gcc/testsuite/ChangeLog Subject: Bug 42952 Author: rguenth Date: Thu Feb 4 16:18:01 2010 New Revision: 156495 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=156495 Log: 2010-02-04 Richard Guenther <rguenther@suse.de> PR rtl-optimization/42952 * dse.c (const_or_frame_p): Remove MEM handling. * gcc.dg/torture/pr42952.c: New testcase. Added: branches/gcc-4_4-branch/gcc/testsuite/gcc.dg/torture/pr42952.c Modified: branches/gcc-4_4-branch/gcc/ChangeLog branches/gcc-4_4-branch/gcc/dse.c branches/gcc-4_4-branch/gcc/testsuite/ChangeLog Subject: Bug 42952 Author: rguenth Date: Thu Feb 4 16:21:47 2010 New Revision: 156496 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=156496 Log: 2010-02-04 Richard Guenther <rguenther@suse.de> PR rtl-optimization/42952 * dse.c (const_or_frame_p): Remove MEM handling. * gcc.dg/torture/pr42952.c: New testcase. Added: branches/gcc-4_3-branch/gcc/testsuite/gcc.dg/torture/pr42952.c Modified: branches/gcc-4_3-branch/gcc/ChangeLog branches/gcc-4_3-branch/gcc/dse.c branches/gcc-4_3-branch/gcc/testsuite/ChangeLog Fixed. |