See thread at <http://gcc.gnu.org/ml/gcc-patches/2003-12/msg00236.html> regarding the setting of RTX_UNCHANGING_P and its affect on clrstr, etc.
If I read the thread correctly this is a regression.
Hey! Hey! Don't forget SPARC :-)
Subject: Re: [3.4 Regression] 20031202-1.c is miscompiled on all PA ports > If I read the thread correctly this is a regression. I'm not sure that I would class this as a regression. If so, it also is a regression for 3.3. Jakub's patch changed the nature of the bug but it didn't provide a complete fix. The testcase is new. Dave
It's a regression on mainline, at least on SPARC: we have a testcase that passed before the patch and doesn't after. But I don't see the bug on the 3.3 branch, because RTX_UNCHANGING_P is not set anymore at all: before Jakub's patch, the situation was (1,1) on mainline and (0,1) on branch, so the two MEMs were detected as aliasing on mainline and not on branch. After Jakub's patch, the situation is (1,0) on mainline and (0,0) on branch, so the two MEMs are detected as aliasing on branch but not on mainline.
This testcase was failing on all targets as for a while and has been fixed now, does the patch also fix it for happa and sparc?
never mind about my pervious comment, I misread what the patch was for (stupid close testcases names).
Subject: Re: [3.4 Regression] gcc.dg/20031202-1.c is > This testcase was failing on all targets as for a while and has been fixed > now, does the > patch also fix it for happa and sparc? I can confirm that the problem is fixed on hppa-unknown-linux-gnu. Dave
Subject: Re: [3.4 Regression] gcc.dg/20031202-1.c > > This testcase was failing on all targets as for a while and has been fixed > > now, does the > > patch also fix it for happa and sparc? > > I can confirm that the problem is fixed on hppa-unknown-linux-gnu. Sorry, it's gcc.dg/20031201-1.c that's fixed. gcc.dg/20031202-1.c still fails. Dave
On SPARC both are still present as of today morning.
Introduced by Jakub's patch.
Related to PR opt/12419.
Also failing on ia64 (3.0 - 3.5, not tree-ssa).
Subject: Bug 13424 CVSROOT: /cvs/gcc Module name: gcc Changes by: jakub@gcc.gnu.org 2004-01-29 08:05:49 Modified files: gcc : ChangeLog expr.c emit-rtl.c gcc/config/i386: i386-protos.h i386.c i386.md Log message: PR optimization/13424 * expr.c (store_constructor): Revert 2003-12-03 change. * emit-rtl.c (change_address): Check also if MEM_ATTRS is set as expected before returning early. Avoid sharing RTL if they need to be changed. * config/i386/i386.c (ix86_expand_movstr): Rework rep_mov and strmov handling so that memory attributes are preserved. Don't call ix86_set_move_mem_attrs. (ix86_set_move_mem_attrs_1, ix86_set_move_mem_attrs): Removed. (ix86_expand_clrstr): Rename src argument to dst. Rework rep_stos and strset handling so that memory attributes are preserved. (ix86_expand_strlen): Pass src argument to ix86_expand_strlensi_unroll_1. Rework strlenqi_1 handling so that memory attributes are preserved. (ix86_expand_strlensi_unroll_1): Add src argument. Use change_address instead of gen_rtx_MEM. * config/i386/i386.md (strmov, strmov_singleop, rep_mov): New expanders. (strmovdi_rex64, strmovsi, strmovsi_rex64, strmovhi, strmovhi_rex64, strmovqi, strmovqi_rex64): Remove. (rep_mov*, strmov*): Prefix insn names with *. (strset, strset_singleop, rep_stos): New expanders. (strsetdi_rex64, strsetsi, strsetsi_rex64, strsethi, strsethi_rex64, strsetqi, strsetqi_rex64): Remove. (rep_stos*, strset*): Prefix insn names with *. (rep_stosqi_rex64): Likewise. Fix mode of dirflag reg from DImode to SImode. (cmpstrsi): Rework cmpstrqi_1 handling so that memory attributes are preserved. (cmpstrqi_nz_1, cmpstrqi_nz_rex_1, cmpstrqi_1, cmpstrqi_rex_1): Prefix insn names with *. (cmpstrqi_nz_1, cmpstrqi_1): New expanders. (strlenqi_1, strlenqi_rex_1): Prefix insn names with *. (strlenqi_1): New expander. * config/i386/i386.h (ix86_set_move_mem_attrs): Remove prototype. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.2548&r2=2.2549 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/expr.c.diff?cvsroot=gcc&r1=1.618&r2=1.619 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/emit-rtl.c.diff?cvsroot=gcc&r1=1.369&r2=1.370 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/i386-protos.h.diff?cvsroot=gcc&r1=1.104&r2=1.105 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/i386.c.diff?cvsroot=gcc&r1=1.639&r2=1.640 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/i386.md.diff?cvsroot=gcc&r1=1.505&r2=1.506
Any plan to backport the patch to the 3.4 branch, Jakub?
Subject: Bug 13424 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-3_4-branch Changes by: jakub@gcc.gnu.org 2004-02-10 18:07:58 Modified files: gcc : ChangeLog expr.c emit-rtl.c gcc/config/i386: i386.c i386.md i386-protos.h Log message: PR optimization/13424 Backport from mainline: 2004-01-29 Jakub Jelinek <jakub@redhat.com> * emit-rtl.c (change_address): Use XEXP (memref, 0) instead of addr when creating MEM copy. 2004-01-28 Jakub Jelinek <jakub@redhat.com> * expr.c (store_constructor): Revert 2003-12-03 change. * emit-rtl.c (change_address): Check also if MEM_ATTRS is set as expected before returning early. Avoid sharing RTL if they need to be changed. * config/i386/i386.c (ix86_expand_movstr): Rework rep_mov and strmov handling so that memory attributes are preserved. Don't call ix86_set_move_mem_attrs. (ix86_set_move_mem_attrs_1, ix86_set_move_mem_attrs): Removed. (ix86_expand_clrstr): Rename src argument to dst. Rework rep_stos and strset handling so that memory attributes are preserved. (ix86_expand_strlen): Pass src argument to ix86_expand_strlensi_unroll_1. Rework strlenqi_1 handling so that memory attributes are preserved. (ix86_expand_strlensi_unroll_1): Add src argument. Use change_address instead of gen_rtx_MEM. * config/i386/i386.md (strmov, strmov_singleop, rep_mov): New expanders. (strmovdi_rex64, strmovsi, strmovsi_rex64, strmovhi, strmovhi_rex64, strmovqi, strmovqi_rex64): Remove. (rep_mov*, strmov*): Prefix insn names with *. (strset, strset_singleop, rep_stos): New expanders. (strsetdi_rex64, strsetsi, strsetsi_rex64, strsethi, strsethi_rex64, strsetqi, strsetqi_rex64): Remove. (rep_stos*, strset*): Prefix insn names with *. (rep_stosqi_rex64): Likewise. Fix mode of dirflag reg from DImode to SImode. (cmpstrsi): Rework cmpstrqi_1 handling so that memory attributes are preserved. (cmpstrqi_nz_1, cmpstrqi_nz_rex_1, cmpstrqi_1, cmpstrqi_rex_1): Prefix insn names with *. (cmpstrqi_nz_1, cmpstrqi_1): New expanders. (strlenqi_1, strlenqi_rex_1): Prefix insn names with *. (strlenqi_1): New expander. * config/i386/i386.h (ix86_set_move_mem_attrs): Remove prototype. 2004-01-24 Jan Hubicka <jh@suse.cz> * emit-rtl.c (change_address, adjust_address_1, offset_address, widen_memory_access): Return early when there is nothing to change. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=2.2326.2.174&r2=2.2326.2.175 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/expr.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.615.4.2&r2=1.615.4.3 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/emit-rtl.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.365.4.1&r2=1.365.4.2 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/i386.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.635.2.3&r2=1.635.2.4 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/i386.md.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.502&r2=1.502.2.1 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/i386-protos.h.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.104.4.1&r2=1.104.4.2
Fixed for 3.4.
Still fails on hppa.
Then let's reopen, but unassign from Jakub because he has done his part. The testcase doesn't fail anymore on SPARC, but here's another one that fails: /* { dg-options "-O2 -mcpu=ultrasparc" } */ struct S { const int i1; const long l1; const int i2; const long l2; int i3; int i4; }; extern void abort(void); void bar (struct S *s) { if (s->i3 != 2) abort(); } int main(void) { struct S s = { 0, 0, 0, 0 }; s.i3 = 2; bar(&s); return 0; } The problem is that the /u flag is put on a SET to a MEM during clearing, but not during the subsequent assignment, and the scheduler swaps the two SETs. It appears that the clearing optimization is not safe in presence of readonly fields; now, if we disable it, we have a pessimization for structures with const members over structures without const members. Really tricky...
Subject: Bug 13424 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-3_2-rhl8-branch Changes by: jakub@gcc.gnu.org 2004-02-28 11:26:21 Modified files: gcc : ChangeLog emit-rtl.c expr.c gcc/config/i386: i386-protos.h i386.md i386.c Added files: gcc/testsuite/gcc.dg: 20040216-1.c Log message: 2004-02-16 Jakub Jelinek <jakub@redhat.com> PR optimization/13424 * expr.c (store_constructor): Revert 2003-12-03 change. * emit-rtl.c (change_address): Check also if MEM_ATTRS is set as expected before returning early. Avoid sharing RTL if they need to be changed. * config/i386/i386.c (ix86_expand_movstr): Rework rep_mov and strmov handling so that memory attributes are preserved. Don't call ix86_set_move_mem_attrs. (ix86_set_move_mem_attrs_1, ix86_set_move_mem_attrs): Removed. (ix86_expand_clrstr): Rename src argument to dst. Rework rep_stos and strset handling so that memory attributes are preserved. (ix86_expand_strlen): Pass src argument to ix86_expand_strlensi_unroll_1. Rework strlenqi_1 handling so that memory attributes are preserved. (ix86_expand_strlensi_unroll_1): Add src argument. Use change_address instead of gen_rtx_MEM. * config/i386/i386.md (strmov, strmov_singleop, rep_mov): New expanders. (strmovdi_rex64, strmovsi, strmovsi_rex64, strmovhi, strmovhi_rex64, strmovqi, strmovqi_rex64): Remove. (rep_mov*, strmov*): Prefix insn names with *. (strset, strset_singleop, rep_stos): New expanders. (strsetdi_rex64, strsetsi, strsetsi_rex64, strsethi, strsethi_rex64, strsetqi, strsetqi_rex64): Remove. (rep_stos*, strset*): Prefix insn names with *. (rep_stosqi_rex64): Likewise. Fix mode of dirflag reg from DImode to SImode. (cmpstrsi): Rework cmpstrqi_1 handling so that memory attributes are preserved. (cmpstrqi_nz_1, cmpstrqi_nz_rex_1, cmpstrqi_1, cmpstrqi_rex_1): Prefix insn names with *. (cmpstrqi_nz_1, cmpstrqi_1): New expanders. (strlenqi_1, strlenqi_rex_1): Prefix insn names with *. (strlenqi_1): New expander. * config/i386/i386.h (ix86_set_move_mem_attrs): Remove prototype. 2003-04-21 Richard Kenner <kenner@vlsi1.ultra.nyu.edu> * expr.c (store_constructor): Set RTX_UNCHANGING_P if readonly_field_p before clearing. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_2-rhl8-branch&r1=1.13152.2.657.2.27.2.187&r2=1.13152.2.657.2.27.2.188 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/emit-rtl.c.diff?cvsroot=gcc&only_with_tag=gcc-3_2-rhl8-branch&r1=1.249.2.10.4.3&r2=1.249.2.10.4.4 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/expr.c.diff?cvsroot=gcc&only_with_tag=gcc-3_2-rhl8-branch&r1=1.423.2.19.4.1.2.20&r2=1.423.2.19.4.1.2.21 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/i386-protos.h.diff?cvsroot=gcc&only_with_tag=gcc-3_2-rhl8-branch&r1=1.68.2.3.4.6&r2=1.68.2.3.4.7 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/i386.md.diff?cvsroot=gcc&only_with_tag=gcc-3_2-rhl8-branch&r1=1.339.2.13.2.1.2.20&r2=1.339.2.13.2.1.2.21 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/i386.c.diff?cvsroot=gcc&only_with_tag=gcc-3_2-rhl8-branch&r1=1.368.2.19.2.2.2.37&r2=1.368.2.19.2.2.2.38 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/20040216-1.c.diff?cvsroot=gcc&only_with_tag=gcc-3_2-rhl8-branch&r1=NONE&r2=1.1.2.1
I am distinctly tired of this RTX_UNCHANGING_P mess. It's demonstrably so badly designed that no-one has a good handle on how to fix it properly. I propose that it simply be disabled on 3.4/3.5, and hope to fix the problem differently with tree-ssa.
Subject: Re: [3.4/3.5 Regression] gcc.dg/20031202-1.c > ------- Additional Comments From rth at gcc dot gnu dot org 2004-03-02 > 23:43 ------- > I am distinctly tired of this RTX_UNCHANGING_P mess. It's demonstrably so > badly designed that no-one has a good handle on how to fix it properly. > > I propose that it simply be disabled on 3.4/3.5, and hope to fix the problem > differently with tree-ssa. The patch that Jakub proposed for 3.3 (?) fixed the test failure on the PA but as noted in the discussion at the time it wasn't a proper fix. Dave -- J. David Anglin dave.anglin@nrc-cnrc.gc.ca National Research Council of Canada (613) 990-0752 (FAX: 952-6602) Index: expr.c =================================================================== RCS file: /cvs/gcc/gcc/gcc/expr.c,v retrieving revision 1.610 diff -u -3 -p -r1.610 expr.c --- expr.c 1 Jan 2004 14:13:41 -0000 1.610 +++ expr.c 2 Jan 2004 03:32:49 -0000 @@ -4565,15 +4565,7 @@ store_constructor (tree exp, rtx target, || ((HOST_WIDE_INT) GET_MODE_SIZE (GET_MODE (target)) == size))) { - rtx xtarget = target; - - if (readonly_fields_p (type)) - { - xtarget = copy_rtx (xtarget); - RTX_UNCHANGING_P (xtarget) = 1; - } - - clear_storage (xtarget, GEN_INT (size)); + clear_storage (target, GEN_INT (size)); cleared = 1; } @@ -4994,9 +4986,9 @@ store_constructor (tree exp, rtx target, target = copy_rtx (target); MEM_KEEP_ALIAS_SET_P (target) = 1; } - else - store_constructor_field (target, bitsize, bitpos, mode, value, - type, cleared, get_alias_set (elttype)); + + store_constructor_field (target, bitsize, bitpos, mode, value, + type, cleared, get_alias_set (elttype)); } } if (vector)
Testing patch from the ACT tree.
Dave, could you tell me if the attached patch does fix the problem on HP-PA? TIA.
Created attachment 5934 [details] Patch from the ACT tree.
Subject: Re: [3.4/3.5 Regression] gcc.dg/20031202-1.c > Dave, could you tell me if the attached patch does fix the problem on HP-PA? > TIA. It doesn't fix the failure of gcc.dg/20031202-1.c on hppa-linux. Dave
Yes, actually it makes it fail everywhere! The problem is that the C front-end is relying on the clearing optimization to implement the semantics mandated by C99 $6.7.8 [19].
Dave, here's another patch based on Mark's suggestion.
Created attachment 5938 [details] Second tentative patch.
Subject: Re: [3.4/3.5 Regression] gcc.dg/20031202-1.c > --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=5938&action=view) > Second tentative patch. This works! Dave
Subject: Bug 13424 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-3_3-rhl-branch Changes by: jakub@gcc.gnu.org 2004-03-27 16:10:09 Modified files: gcc : ChangeLog gcc/config/i386: i386-protos.h i386.md i386.c gcc : emit-rtl.c expr.c Added files: gcc/testsuite/gcc.dg: 20040216-1.c Log message: 2004-02-16 Jakub Jelinek <jakub@redhat.com> PR optimization/13424 * expr.c (store_constructor): Revert 2003-12-03 change. * emit-rtl.c (change_address): Check also if MEM_ATTRS is set as expected before returning early. Avoid sharing RTL if they need to be changed. * config/i386/i386.c (ix86_expand_movstr): Rework rep_mov and strmov handling so that memory attributes are preserved. Don't call ix86_set_move_mem_attrs. (ix86_set_move_mem_attrs_1, ix86_set_move_mem_attrs): Removed. (ix86_expand_clrstr): Rename src argument to dst. Rework rep_stos and strset handling so that memory attributes are preserved. (ix86_expand_strlen): Pass src argument to ix86_expand_strlensi_unroll_1. Rework strlenqi_1 handling so that memory attributes are preserved. (ix86_expand_strlensi_unroll_1): Add src argument. Use change_address instead of gen_rtx_MEM. * config/i386/i386.md (strmov, strmov_singleop, rep_mov): New expanders. (strmovdi_rex64, strmovsi, strmovsi_rex64, strmovhi, strmovhi_rex64, strmovqi, strmovqi_rex64): Remove. (rep_mov*, strmov*): Prefix insn names with *. (strset, strset_singleop, rep_stos): New expanders. (strsetdi_rex64, strsetsi, strsetsi_rex64, strsethi, strsethi_rex64, strsetqi, strsetqi_rex64): Remove. (rep_stos*, strset*): Prefix insn names with *. (rep_stosqi_rex64): Likewise. Fix mode of dirflag reg from DImode to SImode. (cmpstrsi): Rework cmpstrqi_1 handling so that memory attributes are preserved. (cmpstrqi_nz_1, cmpstrqi_nz_rex_1, cmpstrqi_1, cmpstrqi_rex_1): Prefix insn names with *. (cmpstrqi_nz_1, cmpstrqi_1): New expanders. (strlenqi_1, strlenqi_rex_1): Prefix insn names with *. (strlenqi_1): New expander. * config/i386/i386.h (ix86_set_move_mem_attrs): Remove prototype. 2003-04-21 Richard Kenner <kenner@vlsi1.ultra.nyu.edu> * expr.c (store_constructor): Set RTX_UNCHANGING_P if readonly_field_p before clearing. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_3-rhl-branch&r1=1.16114.2.523.2.94&r2=1.16114.2.523.2.95 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/i386-protos.h.diff?cvsroot=gcc&only_with_tag=gcc-3_3-rhl-branch&r1=1.86.2.1.4.1&r2=1.86.2.1.4.2 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/i386.md.diff?cvsroot=gcc&only_with_tag=gcc-3_3-rhl-branch&r1=1.404.2.14.4.11&r2=1.404.2.14.4.12 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/i386/i386.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-rhl-branch&r1=1.495.2.20.4.19&r2=1.495.2.20.4.20 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/emit-rtl.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-rhl-branch&r1=1.303.2.1.2.2&r2=1.303.2.1.2.3 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/expr.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-rhl-branch&r1=1.498.2.17.2.12&r2=1.498.2.17.2.13 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/20040216-1.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-rhl-branch&r1=NONE&r2=1.1.4.1
Subject: Bug 13424 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-3_4-branch Changes by: jakub@gcc.gnu.org 2004-04-02 23:05:44 Modified files: gcc : ChangeLog explow.c expr.c gcc/testsuite : ChangeLog Added files: gcc/testsuite/gcc.c-torture/execute: 20040401-1.c gcc/testsuite/gcc.dg: 20040302-1.c Log message: PR optimization/13424, optimization/12419 * explow.c (maybe_set_unchanging): Revert 2003-04-07 patch. Set RTX_UNCHANGING_P even for read-only DECL_EXTERNAL decls. * expr.c (store_constructor): When clearing aggregate because of an incomplete or mostly zero constructor, do the clearing without /u flag and then emit a blockage. * gcc.dg/20040302-1.c: New test. * gcc.c-torture/execute/20040401-1.c: New test. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=2.2326.2.377&r2=2.2326.2.378 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/explow.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.118&r2=1.118.4.1 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/expr.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.615.4.10&r2=1.615.4.11 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.3389.2.161&r2=1.3389.2.162 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.c-torture/execute/20040401-1.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=NONE&r2=1.1.2.1 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/20040302-1.c.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=NONE&r2=1.1.2.1
So is this fixed now, can the bug report be closed?
It ought to be fixed on gcc-3_4-branch only ATM (no trunk yet) and not on HPPA which has buggy backend, see: http://gcc.gnu.org/ml/gcc-patches/2004-04/msg00186.html
Subject: Re: [3.4/3.5 Regression] gcc.dg/20031202-1.c > It ought to be fixed on gcc-3_4-branch only ATM (no trunk yet) and > not on HPPA which has buggy backend, see: > http://gcc.gnu.org/ml/gcc-patches/2004-04/msg00186.html With the enclosed patch, the regression introduced by Jakub's fix for this PR is fixed (c/14828). I confirm that the problem reported in this PR is also fixed. I also don't see the failure on the 3.5 branch anymore when the enclosed patch is installed. The patch also fixes the Ada bootstrap problem reported in 14462. If this patch is approved, I would like to close 13424, 14462 and 14828. If there are remaining issues with RTX_UNCHANGING_P on 3.5, I think they should be placed in a new PR. The enclosed patch has been tested on hppa-unknown-linux-gnu (3.4.0, 3.5.0), hppa2.0w-hp-hpux11.00 (3.4), hppa64-hp-hpux11.00 (3.4) and hppa64-hp-hpux11.11 (3.5 c only) with no observed regressions. Mark, is this ok for 3.4? I very much appreciate all the work that Jakub did in resolving this problem. I just extended and tested what he had done. Hopefully, this closes the long saga in reworking movstrsi. Dave -- J. David Anglin dave.anglin@nrc-cnrc.gc.ca National Research Council of Canada (613) 990-0752 (FAX: 952-6602) 2004-04-05 Jakub Jelinek <jakuB@redhat.com> John David Anglin <dave.anglin@nrc-cnrc.gc.ca> PR optimization/13424 (hppa), bootstrap/14462, c/14828 * pa.md: Use replace_equiv_address to retain the attributes of the memory operands used in the split and peephole2 patterns for optimizing the pre-reload movstrsi, movstrdi, clrstrsi and clrstrdi patterns. Index: pa.md =================================================================== RCS file: /cvs/gcc/gcc/gcc/config/pa/pa.md,v retrieving revision 1.138.4.4 diff -u -3 -p -r1.138.4.4 pa.md --- pa.md 6 Mar 2004 19:55:09 -0000 1.138.4.4 +++ pa.md 3 Apr 2004 17:03:15 -0000 @@ -3206,8 +3204,8 @@ [(set_attr "type" "multi,multi")]) (define_split - [(parallel [(set (mem:BLK (match_operand:SI 0 "register_operand" "")) - (mem:BLK (match_operand:SI 1 "register_operand" ""))) + [(parallel [(set (match_operand:BLK 0 "memory_operand" "") + (match_operand:BLK 1 "memory_operand" "")) (clobber (match_operand:SI 2 "register_operand" "")) (clobber (match_operand:SI 3 "register_operand" "")) (clobber (match_operand:SI 6 "register_operand" "")) @@ -3215,10 +3213,14 @@ (clobber (match_operand:SI 8 "register_operand" "")) (use (match_operand:SI 4 "arith_operand" "")) (use (match_operand:SI 5 "const_int_operand" ""))])] - "!TARGET_64BIT && reload_completed && !flag_peephole2" - [(set (match_dup 7) (match_dup 0)) - (set (match_dup 8) (match_dup 1)) - (parallel [(set (mem:BLK (match_dup 7)) (mem:BLK (match_dup 8))) + "!TARGET_64BIT && reload_completed && !flag_peephole2 + && GET_CODE (operands[0]) == MEM + && register_operand (XEXP (operands[0], 0), SImode) + && GET_CODE (operands[1]) == MEM + && register_operand (XEXP (operands[1], 0), SImode)" + [(set (match_dup 7) (match_dup 9)) + (set (match_dup 8) (match_dup 10)) + (parallel [(set (match_dup 0) (match_dup 1)) (clobber (match_dup 2)) (clobber (match_dup 3)) (clobber (match_dup 6)) @@ -3227,11 +3229,17 @@ (use (match_dup 4)) (use (match_dup 5)) (const_int 0)])] - "") + " +{ + operands[9] = XEXP (operands[0], 0); + operands[10] = XEXP (operands[1], 0); + operands[0] = replace_equiv_address (operands[0], operands[7]); + operands[1] = replace_equiv_address (operands[1], operands[8]); +}") (define_peephole2 - [(parallel [(set (mem:BLK (match_operand:SI 0 "register_operand" "")) - (mem:BLK (match_operand:SI 1 "register_operand" ""))) + [(parallel [(set (match_operand:BLK 0 "memory_operand" "") + (match_operand:BLK 1 "memory_operand" "")) (clobber (match_operand:SI 2 "register_operand" "")) (clobber (match_operand:SI 3 "register_operand" "")) (clobber (match_operand:SI 6 "register_operand" "")) @@ -3239,8 +3247,12 @@ (clobber (match_operand:SI 8 "register_operand" "")) (use (match_operand:SI 4 "arith_operand" "")) (use (match_operand:SI 5 "const_int_operand" ""))])] - "!TARGET_64BIT" - [(parallel [(set (mem:BLK (match_dup 7)) (mem:BLK (match_dup 8))) + "!TARGET_64BIT + && GET_CODE (operands[0]) == MEM + && register_operand (XEXP (operands[0], 0), SImode) + && GET_CODE (operands[1]) == MEM + && register_operand (XEXP (operands[1], 0), SImode)" + [(parallel [(set (match_dup 0) (match_dup 1)) (clobber (match_dup 2)) (clobber (match_dup 3)) (clobber (match_dup 6)) @@ -3251,15 +3263,23 @@ (const_int 0)])] " { - if (dead_or_set_p (curr_insn, operands[0])) - operands[7] = operands[0]; + rtx addr = XEXP (operands[0], 0); + if (dead_or_set_p (curr_insn, addr)) + operands[7] = addr; else - emit_insn (gen_rtx_SET (VOIDmode, operands[7], operands[0])); + { + emit_insn (gen_rtx_SET (VOIDmode, operands[7], addr)); + operands[0] = replace_equiv_address (operands[0], operands[7]); + } - if (dead_or_set_p (curr_insn, operands[1])) - operands[8] = operands[1]; + addr = XEXP (operands[1], 0); + if (dead_or_set_p (curr_insn, addr)) + operands[8] = addr; else - emit_insn (gen_rtx_SET (VOIDmode, operands[8], operands[1])); + { + emit_insn (gen_rtx_SET (VOIDmode, operands[8], addr)); + operands[1] = replace_equiv_address (operands[1], operands[8]); + } }") (define_insn "movstrsi_postreload" @@ -3372,8 +3392,8 @@ [(set_attr "type" "multi,multi")]) (define_split - [(parallel [(set (mem:BLK (match_operand:DI 0 "register_operand" "")) - (mem:BLK (match_operand:DI 1 "register_operand" ""))) + [(parallel [(set (match_operand:BLK 0 "memory_operand" "") + (match_operand:BLK 1 "memory_operand" "")) (clobber (match_operand:DI 2 "register_operand" "")) (clobber (match_operand:DI 3 "register_operand" "")) (clobber (match_operand:DI 6 "register_operand" "")) @@ -3381,10 +3401,14 @@ (clobber (match_operand:DI 8 "register_operand" "")) (use (match_operand:DI 4 "arith_operand" "")) (use (match_operand:DI 5 "const_int_operand" ""))])] - "TARGET_64BIT && reload_completed && !flag_peephole2" - [(set (match_dup 7) (match_dup 0)) - (set (match_dup 8) (match_dup 1)) - (parallel [(set (mem:BLK (match_dup 7)) (mem:BLK (match_dup 8))) + "TARGET_64BIT && reload_completed && !flag_peephole2 + && GET_CODE (operands[0]) == MEM + && register_operand (XEXP (operands[0], 0), DImode) + && GET_CODE (operands[1]) == MEM + && register_operand (XEXP (operands[1], 0), DImode)" + [(set (match_dup 7) (match_dup 9)) + (set (match_dup 8) (match_dup 10)) + (parallel [(set (match_dup 0) (match_dup 1)) (clobber (match_dup 2)) (clobber (match_dup 3)) (clobber (match_dup 6)) @@ -3393,11 +3417,17 @@ (use (match_dup 4)) (use (match_dup 5)) (const_int 0)])] - "") + " +{ + operands[9] = XEXP (operands[0], 0); + operands[10] = XEXP (operands[1], 0); + operands[0] = replace_equiv_address (operands[0], operands[7]); + operands[1] = replace_equiv_address (operands[1], operands[8]); +}") (define_peephole2 - [(parallel [(set (mem:BLK (match_operand:DI 0 "register_operand" "")) - (mem:BLK (match_operand:DI 1 "register_operand" ""))) + [(parallel [(set (match_operand:BLK 0 "memory_operand" "") + (match_operand:BLK 1 "memory_operand" "")) (clobber (match_operand:DI 2 "register_operand" "")) (clobber (match_operand:DI 3 "register_operand" "")) (clobber (match_operand:DI 6 "register_operand" "")) @@ -3405,8 +3435,12 @@ (clobber (match_operand:DI 8 "register_operand" "")) (use (match_operand:DI 4 "arith_operand" "")) (use (match_operand:DI 5 "const_int_operand" ""))])] - "TARGET_64BIT" - [(parallel [(set (mem:BLK (match_dup 7)) (mem:BLK (match_dup 8))) + "TARGET_64BIT + && GET_CODE (operands[0]) == MEM + && register_operand (XEXP (operands[0], 0), DImode) + && GET_CODE (operands[1]) == MEM + && register_operand (XEXP (operands[1], 0), DImode)" + [(parallel [(set (match_dup 0) (match_dup 1)) (clobber (match_dup 2)) (clobber (match_dup 3)) (clobber (match_dup 6)) @@ -3417,15 +3451,23 @@ (const_int 0)])] " { - if (dead_or_set_p (curr_insn, operands[0])) - operands[7] = operands[0]; + rtx addr = XEXP (operands[0], 0); + if (dead_or_set_p (curr_insn, addr)) + operands[7] = addr; else - emit_insn (gen_rtx_SET (VOIDmode, operands[7], operands[0])); + { + emit_insn (gen_rtx_SET (VOIDmode, operands[7], addr)); + operands[0] = replace_equiv_address (operands[0], operands[7]); + } - if (dead_or_set_p (curr_insn, operands[1])) - operands[8] = operands[1]; + addr = XEXP (operands[1], 0); + if (dead_or_set_p (curr_insn, addr)) + operands[8] = addr; else - emit_insn (gen_rtx_SET (VOIDmode, operands[8], operands[1])); + { + emit_insn (gen_rtx_SET (VOIDmode, operands[8], addr)); + operands[1] = replace_equiv_address (operands[1], operands[8]); + } }") (define_insn "movstrdi_postreload" @@ -3491,31 +3533,39 @@ [(set_attr "type" "multi,multi")]) (define_split - [(parallel [(set (mem:BLK (match_operand:SI 0 "register_operand" "")) + [(parallel [(set (match_operand:BLK 0 "memory_operand" "") (const_int 0)) (clobber (match_operand:SI 1 "register_operand" "")) (clobber (match_operand:SI 4 "register_operand" "")) (use (match_operand:SI 2 "arith_operand" "")) (use (match_operand:SI 3 "const_int_operand" ""))])] - "!TARGET_64BIT && reload_completed && !flag_peephole2" - [(set (match_dup 4) (match_dup 0)) - (parallel [(set (mem:BLK (match_dup 4)) (const_int 0)) + "!TARGET_64BIT && reload_completed && !flag_peephole2 + && GET_CODE (operands[0]) == MEM + && register_operand (XEXP (operands[0], 0), SImode)" + [(set (match_dup 4) (match_dup 5)) + (parallel [(set (match_dup 0) (const_int 0)) (clobber (match_dup 1)) (clobber (match_dup 4)) (use (match_dup 2)) (use (match_dup 3)) (const_int 0)])] - "") + " +{ + operands[5] = XEXP (operands[0], 0); + operands[0] = replace_equiv_address (operands[0], operands[4]); +}") (define_peephole2 - [(parallel [(set (mem:BLK (match_operand:SI 0 "register_operand" "")) + [(parallel [(set (match_operand:BLK 0 "memory_operand" "") (const_int 0)) (clobber (match_operand:SI 1 "register_operand" "")) (clobber (match_operand:SI 4 "register_operand" "")) (use (match_operand:SI 2 "arith_operand" "")) (use (match_operand:SI 3 "const_int_operand" ""))])] - "!TARGET_64BIT" - [(parallel [(set (mem:BLK (match_dup 4)) (const_int 0)) + "!TARGET_64BIT + && GET_CODE (operands[0]) == MEM + && register_operand (XEXP (operands[0], 0), SImode)" + [(parallel [(set (match_dup 0) (const_int 0)) (clobber (match_dup 1)) (clobber (match_dup 4)) (use (match_dup 2)) @@ -3523,10 +3573,14 @@ (const_int 0)])] " { - if (dead_or_set_p (curr_insn, operands[0])) - operands[4] = operands[0]; + rtx addr = XEXP (operands[0], 0); + if (dead_or_set_p (curr_insn, addr)) + operands[4] = addr; else - emit_insn (gen_rtx_SET (VOIDmode, operands[4], operands[0])); + { + emit_insn (gen_rtx_SET (VOIDmode, operands[4], addr)); + operands[0] = replace_equiv_address (operands[0], operands[4]); + } }") (define_insn "clrstrsi_postreload" @@ -3589,31 +3643,39 @@ [(set_attr "type" "multi,multi")]) (define_split - [(parallel [(set (mem:BLK (match_operand:DI 0 "register_operand" "")) + [(parallel [(set (match_operand:BLK 0 "memory_operand" "") (const_int 0)) (clobber (match_operand:DI 1 "register_operand" "")) (clobber (match_operand:DI 4 "register_operand" "")) (use (match_operand:DI 2 "arith_operand" "")) (use (match_operand:DI 3 "const_int_operand" ""))])] - "TARGET_64BIT && reload_completed && !flag_peephole2" - [(set (match_dup 4) (match_dup 0)) - (parallel [(set (mem:BLK (match_dup 4)) (const_int 0)) + "TARGET_64BIT && reload_completed && !flag_peephole2 + && GET_CODE (operands[0]) == MEM + && register_operand (XEXP (operands[0], 0), DImode)" + [(set (match_dup 4) (match_dup 5)) + (parallel [(set (match_dup 0) (const_int 0)) (clobber (match_dup 1)) (clobber (match_dup 4)) (use (match_dup 2)) (use (match_dup 3)) (const_int 0)])] - "") + " +{ + operands[5] = XEXP (operands[0], 0); + operands[0] = replace_equiv_address (operands[0], operands[4]); +}") (define_peephole2 - [(parallel [(set (mem:BLK (match_operand:DI 0 "register_operand" "")) + [(parallel [(set (match_operand:BLK 0 "memory_operand" "") (const_int 0)) (clobber (match_operand:DI 1 "register_operand" "")) (clobber (match_operand:DI 4 "register_operand" "")) (use (match_operand:DI 2 "arith_operand" "")) (use (match_operand:DI 3 "const_int_operand" ""))])] - "TARGET_64BIT" - [(parallel [(set (mem:BLK (match_dup 4)) (const_int 0)) + "TARGET_64BIT + && GET_CODE (operands[0]) == MEM + && register_operand (XEXP (operands[0], 0), DImode)" + [(parallel [(set (match_dup 0) (const_int 0)) (clobber (match_dup 1)) (clobber (match_dup 4)) (use (match_dup 2)) @@ -3621,10 +3683,14 @@ (const_int 0)])] " { - if (dead_or_set_p (curr_insn, operands[0])) - operands[4] = operands[0]; + rtx addr = XEXP (operands[0], 0); + if (dead_or_set_p (curr_insn, addr)) + operands[4] = addr; else - emit_insn (gen_rtx_SET (VOIDmode, operands[4], operands[0])); + { + emit_insn (gen_rtx_SET (VOIDmode, operands[4], addr)); + operands[0] = replace_equiv_address (operands[0], operands[4]); + } }") (define_insn "clrstrdi_postreload"
Subject: Bug 13424 CVSROOT: /cvs/gcc Module name: gcc Changes by: danglin@gcc.gnu.org 2004-04-05 16:41:20 Modified files: gcc : ChangeLog gcc/config/pa : pa.md Log message: PR optimization/13424 (hppa), bootstrap/14462, c/14828 * pa.md: Use replace_equiv_address to retain the attributes of the memory operands used in the split and peephole2 patterns for optimizing the pre-reload movstrsi, movstrdi, clrstrsi and clrstrdi patterns. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.3345&r2=2.3346 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/pa/pa.md.diff?cvsroot=gcc&r1=1.143&r2=1.144
The SPARC bug is now fixed on the 3.4 branch. YYYYYYYES :-) But fingers crossed though, because * explow.c (maybe_set_unchanging): Revert 2003-04-07 patch. may bring back problems for random back-ends.
Subject: Re: [3.4/3.5 Regression] gcc.dg/20031202-1.c John David Anglin wrote: >>It ought to be fixed on gcc-3_4-branch only ATM (no trunk yet) and >>not on HPPA which has buggy backend, see: >>http://gcc.gnu.org/ml/gcc-patches/2004-04/msg00186.html >> >> > >With the enclosed patch, the regression introduced by Jakub's fix >for this PR is fixed (c/14828). I confirm that the problem reported >in this PR is also fixed. I also don't see the failure on the 3.5 branch >anymore when the enclosed patch is installed. The patch also fixes the >Ada bootstrap problem reported in 14462. If this patch is approved, >I would like to close 13424, 14462 and 14828. > > Yes, this patch is OK, and it is OK to close the PRs after you put it in. Please put that in 3.4.0 and let me know when it is in. I am holding up the prerelease for this patch. >If there are remaining issues with RTX_UNCHANGING_P on 3.5, I think >they should be placed in a new PR. > > Agreed. Thanks for your help!
Subject: Bug 13424 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-3_4-branch Changes by: mmitchel@gcc.gnu.org 2004-04-06 09:03:57 Modified files: gcc : ChangeLog gcc/config/pa : pa.md Log message: PR optimization/13424 (hppa), bootstrap/14462, c/14828 * pa.md: Use replace_equiv_address to retain the attributes of the memory operands used in the split and peephole2 patterns for optimizing the pre-reload movstrsi, movstrdi, clrstrsi and clrstrdi patterns. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=2.2326.2.381&r2=2.2326.2.382 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/config/pa/pa.md.diff?cvsroot=gcc&only_with_tag=gcc-3_4-branch&r1=1.138.4.4&r2=1.138.4.5
Fixed.
John, I am unaware of all the detours this has taken, but - the synopsis of the bug says this is a 3.4/3.5 regression - the patch seems to have only been applied to 3.4 Can you say, in one word, whether closing the PR is intentional, and/or whether it should be kept open for 3.5? Thanks for answering a maybe naive question, W.
Subject: Re: [3.4/3.5 Regression] gcc.dg/20031202-1.c > ------- Additional Comments From bangerth at dealii dot org 2004-04-06 > 16:16 ------- > John, I am unaware of all the detours this has taken, but > - the synopsis of the bug says this is a 3.4/3.5 regression > - the patch seems to have only been applied to 3.4 I applied the patch to 3.5 yesterday. Mark applied the patch to 3.4 last night. Dave
Dave, thanks for the feedback. Then it just didn't show up in the audit trail of this PR. I assume you know how to automatically get it there by putting something like "PR 13424" into the ChangeLog of a CVS commit. Thanks Wolfgang
Subject: Re: [3.4/3.5 Regression] gcc.dg/20031202-1.c > Dave, thanks for the feedback. Then it just didn't show up in the > audit trail of this PR. I assume you know how to automatically get > it there by putting something like "PR 13424" into the ChangeLog > of a CVS commit. See comments #35 and #38. However, I see the commits didn't show up in 14462 and 14828. Do multiple PR listings need to be on separate lines in the ChangeLog of a CVS commit? Dave
Right, I'm sorry I missed comment #35 :-( As to the exact format, I don't know, but here is one that apparently made it into both PR audit trails: 2004-03-06 Eric Botcazou <ebotcazou@libertysurf.fr> PR c/14465 PR c/14114 * c-decl (pushdecl): Revert previous change. So that seems to be a form that works. W.
Subject: Bug 13424 CVSROOT: /cvs/gcc Module name: gcc Branch: gcc-3_3-rhl-branch Changes by: jakub@gcc.gnu.org 2004-04-13 20:31:19 Modified files: gcc : ChangeLog explow.c expr.c gcc/testsuite : ChangeLog Added files: gcc/testsuite/gcc.c-torture/execute: 20030408-1.c 20040401-1.c gcc/testsuite/gcc.dg: 20040302-1.c Log message: 2004-04-02 Jakub Jelinek <jakuB@redhat.com> Eric Botcazou <ebotcazou@libertysurf.fr> PR optimization/13424, optimization/12419 * explow.c (maybe_set_unchanging): Revert 2003-04-07 patch. * expr.c (store_constructor): When clearing aggregate because of an incomplete or mostly zero constructor, do the clearing without /u flag and then emit a blockage. * gcc.dg/20040302-1.c: New test. * gcc.c-torture/execute/20040401-1.c: New test. 2003-04-08 Glen Nakamura <glen@imodulo.com> * gcc.c-torture/execute/20030408-1.c: New test. Patches: http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_3-rhl-branch&r1=1.16114.2.523.2.102&r2=1.16114.2.523.2.103 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/explow.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-rhl-branch&r1=1.104.4.1.4.2&r2=1.104.4.1.4.3 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/expr.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-rhl-branch&r1=1.498.2.17.2.15&r2=1.498.2.17.2.16 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-3_3-rhl-branch&r1=1.2261.2.170.2.54&r2=1.2261.2.170.2.55 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.c-torture/execute/20030408-1.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-rhl-branch&r1=NONE&r2=1.1.28.1 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.c-torture/execute/20040401-1.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-rhl-branch&r1=NONE&r2=1.1.6.1 http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/gcc.dg/20040302-1.c.diff?cvsroot=gcc&only_with_tag=gcc-3_3-rhl-branch&r1=NONE&r2=1.1.6.1