This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Followup on previous inheritance fix


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)



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]