Patchwork Small ira.c:setup_pressure_classes tweak

login
register
mail settings
Submitter Richard Sandiford
Date Sept. 7, 2012, 6:59 a.m.
Message ID <87bohi48tv.fsf@talisman.home>
Download mbox | patch
Permalink /patch/182334/
State New
Headers show

Comments

Richard Sandiford - Sept. 7, 2012, 6:59 a.m.
ira.c:setup_pressure_classes treats "leaf" classes as pressure classes
even if moves between them are more expensive than moves to or from memory:

      if (ira_class_hard_regs_num[cl] != 1
	  /* A register class without subclasses may contain a few
	     hard registers and movement between them is costly
	     (e.g. SPARC FPCC registers).  We still should consider it
	     as a candidate for a pressure class.  */
	  && alloc_reg_class_subclasses[cl][0] != LIM_REG_CLASSES)
	{

MIPS accumulators are another case where this inclusion is important.

Everything works as expected with current sources, where:

    MD_REGS = MD0_REG + MD1_REG
    ACC_REGS = MD_REGS + DSP_ACC_REGS

So the three leaf classes are the singleton MD0_REG and MD1_REG (available
on all MIPS targets) and DSP_ACC_REGS (available only when using the DSP ASE).
All three are treated as pressure classes where appropriate.

However, MD0 and MD1 are no longer independently allocatable.  Splitting
what is effectively one register into two classes creates some confusion,
so I'd like to get rid of them.  We're then left with just:

    ACC_REGS = MD_REGS + DSP_ACC_REGS

where MD_REGS and DSP_ACC_REGS are both leaf classes.  Without the DSP ASE,
that reduces to:

    ACC_REGS = MD_REGS

The problem is that alloc_reg_class_subclasses (unlike reg_class_subclasses)
includes higher-numbered classes too.  So when ACC_REGS = MD_REGS,
alloc_reg_class_subclasses lists ACC_REGS as a subset of MD_REGS
and MD_REGS as a subset of ACC_REGS.  The condition:

	  alloc_reg_class_subclasses[cl][0] != LIM_REG_CLASSES

therefore holds for both of them.  This is similar to the situation
mentioned in setup_uniform_class_p:

      /* We can not use alloc_reg_class_subclasses here because move
	 cost hooks does not take into account that some registers are
	 unavailable for the subtarget.  E.g. for i686, INT_SSE_REGS
	 is element of alloc_reg_class_subclasses for GENERAL_REGS
	 because SSE regs are unavailable.  */
      for (i = 0; (cl2 = reg_class_subclasses[cl][i]) != LIM_REG_CLASSES; i++)

When we have several equivalent leaf classes, I think we should continue
to treat the lowest-numbered one as a pressure class.  This produces the
expected results with the MIPS patch described above.

Tested on x86_64-linux-gnu and mipsisa64-elf.  Also tested by making sure
that there were no changed in cc1 .ii files for x86_64-linux-gnu.
OK to install?


gcc/
	* ira.c (setup_pressure_classes): Handle synonymous classes.
Vladimir Makarov - Sept. 7, 2012, 8:31 p.m.
On 09/07/2012 02:59 AM, Richard Sandiford wrote:
> ira.c:setup_pressure_classes treats "leaf" classes as pressure classes
> even if moves between them are more expensive than moves to or from memory:
>
>        if (ira_class_hard_regs_num[cl] != 1
> 	  /* A register class without subclasses may contain a few
> 	     hard registers and movement between them is costly
> 	     (e.g. SPARC FPCC registers).  We still should consider it
> 	     as a candidate for a pressure class.  */
> 	  && alloc_reg_class_subclasses[cl][0] != LIM_REG_CLASSES)
> 	{
>
> MIPS accumulators are another case where this inclusion is important.
>
> Everything works as expected with current sources, where:
>
>      MD_REGS = MD0_REG + MD1_REG
>      ACC_REGS = MD_REGS + DSP_ACC_REGS
>
> So the three leaf classes are the singleton MD0_REG and MD1_REG (available
> on all MIPS targets) and DSP_ACC_REGS (available only when using the DSP ASE).
> All three are treated as pressure classes where appropriate.
>
> However, MD0 and MD1 are no longer independently allocatable.  Splitting
> what is effectively one register into two classes creates some confusion,
> so I'd like to get rid of them.  We're then left with just:
>
>      ACC_REGS = MD_REGS + DSP_ACC_REGS
>
> where MD_REGS and DSP_ACC_REGS are both leaf classes.  Without the DSP ASE,
> that reduces to:
>
>      ACC_REGS = MD_REGS
>
> The problem is that alloc_reg_class_subclasses (unlike reg_class_subclasses)
> includes higher-numbered classes too.  So when ACC_REGS = MD_REGS,
> alloc_reg_class_subclasses lists ACC_REGS as a subset of MD_REGS
> and MD_REGS as a subset of ACC_REGS.  The condition:
>
> 	  alloc_reg_class_subclasses[cl][0] != LIM_REG_CLASSES
>
> therefore holds for both of them.  This is similar to the situation
> mentioned in setup_uniform_class_p:
>
>        /* We can not use alloc_reg_class_subclasses here because move
> 	 cost hooks does not take into account that some registers are
> 	 unavailable for the subtarget.  E.g. for i686, INT_SSE_REGS
> 	 is element of alloc_reg_class_subclasses for GENERAL_REGS
> 	 because SSE regs are unavailable.  */
>        for (i = 0; (cl2 = reg_class_subclasses[cl][i]) != LIM_REG_CLASSES; i++)
>
> When we have several equivalent leaf classes, I think we should continue
> to treat the lowest-numbered one as a pressure class.  This produces the
> expected results with the MIPS patch described above.
>
> Tested on x86_64-linux-gnu and mipsisa64-elf.  Also tested by making sure
> that there were no changed in cc1 .ii files for x86_64-linux-gnu.
> OK to install?
>
Ok.  It looks reasonable although I am not sure that it will work for 
all situations.  That is a really tricky code.  It is hard to say what 
the final outcome will be in all situations.  But in what I am sure is 
that the change is safe.

Thanks for the patch, Richard.

Patch

Index: gcc/ira.c
===================================================================
--- gcc/ira.c	2012-09-05 21:33:00.115078796 +0100
+++ gcc/ira.c	2012-09-05 22:01:05.747029786 +0100
@@ -789,7 +789,7 @@  setup_pressure_classes (void)
 	     hard registers and movement between them is costly
 	     (e.g. SPARC FPCC registers).  We still should consider it
 	     as a candidate for a pressure class.  */
-	  && alloc_reg_class_subclasses[cl][0] != LIM_REG_CLASSES)
+	  && alloc_reg_class_subclasses[cl][0] < cl)
 	{
 	  /* Check that the moves between any hard registers of the
 	     current class are not more expensive for a legal mode