[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
--- Comment #1 from hp at gcc dot gnu dot org 2009-05-10 00:04 --- Created an attachment (id=17839) --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=17839&action=view) slightly reduced test-case It's the second abort call that's executed, but removing the first causes the test to pass. Here's a slightly reduced case, removing the parts after the abort call. Tell me if you need further assistance. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
--- Comment #2 from bonzini at gnu dot org 2009-05-10 17:22 --- Well, if only -O1 fails now, it is almost definitely my patch. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
--- Comment #3 from hp at gcc dot gnu dot org 2009-05-10 19:26 --- (In reply to comment #2) > Well, if only -O1 fails now, it is almost definitely my patch. It definitely is, there are no other authors in that revision range! :) -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
--- Comment #4 from bonzini at gnu dot org 2009-05-11 10:15 --- I meant my fwprop patch. :-) -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
--- Comment #5 from bonzini at gnu dot org 2009-05-11 12:28 ---
It's a latent bug somewhere, possibly in delayed branch scheduling? fwprop1
does a single copy propagation which is trivially valid.
Analyzing -O1 -fno-delayed-branch vs. -O1 -fno-delayed-branch
-fno-forward-propagate, I'm pretty sure that every change remains valid up to
the dbr pass. In particular, as i said, the effect of fwprop is trivial:
L154:
155 NOTE_INSN_BASIC_BLOCK
156 r94:SI=r97:SI+0x1
- 157 r92:SI=r94:SI
158 r126:SI=r8:SI-0xa
159 cc0=[r126:SI+r97:SI]
160 pc={(cc0==0x0)?L165:pc}
@@ -913,7 +906,7 @@
161 NOTE_INSN_BASIC_BLOCK
162 r127:SI=r8:SI-0x41
163 r128:SI=r97:SI<<0x2
- 164 [r127:SI+r128:SI]=r92:SI
+ 164 [r127:SI+r128:SI]=r94:SI
L165:
166 NOTE_INSN_BASIC_BLOCK
168 cc0=cmp(r94:SI,0xa)
Also after register allocation there is no major difference, except that
without fwprop r92 is allocated to the (special?) register $srp, giving:
L154:
155 NOTE_INSN_BASIC_BLOCK
- 265 r9:SI=r13:SI
- 156 r9:SI=r9:SI+0x1
- 157 srp:SI=r9:SI
+ 265 r13:SI=r9:SI
+ 156 r13:SI=r13:SI+0x1
266 r12:SI=r8:SI
158 r12:SI=r12:SI-0xa
REG_EQUIV: r8:SI-0xa
- 159 cc0=[r12:SI+r13:SI]
+ 159 cc0=[r12:SI+r9:SI]
160 pc={(cc0==0x0)?L165:pc}
REG_BR_PROB: 0x1388
161 NOTE_INSN_BASIC_BLOCK
@@ -1025,14 +1009,14 @@
267 r12:SI=r8:SI
243 r12:SI=r12:SI-0x41
REG_EQUIV: r8:SI-0x41
- 164 [r13:SI*0x4+r12:SI]=srp:SI
+ 164 [r9:SI*0x4+r12:SI]=r13:SI
L165:
166 NOTE_INSN_BASIC_BLOCK
- 168 cc0=cmp(r9:SI,0xa)
+ 168 cc0=cmp(r13:SI,0xa)
169 pc={(cc0==0x0)?L167:pc}
REG_BR_PROB: 0x38d
172 NOTE_INSN_BASIC_BLOCK
- 55 r13:SI=r9:SI
+ 55 r9:SI=r13:SI
237 pc=L154
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
--- Comment #6 from hp at gcc dot gnu dot org 2009-05-11 19:40 --- (In reply to comment #4) > I meant my fwprop patch. :-) That's the patch in that revision range, yes... (FWIW, $SRP is the subroutine return pointer, valid to use as data storage if saved.) It's a bit unclear whether L154 is fall-through && R9 live and holding "r97" before your second dump snippet, and the first diff (both reversed, I believe?) doesn't tell whether *all* uses of r92 have been replaced. I'll have a look myself. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
--- Comment #7 from hp at gcc dot gnu dot org 2009-05-11 20:47 --- (In reply to comment #5) > It's a latent bug somewhere, possibly in delayed branch scheduling? With vs. without -fno-delayed-branch at 147274 seems to justify this blame. Ugh. I'll see if I can nail it. But, for the record, having said that, still your exposure, your bug. ;) -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
--- Comment #8 from hp at gcc dot gnu dot org 2009-05-11 21:14 --- I see at least one invalid dbr transformation. Assuming you can read m68k assembler, I'm sure you'll have no problem gripping the important parts of: subq 25,$r13 test.b [$r13+$r9.b] - beq .L6 - nop + beq .L33 + addq 1,$r9 + subu.b 100,$r13 clear.d [$r13+$r9.d] -.L6: addq 1,$r9 +.L33: That's right, dbr moves an increment of $r9 *over a use* (the "clear.d [$r13+$r9.d]" is like "[r9:SI*0x4+r13:SI]=0"). Bah. Over to gdb... -- hp at gcc dot gnu dot org changed: What|Removed |Added Status|UNCONFIRMED |NEW Ever Confirmed|0 |1 Last reconfirmed|-00-00 00:00:00 |2009-05-11 21:14:13 date|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
--- Comment #9 from hp at gcc dot gnu dot org 2009-05-11 21:19 --- (In reply to comment #8) > dbr moves an increment of $r9 *over a use* Actually, "copies", not "moves", which is even worse (sort of; like "how could it be worse"). I'll take it. -- hp at gcc dot gnu dot org changed: What|Removed |Added AssignedTo|unassigned at gcc dot gnu |hp at gcc dot gnu dot org |dot org | Status|NEW |ASSIGNED Last reconfirmed|2009-05-11 21:14:13 |2009-05-11 21:19:04 date|| http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
--- Comment #10 from bonzini at gnu dot org 2009-05-12 05:58 --- Thanks H-P. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
--- Comment #11 from hp at gcc dot gnu dot org 2009-05-13 00:09 ---
Two insns above the assembly sequence for the delay-slot fill quoted above,
there used to be a "clear.d $r9". That insn is moved into a delay-slot in the
first reorg_pass_number round by fill_simple_delay_slots (0). As a dataflow
marker, the insn is replaced by a USE:
(insn 308 204 94 (use (insn/s 52 286 310 (set (reg:SI 9 r9 [orig:95 ivtmp.101 ]
[95])
(const_int 0 [0x0])) 38 {*movsi_internal} (expr_list:REG_EQUAL
(const_int 0 [0x0])
(nil -1 (nil))
The bug seems to be that at the next relax_delay_slots (first) call, the USE
placeholder is removed for some reason. The df live-regs info is then
incorrect and at the next (the last) round, when fill_eager_delay_slots () is
called, it does the invalid delay-slot filling quoted above.
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
--- Comment #12 from hp at gcc dot gnu dot org 2009-05-13 13:09 --- The relax_delay_slots (first) call finds that there's a branch to a redundant insn that it can eliminate. It does this by redirecting the branch to a new or existing label and deleting the insn, or rather move it and make it a USE-mark at the "new" label. When doing this, the old USE-mark quoted above (not related to the new USE-mark by other means being before it in the execution path) is now deemed unreachable and removed by jump.c:delete_related_insns. So, it seems a solution will include propagating existing USE-marks when redirecting branch labels, perhaps in reorg_redirect_jump or update_block. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
--- Comment #13 from hp at gcc dot gnu dot org 2009-05-14 00:12 --- By chance I stumbled upon an old fix I did some years ago in which I changed a use of next_active_insn to next_real insn (to avoid skipping USE insns). You can see it in comments referring to a now-deleted "main" use (the references gone stale). This PR is just the same bug in another place! I think I'll be going for the same solution; it will reduce the number of filled delay-slots or redirected jumps and removed redundant insns, but presumably safer as it will not include moving or duplicating the USE insns. It would also be solved by a reorg rewrite, in which USE insns wouldn't be sprinkled into the insn stream, sometimes ignored, sometimes incorrectly... -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
--- Comment #14 from hp at gcc dot gnu dot org 2009-05-14 01:18 --- Created an attachment (id=17861) --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=17861&action=view) patch for just the bug at hand. This patch just changes the next_active_insn for the exposure in this PR. I think most uses of next_active_insn in reorg.c and resource.c should be changed to next_real_insn. Or reorg.c rewritten, at least to not use in-stream liveness markers like those USE insns. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
--- Comment #15 from hp at gcc dot gnu dot org 2009-05-14 11:17 --- patch posted after testing -- hp at gcc dot gnu dot org changed: What|Removed |Added URL||http://gcc.gnu.org/ml/gcc- ||patches/2009- ||05/msg00811.html Keywords||patch, wrong-code http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
-- rguenth at gcc dot gnu dot org changed: What|Removed |Added Target Milestone|--- |4.5.0 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
-- rguenth at gcc dot gnu dot org changed: What|Removed |Added Priority|P3 |P4 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
--- Comment #16 from bonzini at gnu dot org 2009-05-24 08:04 --- Subject: Re: [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution I'll note that, even though the bug is very rare and no testcase is known in general, it affects every target with delay slots -- so it should be P2 IMO. -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
--- Comment #18 from hp at gcc dot gnu dot org 2009-11-30 07:13 --- Subject: Bug 40086 Author: hp Date: Mon Nov 30 07:13:21 2009 New Revision: 154751 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=154751 Log: PR rtl-optimization/40086 * reorg.c (relax_delay_slots): When looking for redundant insn at the branch target, use next_real_insn, not next_active_insn. Modified: trunk/gcc/ChangeLog trunk/gcc/reorg.c -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
--- Comment #19 from hp at gcc dot gnu dot org 2009-11-30 07:19 --- a comment -- hp at gcc dot gnu dot org changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution||FIXED http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086 --- Comment #20 from John David Anglin 2011-05-23 22:31:59 UTC --- Author: danglin Date: Mon May 23 22:31:55 2011 New Revision: 174090 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=174090 Log: PR rtl-optimization/49007 Backport from mainline: 2009-11-30 Hans-Peter Nilsson PR rtl-optimization/40086 * reorg.c (relax_delay_slots): When looking for redundant insn at the branch target, use next_real_insn, not next_active_insn. Modified: branches/gcc-4_4-branch/gcc/ChangeLog branches/gcc-4_4-branch/gcc/reorg.c
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086 --- Comment #21 from John David Anglin 2011-05-24 18:44:38 UTC --- Author: danglin Date: Tue May 24 18:44:35 2011 New Revision: 174132 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=174132 Log: PR rtl-optimization/49007 Backport from mainline: 2009-11-30 Hans-Peter Nilsson PR rtl-optimization/40086 * reorg.c (relax_delay_slots): When looking for redundant insn at the branch target, use next_real_insn, not next_active_insn. Modified: branches/gcc-4_3-branch/gcc/ChangeLog branches/gcc-4_3-branch/gcc/reorg.c
[Bug rtl-optimization/40086] [4.5 Regression]: cris-elf gfortran.dg/forall_1.f90 -O1 execution
--- Comment #17 from hp at gcc dot gnu dot org 2009-08-16 00:31 --- For the record, the gfortran.dg/forall_1.f90 regression has been covered since r150588. (And I'm still on the hook to address the concerns with the posted patch.) -- http://gcc.gnu.org/bugzilla/show_bug.cgi?id=40086
