diff mbox

[SH] Fix target/48596

Message ID 20120301.131149.253359368.kkojima@rr.iij4u.or.jp
State New
Headers show

Commit Message

Kaz Kojima March 1, 2012, 4:11 a.m. UTC
Hi,

The attached patch is to avoid PR target/48596 which is a 4.7
regression on SH.  It seems that now IRA aggressively tried to
use fp regs as the holder for memory addresses on this port.
SH requires a special fpul register to move from the fp regs to
the general regs which are legitimate for pointers and, in
the problematic situation, fpul is already reserved for reload
and the spill failure resulted for the reg equiv processing in RA.
I guess that no other targets have such restrictions.  Perhaps
if there eas a direct way to notify IRA that some register classes
will be too costly for the addresses, SH port will utilize it,
though it looks to be invasive.

The patch tries to work around the problem with increasing the move
cost between fp and general registers for SImode.  The usual tests
are done successfully on sh4-unknown-linux-gnu with no new failures.
A bit surprisingly, there are no size/performance regressions and
a few code size improvements with CSiBE tests I've done.
I'd like to hear the suggestions from the experts before applying
this work around.

Regards,
	kaz
--
	PR target/48596
	* config/sh/sh.c (sh_register_move_cost): Increase cost between
	GENERAL_REGS and FP_REGS for SImode.

Comments

Mike Stump March 1, 2012, 6:37 p.m. UTC | #1
On Feb 29, 2012, at 8:11 PM, Kaz Kojima wrote:
> The attached patch is to avoid PR target/48596 which is a 4.7
> regression on SH.  It seems that now IRA aggressively tried to
> use fp regs as the holder for memory addresses on this port.

Gosh, I hope IRA is cleaned up, as other ports do have weird register restrictions...  that don't want to get hit by the new IRA code.  [ crosses fingers that when I update to 4.7, I won't hit this ]
Oleg Endo March 1, 2012, 9:58 p.m. UTC | #2
On Thu, 2012-03-01 at 13:11 +0900, Kaz Kojima wrote:
> Hi,
> 
> The attached patch is to avoid PR target/48596 which is a 4.7
> regression on SH.  It seems that now IRA aggressively tried to
> use fp regs as the holder for memory addresses on this port.
> SH requires a special fpul register to move from the fp regs to
> the general regs which are legitimate for pointers and, in
> the problematic situation, fpul is already reserved for reload
> and the spill failure resulted for the reg equiv processing in RA.
> I guess that no other targets have such restrictions.  Perhaps
> if there eas a direct way to notify IRA that some register classes
> will be too costly for the addresses, SH port will utilize it,
> though it looks to be invasive.
> 
> The patch tries to work around the problem with increasing the move
> cost between fp and general registers for SImode.  The usual tests
> are done successfully on sh4-unknown-linux-gnu with no new failures.
> A bit surprisingly, there are no size/performance regressions and
> a few code size improvements with CSiBE tests I've done.
> I'd like to hear the suggestions from the experts before applying
> this work around.
> 

I was afraid that with this patch, in the following use case ...

int32_t float_as_int (float val)
{
  union { float f; int32_t i; } u;
  u.f = val;
  return u.i;
}

... the value would somehow get pushed/popped through the stack.
Luckily this is not the case, it still goes through the fpul register :)

__Z12float_as_intf:
	flds	fr5,fpul
	rts	
	sts	fpul,r0

(I've got a similar use case where it does go through the stack, but it
also happens without the patch...)

BTW, the patch also seems to fix PR 48806.

Cheers,
Oleg
Kaz Kojima March 1, 2012, 11:05 p.m. UTC | #3
Oleg Endo <oleg.endo@t-online.de> wrote:
> BTW, the patch also seems to fix PR 48806.

Ah, I've confirmed it.  Thanks for pointing it out.

Regards,
	kaz
diff mbox

Patch

--- ORIG/trunk/gcc/config/sh/sh.c	2012-02-23 21:24:18.000000000 +0900
+++ trunk/gcc/config/sh/sh.c	2012-03-01 09:41:00.000000000 +0900
@@ -11497,8 +11498,15 @@  sh_register_move_cost (enum machine_mode
        && REGCLASS_HAS_GENERAL_REG (srcclass))
       || (REGCLASS_HAS_GENERAL_REG (dstclass)
 	  && REGCLASS_HAS_FP_REG (srcclass)))
-    return (((TARGET_SHMEDIA ? 4 : TARGET_FMOVD ? 8 : 12) + 64)
-	    * ((GET_MODE_SIZE (mode) + 7) / 8U));
+    {
+      /* Discourage trying to use fp regs for a pointer.  This also
+	 discourages fp regs with SImode because Pmode is an alias
+	 of SImode on this target.  See PR target/48596.  */
+      int addend = (mode == Pmode) ? 40 : 0;
+
+      return (((TARGET_SHMEDIA ? 4 : TARGET_FMOVD ? 8 : 12) + addend)
+	      * ((GET_MODE_SIZE (mode) + 7) / 8U));
+    }
 
   if ((dstclass == FPUL_REGS
        && REGCLASS_HAS_GENERAL_REG (srcclass))