Rask Ingemann Lambertsen wrote:
On Mon, May 14, 2007 at 10:47:13PM +0100, Mark Shinwell wrote:
I'm fairly certain that this is the correct approach to fix this
issue, but I'm less certain that I have correctly guarded the call
to forget_old_reloads_1, and indeed that I've done everything to
eradicate the previous reloads involving H. For example, should I be
wiping the necessary range in reg_last_reload_reg?
No, any use of reg_last_reload_reg should be guarded by
TEST_HARD_REG_BIT(reg_reloaded_valid, reg). It is just an optimization to
avoid a linear scan of reg_reloaded_contents.
OK, thanks.
There are two places in reload_as_needed() where note_stores() calls
forget_old_reloads(). The first one at the top before emitting reload insns:
else if (INSN_P (insn))
{
regset_head regs_to_forget;
INIT_REG_SET (®s_to_forget);
note_stores (PATTERN (insn), forget_old_reloads_1, ®s_to_forget);
This one won't catch the store to the reload register r9 that causes
the clobber because it's only scanning PATTERN (insn). It's the same
issue as noted in the existing comment in emit_reload_insns:
/* If a register gets output-reloaded from a non-spill register,
that invalidates any previous reloaded copy of it.
But forget_old_reloads_1 won't get to see it, because
it thinks only about the original insn. So invalidate it here.
Also do the same thing for RELOAD_OTHER constraints where the
output is discarded. */
The second one after emitting reload insns, just before the big
AUTO_INC_DEC block at the end:
if (num_eliminable && chain->need_elim)
update_eliminable_offsets ();
/* Any previously reloaded spilled pseudo reg, stored in this
* insn,
is no longer validly lying around to save a future reload.
Note that this does not detect pseudos that were reloaded
for this insn in order to be stored in
(obeying register constraints). That is correct; such reload
registers ARE still valid. */
forget_marked_reloads (®s_to_forget);
CLEAR_REG_SET (®s_to_forget);
/* There may have been CLOBBER insns placed after INSN. So scan
between INSN and NEXT and use them to forget old reloads. */
for (x = NEXT_INSN (insn); x != old_next; x = NEXT_INSN (x))
if (NONJUMP_INSN_P (x) && GET_CODE (PATTERN (x)) == CLOBBER)
note_stores (PATTERN (x), forget_old_reloads_1, NULL);
#ifdef AUTO_INC_DEC
/* Likewise for regs altered by auto-increment in this insn.
This case relies on the presence of a CLOBBER though, which we don't
have.
As per Bernd's request I'll provide some more reload/RTL dumps shortly
which might clarify this.
> I'm thinking that maybe we should
include the reload insns themselves in the scan.
I'm not suitably qualified to answer this one, but it sounds more
invasive than my current patch.
Mark
