Alexandre Oliva via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> When the mode of regno_reg_rtx is not hard_regno_mode_ok for the
> target, try grouping the register with subsequent ones. This enables
> s16 to s31 and their hidden pairs to be zeroed with the default logic
> on some arm variants.
>
> Regstrapped on x86_64-linux-gnu, also tested on an affected arm
> configuration. Ok to install?
>
>
> for gcc/ChangeLog
>
> * targhooks.c (default_zero_call_used_regs): Attempt to group
> regs that the target refuses to use in their natural modes.
Thanks for doing this. Some comments below…
> ---
> gcc/targhooks.cc | 79
> ++++++++++++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 70 insertions(+), 9 deletions(-)
>
> diff --git a/gcc/targhooks.cc b/gcc/targhooks.cc
> index fc49235eb38ee..bdaab9c63c7ee 100644
> --- a/gcc/targhooks.cc
> +++ b/gcc/targhooks.cc
> @@ -1035,16 +1035,45 @@ default_zero_call_used_regs (HARD_REG_SET
> need_zeroed_hardregs)
> if (TEST_HARD_REG_BIT (need_zeroed_hardregs, regno))
> {
> rtx_insn *last_insn = get_last_insn ();
> - machine_mode mode = GET_MODE (regno_reg_rtx[regno]);
> + rtx regno_rtx = regno_reg_rtx[regno];
> + machine_mode mode = GET_MODE (regno_rtx);
> +
> + /* If the natural mode doesn't work, try some wider mode. */
> + if (!targetm.hard_regno_mode_ok (regno, mode))
> + {
> + for (int nregs = 2;
> + regno + nregs <= FIRST_PSEUDO_REGISTER
> + && TEST_HARD_REG_BIT (need_zeroed_hardregs,
> + regno + nregs - 1);
> + nregs++)
> + {
> + mode = choose_hard_reg_mode (regno, nregs, 0);
I like the idea, but it would be good to avoid the large:
FIRST_PSEUDO_REGISTER * FIRST_PSEUDO_REGISTER * NUM_MACHINE_MODES
constant factor. How about if init_reg_modes_target recorded the
maximum value of x_hard_regno_nregs?
> + if (mode == E_VOIDmode)
> + continue;
> + gcc_checking_assert (targetm.hard_regno_mode_ok (regno, mode));
> + regno_rtx = gen_rtx_REG (mode, regno);
> + break;
> + }
> + if (mode != GET_MODE (regno_rtx)
> + || regno_rtx == regno_reg_rtx[regno])
> + {
> + SET_HARD_REG_BIT (failed, regno);
> + continue;
> + }
> + }
> +
> rtx zero = CONST0_RTX (mode);
> - rtx_insn *insn = emit_move_insn (regno_reg_rtx[regno], zero);
> + rtx_insn *insn = emit_move_insn (regno_rtx, zero);
> if (!valid_insn_p (insn))
> {
> SET_HARD_REG_BIT (failed, regno);
> delete_insns_since (last_insn);
> }
> else
> - progress = true;
> + {
> + progress = true;
> + regno += hard_regno_nregs (regno, mode) - 1;
> + }
> }
>
> /* Now retry with copies from zeroed registers, as long as we've
> @@ -1060,7 +1089,34 @@ default_zero_call_used_regs (HARD_REG_SET
> need_zeroed_hardregs)
> for (unsigned int regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++)
> if (TEST_HARD_REG_BIT (retrying, regno))
> {
> - machine_mode mode = GET_MODE (regno_reg_rtx[regno]);
> + rtx regno_rtx = regno_reg_rtx[regno];
> + machine_mode mode = GET_MODE (regno_rtx);
> +
> + /* If the natural mode doesn't work, try some wider mode. */
> + if (!targetm.hard_regno_mode_ok (regno, mode))
> + {
> + for (int nregs = 2;
> + regno + nregs <= FIRST_PSEUDO_REGISTER
> + && TEST_HARD_REG_BIT (need_zeroed_hardregs,
> + regno + nregs - 1);
> + nregs++)
> + {
> + mode = choose_hard_reg_mode (regno, nregs, 0);
> + if (mode == E_VOIDmode)
> + continue;
> + gcc_checking_assert (targetm.hard_regno_mode_ok (regno,
> + mode));
> + regno_rtx = gen_rtx_REG (mode, regno);
> + break;
> + }
> + if (mode != GET_MODE (regno_rtx)
> + || regno_rtx == regno_reg_rtx[regno])
> + {
> + SET_HARD_REG_BIT (failed, regno);
> + continue;
> + }
> + }
> +
This seems big enough to be worth splitting out into a helper, rather
than repeating. That should also simplify the failure detection:
the helper can return nonnull on success and null on failure.
> bool success = false;
> /* Look for a source. */
> for (unsigned int src = 0; src < FIRST_PSEUDO_REGISTER; src++)
> @@ -1086,8 +1142,10 @@ default_zero_call_used_regs (HARD_REG_SET
> need_zeroed_hardregs)
>
> /* SRC is usable, try to copy from it. */
> rtx_insn *last_insn = get_last_insn ();
> - rtx zsrc = gen_rtx_REG (mode, src);
> - rtx_insn *insn = emit_move_insn (regno_reg_rtx[regno], zsrc);
> + rtx src_rtx = (mode == GET_MODE (regno_reg_rtx[src])
> + ? regno_reg_rtx[src]
> + : gen_rtx_REG (mode, src));
Is this needed? The original gen_rtx_REG (mode, src) seems OK.
Thanks,
Richard
> + rtx_insn *insn = emit_move_insn (regno_rtx, src_rtx);
> if (!valid_insn_p (insn))
> /* It didn't work, remove any inserts. We'll look
> for another SRC. */
> @@ -1100,13 +1158,16 @@ default_zero_call_used_regs (HARD_REG_SET
> need_zeroed_hardregs)
> }
> }
>
> - /* If nothing worked for REGNO this round, marked it to be
> + /* If nothing worked for REGNO this round, mark it to be
> retried if we get another round. */
> if (!success)
> SET_HARD_REG_BIT (failed, regno);
> else
> - /* Take note so as to enable another round if needed. */
> - progress = true;
> + {
> + /* Take note so as to enable another round if needed. */
> + progress = true;
> + regno += hard_regno_nregs (regno, mode) - 1;
> + }
> }
> }
