[Bug rtl-optimization/69447] [5/6 Regression] wrong code with -O2 -fno-schedule-insns and mixed 8/16/32/64bit arithmetics @ armv7a
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69447 --- Comment #15 from Richard Henderson --- Author: rth Date: Wed Jan 27 22:08:02 2016 New Revision: 232905 URL: https://gcc.gnu.org/viewcvs?rev=232905=gcc=rev Log: PR rtl-opt/69447 * lra-remat.c (subreg_regs): New. (dump_candidates_and_remat_bb_data): Dump it. (operand_to_remat): Reject if operand in subreg_regs. (set_bb_regs): Collect subreg_regs. (lra_remat): Init and free subreg_regs. Compute calculate_local_reg_remat_bb_data before create_cands. Added: trunk/gcc/testsuite/gcc.c-torture/execute/pr69447.c Modified: trunk/gcc/ChangeLog trunk/gcc/lra-remat.c trunk/gcc/testsuite/ChangeLog
[Bug rtl-optimization/69447] [5/6 Regression] wrong code with -O2 -fno-schedule-insns and mixed 8/16/32/64bit arithmetics @ armv7a
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69447 Richard Henderson changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #16 from Richard Henderson --- Fixed.
[Bug rtl-optimization/69447] [5/6 Regression] wrong code with -O2 -fno-schedule-insns and mixed 8/16/32/64bit arithmetics @ armv7a
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69447 --- Comment #17 from Zdenek Sojka --- (In reply to Richard Henderson from comment #16) > Fixed. This patch is not going to the 5-branch?
[Bug rtl-optimization/69447] [5/6 Regression] wrong code with -O2 -fno-schedule-insns and mixed 8/16/32/64bit arithmetics @ armv7a
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69447 --- Comment #12 from Richard Henderson --- (In reply to ktkachov from comment #10) > This patch also seems to fix the wrong code in PR 69124 Good to know -- I wasn't able to reproduce that failure myself.
[Bug rtl-optimization/69447] [5/6 Regression] wrong code with -O2 -fno-schedule-insns and mixed 8/16/32/64bit arithmetics @ armv7a
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69447 --- Comment #13 from Richard Henderson --- (In reply to Jakub Jelinek from comment #11) > Without knowing the lra-remat code at all, I just wonder if subreg_regs > needs to be one per the whole function, rather than say per extended basic > block or basic block, with the patch any uses in multi-reg subregs anywhere > in the function will affect remat of all other spots where it is used. I started with subreg_regs being per-block. But since IRA has some global component, I was concerned that there would be some edge case that would be missed, and switched to a global bitmap. Perhaps someone who knows the allocator better can comment.
[Bug rtl-optimization/69447] [5/6 Regression] wrong code with -O2 -fno-schedule-insns and mixed 8/16/32/64bit arithmetics @ armv7a
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69447 --- Comment #14 from Vladimir Makarov --- (In reply to Richard Henderson from comment #13) > (In reply to Jakub Jelinek from comment #11) > > Without knowing the lra-remat code at all, I just wonder if subreg_regs > > needs to be one per the whole function, rather than say per extended basic > > block or basic block, with the patch any uses in multi-reg subregs anywhere > > in the function will affect remat of all other spots where it is used. > > I started with subreg_regs being per-block. But since IRA has > some global component, I was concerned that there would be some > edge case that would be missed, and switched to a global bitmap. > > Perhaps someone who knows the allocator better can comment. Richard, thanks for working on it. You used the right approach. I believe some edge cases are possible, e.g. sched1 could move one subreg insn into another block. So just non-global approach would be vulnerable. The patch looks ok to me. So you can commit it if the test results are ok. Thanks again. I had no time to work on it as I am too busy with other RA bugs.
[Bug rtl-optimization/69447] [5/6 Regression] wrong code with -O2 -fno-schedule-insns and mixed 8/16/32/64bit arithmetics @ armv7a
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69447 --- Comment #10 from ktkachov at gcc dot gnu.org --- (In reply to Richard Henderson from comment #9) > Created attachment 37484 [details] > proposed patch > > Fixes the test case, in that it prevents the remat. > > Starting overnight bootstraps for armv7hf, i686, and x86_64... This patch also seems to fix the wrong code in PR 69124
[Bug rtl-optimization/69447] [5/6 Regression] wrong code with -O2 -fno-schedule-insns and mixed 8/16/32/64bit arithmetics @ armv7a
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69447 --- Comment #11 from Jakub Jelinek --- Without knowing the lra-remat code at all, I just wonder if subreg_regs needs to be one per the whole function, rather than say per extended basic block or basic block, with the patch any uses in multi-reg subregs anywhere in the function will affect remat of all other spots where it is used.
[Bug rtl-optimization/69447] [5/6 Regression] wrong code with -O2 -fno-schedule-insns and mixed 8/16/32/64bit arithmetics @ armv7a
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69447 Richard Henderson changed: What|Removed |Added Status|NEW |ASSIGNED --- Comment #8 from Richard Henderson --- Further archaeology suggests that PR 66424 is related, and had an incomplete fix applied. The fix for PR 66424 prevents an insn being moved if it contains a subreg of a multi-word pseudo. But the same problem exists if one moves a complete multi-word pseudo if any of its subregs have ever been accessed.
[Bug rtl-optimization/69447] [5/6 Regression] wrong code with -O2 -fno-schedule-insns and mixed 8/16/32/64bit arithmetics @ armv7a
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69447 --- Comment #9 from Richard Henderson --- Created attachment 37484 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=37484=edit proposed patch Fixes the test case, in that it prevents the remat. Starting overnight bootstraps for armv7hf, i686, and x86_64...
[Bug rtl-optimization/69447] [5/6 Regression] wrong code with -O2 -fno-schedule-insns and mixed 8/16/32/64bit arithmetics @ armv7a
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69447 --- Comment #4 from ktkachov at gcc dot gnu.org --- -fno-lra-remat makes the testcase pass. Could this be a dup of PR 69124?
[Bug rtl-optimization/69447] [5/6 Regression] wrong code with -O2 -fno-schedule-insns and mixed 8/16/32/64bit arithmetics @ armv7a
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69447
--- Comment #5 from Richard Henderson ---
This appears to be exposed by lra-remat, but not
the location of the bug.
Before remat:
Insn 55: point = 3
...
Insn 17: point = 26
r111: [4..26]
After remat:
Inserting rematerialization insn before:
76: {r0:DI=r111:DI-0x2;clobber cc:CC;}
Insn 55: point = 3
...
Insn 76: point = 7
...
Insn 17: point = 24
So remat hasn't expanded the lifetime of r111 at all,
merely introduced a new use within the existing live
range. So the failure to keep the top half of the
pseudo live is elsewhere.
[Bug rtl-optimization/69447] [5/6 Regression] wrong code with -O2 -fno-schedule-insns and mixed 8/16/32/64bit arithmetics @ armv7a
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69447 --- Comment #6 from Richard Henderson --- ... except that's not the same set of live ranges computed by IRA: Insn 55(l0): point = 9 Insn 70(l0): point = 11 ... Insn 50(l0): point = 19 ... Insn 21(l0): point = 61 Insn 19(l0): point = 65 Insn 17(l0): point = 67 a3(r111 [0]): [10..67] a3(r111 [1]): [62..67] If we remat insn 21 into insn 50, we don't extend the lifetime of r111[0], but we do extend the lifetime of r111[1]. The problem appears to be that IRA takes subregs into account but LRA doesn't. Vlad, am I on the right track here?
[Bug rtl-optimization/69447] [5/6 Regression] wrong code with -O2 -fno-schedule-insns and mixed 8/16/32/64bit arithmetics @ armv7a
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=69447 --- Comment #7 from Vladimir Makarov --- (In reply to Richard Henderson from comment #6) > ... except that's not the same set of live ranges > computed by IRA: > >Insn 55(l0): point = 9 >Insn 70(l0): point = 11 >... >Insn 50(l0): point = 19 >... >Insn 21(l0): point = 61 >Insn 19(l0): point = 65 >Insn 17(l0): point = 67 > > a3(r111 [0]): [10..67] > a3(r111 [1]): [62..67] > > If we remat insn 21 into insn 50, we don't extend the lifetime > of r111[0], but we do extend the lifetime of r111[1]. > > The problem appears to be that IRA takes subregs into account > but LRA doesn't. Vlad, am I on the right track here? May be. Bernd extended IRA lately to follow separate subreg live ranges (only 2 subregs). LRA does not do that.
