[4.2 only] RFA: PR 33848: reload rematerialisation of labels
Richard Sandiford
rsandifo@nildram.co.uk
Sat Oct 27 11:02:00 GMT 2007
Eric Botcazou <ebotcazou@libertysurf.fr> writes:
>> Well, we build jump labels immediately after expanding, and try to keep
>> them up-to-date when changing jumps later. Not keeping them up-to-date
>> would be a bug.
>
> It's 4.2.x so we have to be conservative and avoid pulling too long a string,
> that's why I would go for a very safe change in this case.
>
> The original code was:
>
> + /* If we're replacing a LABEL_REF with a register, add a
> + REG_LABEL note to indicate to flow which label this
> + register refers to. */
> + if (GET_CODE (*r->where) == LABEL_REF
> + && GET_CODE (insn) == JUMP_INSN)
> + REG_NOTES (insn) = gen_rtx_EXPR_LIST (REG_LABEL,
> + XEXP (*r->where, 0),
> + REG_NOTES (insn));
>
> Then it was modified as follows
>
> @@ -6033,9 +6033,12 @@ subst_reloads (rtx insn)
> register refers to. */
> if (GET_CODE (*r->where) == LABEL_REF
> && JUMP_P (insn))
> - REG_NOTES (insn) = gen_rtx_INSN_LIST (REG_LABEL,
> - XEXP (*r->where, 0),
> - REG_NOTES (insn));
> + {
> + REG_NOTES (insn) = gen_rtx_INSN_LIST (REG_LABEL,
> + XEXP (*r->where, 0),
> + REG_NOTES (insn));
> + JUMP_LABEL (insn) = XEXP (*r->where, 0);
> + }
>
> /* Encapsulate RELOADREG so its machine mode matches what
> used to be there. Note that gen_lowpart_common will
>
>
> I'm afraid your change would void the original intent in the name of fixing
> the subsequent modification. Couldn't we find a middle-ground instead?
I don't think it would reopen the original wound. The change was for
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20606, and as Andrew says
in comment #3:
------------------------------------------------------------------------
(In reply to comment #2)
> Why is computed_jump_p (insn) not returning true?
Actually it is the following insn:
(jump_insn 169 222 170 19 (set (pc)
(reg:DI 0 ax)) 515 {*indirect_jump_rtx64} (nil)
(insn_list:REG_LABEL 172 (nil)))
Which is a computed goto but at this point we have a REG_LABEL which
causes computed_jump_p to return false
------------------------------------------------------------------------
In other words, by unconditionally adding a REG_LABEL note, regardless
of whether there was a JUMP_LABEL, reload was turning something that
satisfied computed_jump_p into something that was neither a computed
jump nor a correct register-indirect jump to a known target.
My patch fixes the same bug by making sure that we only attach
a REG_LABEL note to jumps that already have a known target.
So with my patch, the jump in the testcase would still satsify
computed_jump_p after reload. And I think it's a valid fix.
While turning a computed jump into a jump to a known target
is an interesting optimisation, this is not the place to do it.
Reload only sees a LABEL_REF here because the original pseudo
did not get allocated a hard register, and the transformation
from a computed jump to a jump to a known target should not
depend on register allocation failure. The code above is purely
book keeping; the net effect of reload doesn't change the form
of the instruction, which is register-indirect both before and
after register allocation.
Richard
More information about the Gcc-patches
mailing list