This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
RE: [Patch MIPS] Enable TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS hook
- From: Robert Suchanek <Robert dot Suchanek at imgtec dot com>
- To: Matthew Fortune <Matthew dot Fortune at imgtec dot com>, "Catherine_Moore at mentor dot com" <Catherine_Moore at mentor dot com>
- Cc: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 28 May 2015 13:10:46 +0000
- Subject: RE: [Patch MIPS] Enable TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS hook
- Authentication-results: sourceware.org; auth=none
- References: <B5E67142681B53468FAF6B7C3135656244159813 at hhmail02 dot hh dot imgtec dot org> <6D39441BF12EF246A7ABCE6654B0235321062CEC at LEMAIL01 dot le dot imgtec dot org>
Hi Matthew,
> > +
> > +/* Implement TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS. */
> > +
> > +static reg_class_t
> > +mips_ira_change_pseudo_allocno_class (int regno, reg_class_t
> > +allocno_class) {
> > + if (FLOAT_MODE_P (PSEUDO_REGNO_MODE (regno)) || allocno_class !=
> > ALL_REGS)
> > + return allocno_class;
> > + return GR_REGS;
> > +}
> > +
>
> I'm concerned that this may not be the right condition but either way,
> I think it is better to switch this around to have the special case
> as the conditional. I found it difficult to understand what it is
> doing even when I know the intent :-) A comment about the purpose seems
> appropriate too here as it won't be obvious to someone new.
I tried to write a sensible comment and found the original change hard
to describe. I changed the condition to the special case and did some
experiments. The patch below is now more concise, better fits the purpose and it
seems to have marginally better allocation too.
> Aren't there some fixed point modes that should go in FPRs too? I guess
> paired single (v2sf) doesn't need mentioning as it would never be
> allowed in GR_REGS so pseudos of that mode would never get ALL_REGS,
> is that correct? I.e. will we only see ALL_REGS if a particular
> pseudo/mode truly can be placed in any register according to the
> hard_regno_ok rules?
I think that with the patch below all concerns would be addressed since
the class narrowing would be constrained to integers rather than anything else.
Regards,
Robert
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. */
+ if (INTEGRAL_MODE_P (PSEUDO_REGNO_MODE (regno)) && allocno_class == ALL_REGS)
+ return GR_REGS;
+ return allocno_class;
+}
+
/* 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;