This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [RFC][PATCH][MIPS] Patch to enable LRA for MIPS backend
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: Robert Suchanek <Robert dot Suchanek at imgtec dot com>
- Cc: "gcc-patches\ at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, "vmakarov\ at redhat dot com" <vmakarov at redhat dot com>
- Date: Wed, 23 Apr 2014 15:05:39 +0100
- Subject: Re: [RFC][PATCH][MIPS] Patch to enable LRA for MIPS backend
- Authentication-results: sourceware.org; auth=none
- References: <B5E67142681B53468FAF6B7C313565623D3DD70C at KLMAIL01 dot kl dot imgtec dot org> <87d2h51dm6 dot fsf at talisman dot default> <B5E67142681B53468FAF6B7C313565623D3E3FED at KLMAIL01 dot kl dot imgtec dot org> <87d2gqfb7t dot fsf at talisman dot default> <B5E67142681B53468FAF6B7C313565623D3E4420 at KLMAIL01 dot kl dot imgtec dot org> <87ob02jodp dot fsf at talisman dot default> <B5E67142681B53468FAF6B7C313565623D3E460A at KLMAIL01 dot kl dot imgtec dot org> <87fvl6hnw2 dot fsf at talisman dot default> <B5E67142681B53468FAF6B7C313565623D3E5D0F at KLMAIL01 dot kl dot imgtec dot org>
Robert Suchanek <Robert.Suchanek@imgtec.com> writes:
> If we were going to XFAIL the test then it would apply specifically
> for -mips16 -O1. In any other combination it appears to work. Would
> that be a stopper?
Hmm, in that case maybe we should just leave it failing. The alternative
would be to skip the test altogther for MIPS, with a PR referencing it,
but that seems a bit over-the-top.
> 2014-03-26 Robert Suchanek <Robert.Suchanek@imgtec.com>
>
> * lra-constraints.c (base_to_reg): New function.
> (process_address): Use new function.
>
> * config/mips/constraints.md ("d"): BASE_REG_CLASS
> replaced by "TARGET_MIPS16 ? M16_REGS : GR_REGS".
> * config/mips/mips.c (mips_regno_mode_ok_for_base_p):
> Remove use !strict_p for MIPS16.
> (mips_register_priority): New function that implements
> the target hook TARGET_REGISTER_PRIORITY.
> (mips_spill_class): Likewise for TARGET_SPILL_CLASS
> (mips_lra_p): Likewise for TARGET_LRA_P.
> * config/mips/mips.h (reg_class): Add M16_SP_REGS and SPILL_REGS
> classes.
> (REG_CLASS_NAMES): Likewise.
> (REG_CLASS_CONTENTS): Likewise.
> (BASE_REG_CLASS): Use M16_SP_REGS.
> * config/mips/mips.md (*mul_acc_si, *mul_sub_si): Add alternative
> tuned for LRA. New set attribute to enable alternatives
> depending on the register allocator used.
> (*lea64): Disable pattern for MIPS16.
> * config/mips/mips.opt
> (mlra): New option
Looks good.
> @@ -12115,6 +12102,18 @@ mips_register_move_cost (enum machine_mode mode,
> return 0;
> }
>
> +/* Return a register priority for hard reg REGNO. */
> +
> +static int
> +mips_register_priority (int hard_regno)
> +{
> + /* Treat MIPS16 registers with higher priority than other regs. */
> + if (TARGET_MIPS16
> + && TEST_HARD_REG_BIT (reg_class_contents[M16_REGS], hard_regno))
> + return 1;
> + return 0;
> +}
> +
> /* Implement TARGET_MEMORY_MOVE_COST. */
>
> static int
> @@ -18897,6 +18896,21 @@ mips_atomic_assign_expand_fenv (tree *hold, tree *clear, tree *update)
> *update = build2 (COMPOUND_EXPR, void_type_node, *update,
> atomic_feraiseexcept_call);
> }
> +
> +static reg_class_t
> +mips_spill_class (reg_class_t rclass ATTRIBUTE_UNUSED,
> + enum machine_mode mode ATTRIBUTE_UNUSED)
> +{
> + if (TARGET_MIPS16)
> + return SPILL_REGS;
> + return NO_REGS;
> +}
> +
> +static bool
> +mips_lra_p (void)
> +{
> + return mips_lra_flag;
> +}
>
> /* Initialize the GCC target structure. */
> #undef TARGET_ASM_ALIGNED_HI_OP
Please use comments of the form:
/* Implement TARGET_FOO. */
above all three functions (instead of the current one in the case of
mips_register_priority), just so that it's painfully obvious that
these are target hooks.
OK for the MIPS part with that change, thanks.
Out of interest, do you see any difference if you include $sp in SPILL_REGS?
That obviously doesn't make much conceptual sense, but it would give a
cleaner class hierarchy.
Richard