Quoting Eric Botcazou <ebotca...@adacore.com>:
+static bool
+move2add_valid_value_p (int regno, enum machine_mode mode)
[...]
+ for (i = hard_regno_nregs[regno][mode] - 1; i > 0; i--)
+ if (reg_mode[i] != BLKmode)
+ return false;
I think that a 'regno' is missing in the second hunk.
Huh? Could you be more specific?
If you mean I should check reg_mode[regno], that is tested a few lines above.
And can you use the
same idiom for the loop in both cases (same initial value and iteration)?
Sure. I rembered that Jeff prefered .. - 1; i > 0; i--), but forgot I had
a second instance of ; --i > 0; ).
@@ -1787,8 +1848,7 @@ move2add_use_add3_insn (rtx reg, rtx sym
SET_SRC (pat) = plus_expr;
for (i = 0; i < FIRST_PSEUDO_REGISTER; i++)
- if (reg_set_luid[i] > move2add_last_label_luid
- && reg_mode[i] == GET_MODE (reg)
+ if (move2add_valid_value_p (i, GET_MODE (reg))
&& reg_base_reg[i] < 0
&& reg_symbol_ref[i] != NULL_RTX
&& rtx_equal_p (sym, reg_symbol_ref[i]))
Note that you're weakening the mode equality test here.
Weakening inasmuch as it allows noop truncation, strengthening inasmuch
as it checks that all part of a multi-hard-reg reg are up to date
(which is necessary because I dropped some checks to only allow single
regs). Overall, it makes the logic more consistent.
@@ -2045,7 +2099,10 @@ reload_cse_move2add (rtx first)
/* Reset the information about this register. */
int regno = REGNO (XEXP (note, 0));
if (regno < FIRST_PSEUDO_REGISTER)
- reg_set_luid[regno] = 0;
+ {
+ move2add_record_mode (XEXP (note, 0));
+ reg_set_luid[regno] = 0;
+ }
}
}
note_stores (PATTERN (insn), move2add_note_store, insn);
See comment on third hunk of this set below
@@ -2082,7 +2139,7 @@ reload_cse_move2add (rtx first)
{
if (call_used_regs[i])
/* Reset the information about this register. */
- reg_set_luid[i] = 0;
+ reg_mode[i] = VOIDmode;
This is for a single hard register.
Setting the mode to VOIDmode invalidates all uses.
> @@ -2262,17 +2288,17 @@ move2add_note_store (rtx dst, const_rtx
[...]
+ invalidate:
+ /* Invalidate the contents of the register. */
+ reg_set_luid[regno] = 0;
+ move2add_record_mode (dst);
The intent is the same, why don't we have the same code?
move2add_record_mode (dst) invalidates all but the first reg.
I left the invalidation of the first reg set reg_set_luid on the theory
that this causes less churn (albeit the code is moved, too).
Now, on second thought, I should really use reg_mode[regno] VOIDmode
for the first reg here instead, to make sure this reg ceases to be
tracked as part of a multi-hard-reg register starting at an earlier regnum.###
