This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[RFA:] Fix PR 40086 - reorg.c again and again
- From: Hans-Peter Nilsson <hans-peter dot nilsson at axis dot com>
- To: gcc-patches at gcc dot gnu dot org
- Date: Thu, 14 May 2009 13:16:07 +0200
- Subject: [RFA:] Fix PR 40086 - reorg.c again and again
PR rtl-optimization/40086 is about reorg.c tripping on its own
hacked dataflow info, exposed by a totally unrelated patch that
happened to cause a gfortran test-case to FAIL. The delay-slots
created in one pass are "optimized" (redundant insns removed),
but when doing that, the dataflow info is hosed, so in the next
pass reorg.c makes an invalid delay-slot-fill.
A sad thing is that I fixed a similar bug almost exactly five
years ago (PR optimization/15296). See
<http://gcc.gnu.org/ml/gcc-patches/2004-05/msg00246.html>. It
doesn't help now, though, except making it blindingly obvious
yet again that reorg.c needs more than one change.
In the trail for *this* PR 40086, I listed a couple of
alternative solutions, but I went for the same fix as in PR
optimization/15296. In the patch below, I changed just one
next_active_insn to next_real insn, the one exposed by the
regression in the PR. What I'd like to do is to change *all*
calls to next_active_insn into next_real insn in reorg.c and
resource.c (not completely correct; what I'd *really* like to do
is to rewrite reorg.c, alas...)
Tested cross to cris-elf and mips-elf.
Ok for trunk and all open branches?
How about a patch changing all next_active_insn (well, with
some individual consideration of course) into next_real_insn?
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.
Index: gcc/reorg.c
===================================================================
--- gcc/reorg.c (revision 147274)
+++ gcc/reorg.c (working copy)
@@ -3501,8 +3501,11 @@
}
/* If the first insn at TARGET_LABEL is redundant with a previous
- insn, redirect the jump to the following insn process again. */
- trial = next_active_insn (target_label);
+ insn, redirect the jump to the following insn and process again.
+ We use next_real_insn instead of next_active_insn so we
+ don't skip USE-markers, or we'll end up with incorrect
+ liveness info. */
+ trial = next_real_insn (target_label);
if (trial && GET_CODE (PATTERN (trial)) != SEQUENCE
&& redundant_insn (trial, insn, 0)
&& ! can_throw_internal (trial))
brgds, H-P