RE: [Patch MIPS] Enable TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS hook

[Matthew Fortune <Matthew dot Fortune at imgtec dot com>]

Robert Suchanek <Robert.Suchanek@imgtec.com> writes:
> diff --git a/gcc/config/mips/mips.c b/gcc/config/mips/mips.c
> index c3755f5..976f844 100644
> --- a/gcc/config/mips/mips.c
> +++ b/gcc/config/mips/mips.c
> @@ -19415,6 +19415,21 @@ mips_lra_p (void)
>  {
>    return mips_lra_flag;
>  }
> +
> +/* Implement TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS.  */
> +
> +static reg_class_t
> +mips_ira_change_pseudo_allocno_class (int regno, reg_class_t
> allocno_class)
> +{
> +  /* LRA will generate unnecessary reloads because the LRA's cost pass
> finds
> +     cheaper to move data to/from memory into FP regs rather than GP
> regs.
> +     By narrowing the class for allocnos to GR_REGS for integral modes
> early,
> +     we refrain from using FP regs until they are absolutely necessary.
> */

I'm not sure this comment is accurately describing the issue (or I have
misunderstood something). I thought this change is to counter LRA's
tendency to use an FPR as a spill instead of memory?

i.e.
/* LRA will allocate an FPR for an integer mode pseudo instead of spilling
   to memory if an FPR is present in the allocno class.  It is rare that
   we actually need to place an integer mode value in an FPR so where
   possible limit the allocation to GR_REGS.  This will slightly pessimize
   code that involves integer to/from float conversions as these will have
   to reload into FPRs in LRA. Such reloads are sometimes eliminated and
   sometimes only partially eliminated.  We choose to take this penalty
   in order to eliminate usage of FPRs in code that does not use floating
   point data.

   This change has a similar effect to increasing the cost of FPR->GPR
   register moves for integer modes so that they are higher than the cost
   of memory but changing the allocno class is more reliable.

   This is also similar to forbidding integer mode values in FPRs entirely
   but this would lead to an inconsistency in the integer to/from float
   instructions that say integer mode values must be placed in FPRs.  */

I'm keen to get the description of this right so please feel free to change
it further if it isn't clear (or correct).

I don't know if this change will lead to classic reload being unusable for
MIPS. I'm not worried about that but I think it is probably wise to
remove classic reload support for MIPS now; we are dependent on LRA for
several features already.

Do you have any details on when we are left with suboptimal code for
int->float conversions? I'd like to keep a record of them in this thread
or in the comment so we know what is left to fix.

> +  if (INTEGRAL_MODE_P (PSEUDO_REGNO_MODE (regno)) && allocno_class ==
> ALL_REGS)
> +    return GR_REGS;
> +  return allocno_class;
> +}
> +

Trim the extra trailing newline.

OK to commit if you are happy with the comment.

Thanks,
Matthew

> 
> 
>  /* Initialize the GCC target structure.  */
>  #undef TARGET_ASM_ALIGNED_HI_OP
> @@ -19671,6 +19686,8 @@ mips_lra_p (void)
>  #define TARGET_SPILL_CLASS mips_spill_class
>  #undef TARGET_LRA_P
>  #define TARGET_LRA_P mips_lra_p
> +#undef TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS
> +#define TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS
> mips_ira_change_pseudo_allocno_class
> 
>  struct gcc_target targetm = TARGET_INITIALIZER;
>