This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

RE: [Patch MIPS] Enable TARGET_IRA_CHANGE_PSEUDO_ALLOCNO_CLASS hook


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;
 


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]