This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Followup on previous inheritance fix
- To: egcs-patches at cygnus dot com
- Subject: Followup on previous inheritance fix
- From: Bernd Schmidt <crux at pool dot informatik dot rwth-aachen dot de>
- Date: Mon, 26 Oct 1998 13:26:37 +0100 (MET)
Two weeks ago I submitted a patch that tried to fix a problem in the
inheritance code, where modifying an inherited register could clobber data
in a pseudo. I have now found some more problems in this area.
First, the previous change was buggy: the test whether a hard reg is used
by a pseudo was reversed. I have no idea why it seemed to fix the problem.
The other problem with this code is that it tests reg_reloaded_dead. The
relevant portion of the original condition looked like this:
/* Don't use it if we'd clobber a pseudo reg. */
if (spill_reg_order[i] < 0
&& reload_out[r]
&& ! TEST_HARD_REG_BIT (reg_reloaded_dead, i))
/* don't use it as reload_reg_rtx, just for reload_override_in */
Translated to English, this reads: if the register is used by a pseudo
(spill_reg_order[i] < 0), and we're going to modify it (reload_out[r]),
and the hard reg wasn't reloaded from a pseudo that died in the last reload
insn, then we can't use I as reload reg.
The first problem with using reg_reloaded_dead is that after the recent
inheritance changes, it can contain random data. The code in emit_reload_insns
which the patch below modifies can set reg_reloaded_valid for certain hard
regs, without clearing the corresponding bit in reg_reloaded_dead. Thus,
if the bit was 1 before, it will continue to have the value 1. This confuses
the if statement above.
The second problem with using reg_reloaded_dead in that condition is that it
seems wrong. Consider the situation where we store hard reg A from pseudo B,
and B dies. Then, in the following insn, reg_reloaded_dead is 1 for A, so the
condition above does not apply. This means the compiler could try to use
A for an in-out reload in the circumstances described above, _even if there
is a store to another pseudo that has hard reg A_ in the same insn.
I believe that we should not test reg_reloaded_dead here, just avoid this
optimization whenever reg_used_by_pseudo is nonzero.
Bernd
* reload1.c (choose_reload_regs): Fix typo in Oct 17 change.
In the same condition, don't test reg_reloaded_dead.
(emit_reload_insns): Ensure that when we set reg_reloaded_valid for
any hard reg, reg_reloaded_dead contains valid data.
Index: reload1.c
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/reload1.c,v
retrieving revision 1.85
diff -u -p -r1.85 reload1.c
--- reload1.c 1998/10/22 22:49:04 1.85
+++ reload1.c 1998/10/26 12:08:07
@@ -6189,9 +6162,8 @@ choose_reload_regs (chain, avoid_return_
if (i1 != n_earlyclobbers
/* Don't use it if we'd clobber a pseudo reg. */
- || (! TEST_HARD_REG_BIT (reg_used_by_pseudo, i)
- && reload_out[r]
- && ! TEST_HARD_REG_BIT (reg_reloaded_dead, i))
+ || (TEST_HARD_REG_BIT (reg_used_by_pseudo, i)
+ && reload_out[r])
/* Don't really use the inherited spill reg
if we need it wider than we've got it. */
|| (GET_MODE_SIZE (reload_mode[r])
@@ -7841,6 +7813,7 @@ emit_reload_insns (chain)
spill_reg_stored_to[src_regno + nr] = out;
reg_reloaded_contents[src_regno + nr] = nregno;
reg_reloaded_insn[src_regno + nr] = store_insn;
+ CLEAR_HARD_REG_BIT (reg_reloaded_dead, src_regno + nr);
SET_HARD_REG_BIT (reg_reloaded_valid, src_regno + nr);
SET_HARD_REG_BIT (reg_is_output_reload, src_regno + nr);
if (note)