Patchwork [ARM] Handle simple SImode PLUS and MINUS operations in rtx costs

login
register
mail settings
Submitter Kyrylo Tkachov
Date March 24, 2014, 5:21 p.m.
Message ID <5330697C.3000002@arm.com>
Download mbox | patch
Permalink /patch/333103/
State New
Headers show

Comments

Kyrylo Tkachov - March 24, 2014, 5:21 p.m.
Hi all,

I noticed that we don't handle simple reg-to-reg arithmetic operations in the 
arm rtx cost functions. We should be adding the cost of alu.arith to the costs 
of the operands. This patch does that. Since we don't have any cost tables yet 
that have a non-zero value for that field it shouldn't affect code-gen for any 
current cores.

Bootstrapped and tested on arm-none-linux-gnueabihf.

Ok for next stage1?

Thanks,
Kyrill

2014-03-24  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * config/arm/arm.c (arm_new_rtx_costs): Handle reg-to-reg PLUS
     and MINUS RTXs.
Kyrylo Tkachov - April 2, 2014, 12:55 p.m.
Pinging this for stage1, otherwise I'll forget about it and it'll fall through 
the cracks...

http://gcc.gnu.org/ml/gcc-patches/2014-03/msg01276.html

Thanks,
Kyrill

On 24/03/14 17:21, Kyrill Tkachov wrote:
> Hi all,
>
> I noticed that we don't handle simple reg-to-reg arithmetic operations in the
> arm rtx cost functions. We should be adding the cost of alu.arith to the costs
> of the operands. This patch does that. Since we don't have any cost tables yet
> that have a non-zero value for that field it shouldn't affect code-gen for any
> current cores.
>
> Bootstrapped and tested on arm-none-linux-gnueabihf.
>
> Ok for next stage1?
>
> Thanks,
> Kyrill
>
> 2014-03-24  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>       * config/arm/arm.c (arm_new_rtx_costs): Handle reg-to-reg PLUS
>       and MINUS RTXs.
Kyrylo Tkachov - April 16, 2014, 8:26 a.m.
On 02/04/14 13:55, Kyrill Tkachov wrote:
> Pinging this for stage1, otherwise I'll forget about it and it'll fall through
> the cracks...
>
> http://gcc.gnu.org/ml/gcc-patches/2014-03/msg01276.html
>
> Thanks,
> Kyrill

Ping.
http://gcc.gnu.org/ml/gcc-patches/2014-03/msg01276.html

Thanks,
Kyrill

>
> On 24/03/14 17:21, Kyrill Tkachov wrote:
>> Hi all,
>>
>> I noticed that we don't handle simple reg-to-reg arithmetic operations in the
>> arm rtx cost functions. We should be adding the cost of alu.arith to the costs
>> of the operands. This patch does that. Since we don't have any cost tables yet
>> that have a non-zero value for that field it shouldn't affect code-gen for any
>> current cores.
>>
>> Bootstrapped and tested on arm-none-linux-gnueabihf.
>>
>> Ok for next stage1?
>>
>> Thanks,
>> Kyrill
>>
>> 2014-03-24  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>>        * config/arm/arm.c (arm_new_rtx_costs): Handle reg-to-reg PLUS
>>        and MINUS RTXs.
>
>
Richard Earnshaw - May 2, 2014, 9:58 a.m.
On 24/03/14 17:21, Kyrill Tkachov wrote:
> Hi all,
> 
> I noticed that we don't handle simple reg-to-reg arithmetic operations in the 
> arm rtx cost functions. We should be adding the cost of alu.arith to the costs 
> of the operands. This patch does that. Since we don't have any cost tables yet 
> that have a non-zero value for that field it shouldn't affect code-gen for any 
> current cores.
> 
> Bootstrapped and tested on arm-none-linux-gnueabihf.
> 
> Ok for next stage1?
> 
> Thanks,
> Kyrill
> 
> 2014-03-24  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
>      * config/arm/arm.c (arm_new_rtx_costs): Handle reg-to-reg PLUS
>      and MINUS RTXs.
> 

I think the convention for big switch statements like this is to write

	* config/arm/arm.c (arm_new_rtx_costs, case PLUS, MINUS): ....

However...

> 
> rtx-costs-plus-minus.patch
> 
> 
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index bf5d1b2..3102b67 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -9428,6 +9428,14 @@ arm_new_rtx_costs (rtx x, enum rtx_code code, enum rtx_code outer_code,
>  	      *cost += rtx_cost (XEXP (x, 1), code, 1, speed_p);
>  	      return true;
>  	    }
> +	  else if (REG_P (XEXP (x, 0)))
> +	    {
> +	      *cost += rtx_cost (XEXP (x, 0), code, 0, speed_p)
> +	               + rtx_cost (XEXP (x, 1), code, 1, speed_p);
> +	      if (speed_p)
> +	        *cost += extra_cost->alu.arith;
> +	      return true;
> +	    }
>  

This still doesn't handle the case where the first operand is not a
register.  Furthermore, the only difference between this and what we had
before is that we now add alu.arith to the overall cost.  But we want to
do that anyway.

I think you really just need

	  else if (speed_p)
	    *cost += extra_cost.alu_arith;

and then let recursion (return false) handle the cost of non-register
operands (register will always cost as zero).

>  	  return false;
>  	}
> @@ -9663,6 +9671,14 @@ arm_new_rtx_costs (rtx x, enum rtx_code code, enum rtx_code outer_code,
>  	      *cost += rtx_cost (XEXP (x, 0), PLUS, 0, speed_p);
>  	      return true;
>  	    }
> +	  else if (REG_P (XEXP (x, 1)))
> +	    {
> +	      *cost += rtx_cost (XEXP (x, 0), PLUS, 0, speed_p)
> +	               + rtx_cost (XEXP (x, 1), PLUS, 1, speed_p);
> +	      if (speed_p)
> +	        *cost += extra_cost->alu.arith;
> +	      return true;
> +	    }

The same here.

>  	  return false;
>  	}
>  
> 
R.

Patch

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index bf5d1b2..3102b67 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -9428,6 +9428,14 @@  arm_new_rtx_costs (rtx x, enum rtx_code code, enum rtx_code outer_code,
 	      *cost += rtx_cost (XEXP (x, 1), code, 1, speed_p);
 	      return true;
 	    }
+	  else if (REG_P (XEXP (x, 0)))
+	    {
+	      *cost += rtx_cost (XEXP (x, 0), code, 0, speed_p)
+	               + rtx_cost (XEXP (x, 1), code, 1, speed_p);
+	      if (speed_p)
+	        *cost += extra_cost->alu.arith;
+	      return true;
+	    }
 
 	  return false;
 	}
@@ -9663,6 +9671,14 @@  arm_new_rtx_costs (rtx x, enum rtx_code code, enum rtx_code outer_code,
 	      *cost += rtx_cost (XEXP (x, 0), PLUS, 0, speed_p);
 	      return true;
 	    }
+	  else if (REG_P (XEXP (x, 1)))
+	    {
+	      *cost += rtx_cost (XEXP (x, 0), PLUS, 0, speed_p)
+	               + rtx_cost (XEXP (x, 1), PLUS, 1, speed_p);
+	      if (speed_p)
+	        *cost += extra_cost->alu.arith;
+	      return true;
+	    }
 	  return false;
 	}