diff mbox

ARM patch: Fix an if statement in arm_rtx_costs_1

Message ID 4C4F0479.4090500@codesourcery.com
State New
Headers show

Commit Message

Bernd Schmidt July 27, 2010, 4:08 p.m. UTC 
In the ARM rtx_costs code, there's an if statement which consists of a
logical or of two conditions, the second of which always implies the
first.  This is nonsensical.  The if statement is intended to increase
the cost of operations involving frame pointer or stack pointer, except
for those where the second operand is constant.  This exception is
broken by the presumably incorrect code, and even (plus (sp) (const)) is
given a high cost.  This prevents some transformations in the new
postreload optimization I added a while ago.  Interestingly it's a
really old bug, the code was added in 1994.

The following patch fixes it; typical effects from postreload:
-       add     r4, sp, #32
-       subs    r6, r4, #4
+       add     r6, sp, #28

It also seems to be beneficial in other ways; I've seen stack frames
shrink for some functions.  Ok if it tests cleanly?


Bernd
* config/arm/arm.c (arm_rtx_costs_1): Remove second clause from the
	if statement which adds extra costs to frame-related expressions.

Comments

Richard Earnshaw July 31, 2010, 1:03 p.m. UTC | #1 
On Tue, 2010-07-27 at 18:08 +0200, Bernd Schmidt wrote:
> In the ARM rtx_costs code, there's an if statement which consists of a
> logical or of two conditions, the second of which always implies the
> first.  This is nonsensical.  The if statement is intended to increase
> the cost of operations involving frame pointer or stack pointer, except
> for those where the second operand is constant.  This exception is
> broken by the presumably incorrect code, and even (plus (sp) (const)) is
> given a high cost.  This prevents some transformations in the new
> postreload optimization I added a while ago.  Interestingly it's a
> really old bug, the code was added in 1994.
> 
> The following patch fixes it; typical effects from postreload:
> -       add     r4, sp, #32
> -       subs    r6, r4, #4
> +       add     r6, sp, #28
> 
> It also seems to be beneficial in other ways; I've seen stack frames
> shrink for some functions.  Ok if it tests cleanly?
> 

OK.

R.
diff mbox

Patch

Index: config/arm/arm.c
===================================================================
--- config/arm/arm.c	(revision 162421)
+++ config/arm/arm.c	(working copy)
@@ -6622,12 +6610,10 @@  arm_rtx_costs_1 (rtx x, enum rtx_code ou
 	 since then they might not be moved outside of loops.  As a compromise
 	 we allow integration with ops that have a constant as their second
 	 operand.  */
-      if ((REG_OR_SUBREG_REG (XEXP (x, 0))
-	   && ARM_FRAME_RTX (REG_OR_SUBREG_RTX (XEXP (x, 0)))
-	   && GET_CODE (XEXP (x, 1)) != CONST_INT)
-	  || (REG_OR_SUBREG_REG (XEXP (x, 0))
-	      && ARM_FRAME_RTX (REG_OR_SUBREG_RTX (XEXP (x, 0)))))
-	*total = 4;
+      if (REG_OR_SUBREG_REG (XEXP (x, 0))
+	  && ARM_FRAME_RTX (REG_OR_SUBREG_RTX (XEXP (x, 0)))
+	  && GET_CODE (XEXP (x, 1)) != CONST_INT)
+	*total = COSTS_N_INSNS (1);
 
       if (mode == DImode)
 	{