diff mbox

[ARM] Adjust costs so udiv is preferred over sdiv when both are valid. [Patch (2/2)]

Message ID 5937CF90.9040506@foss.arm.com
State New
Headers show

Commit Message

Kyrill Tkachov June 7, 2017, 10:04 a.m. UTC 
On 02/05/17 16:37, Tamar Christina wrote:
> Hi All,
>
> This patch adjusts the cost model so that when both sdiv and udiv are possible
> it prefers udiv over sdiv. This was done by making sdiv slightly more expensive
> instead of making udiv cheaper to keep the baseline costs of a division the same
> as before.
>
> Similar to aarch64 this patch along with my other two related mid-end changes
> makes a big difference in division by constants.
>
> Given:
>
> int f2(int x)
> {
>   return ((x * x) % 300) + ((x * x) / 300);
> }
>
> we now generate
>
> f2:
>         mul     r3, r0, r0
>         mov     r0, r3
>         ldr     r1, .L3
>         umull   r2, r3, r0, r1
>         lsr     r2, r3, #5
>         add     r3, r2, r2, lsl #2
>         rsb     r3, r3, r3, lsl #4
>         sub     r0, r0, r3, lsl #2
>         add     r0, r0, r2
>         bx      lr
>
> as opposed to
>
> f2:
>         mul     r3, r0, r0
>         mov     r0, r3
>         ldr     r3, .L4
>         push    {r4, r5}
>         smull   r4, r5, r0, r3
>         asr     r3, r0, #31
>         rsb     r3, r3, r5, asr #5
>         add     r2, r3, r3, lsl #2
>         rsb     r2, r2, r2, lsl #4
>         sub     r0, r0, r2, lsl #2
>         add     r0, r0, r3
>         pop     {r4, r5}
>         bx      lr
>
> Bootstrapped and reg tested on arm-none-eabi
> with no regressions.
>
> OK for trunk?
>
> Thanks,
> Tamar
>
>
> gcc/
> 2017-05-02  Tamar Christina  <tamar.christina@arm.com>
>
>         * config/arm/arm.c (arm_rtx_costs_internal): Make sdiv more expensive than udiv.
>
>
> gcc/testsuite/
> 2017-05-02  Tamar Christina  <tamar.christina@arm.com>
>
>         * gcc.target/arm/sdiv_costs_1.c: New.

We usually try to avoid adjusting the costs in units other than COSTS_N_INSNS.
Would adding COSTS_N_INSNS (1) here work?
If so, could you also add a comment here to describe why we're adjusting the cost.

      case MOD:
@@ -9280,7 +9282,7 @@ arm_rtx_costs_internal (rtx x, enum rtx_code code, enum rtx_code outer_code,
  
      /* Fall-through.  */
      case UMOD:
-      *cost = LIBCALL_COST (2);
+      *cost = LIBCALL_COST (2) + (code == MOD ? 1 : 0);

Same here.

Thanks,
Kyrill

Comments

Tamar Christina June 7, 2017, 10:15 a.m. UTC | #1 
Hi Kyrill,

> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index
> b24143e32e2f100000f3b150f7ed0df4fabb3cc8..ecc7688b1db6309a4dd694a8e
> 254e64abe14d7e3 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -9258,6 +9258,8 @@ arm_rtx_costs_internal (rtx x, enum rtx_code
> code, enum rtx_code outer_code,
>   	*cost += COSTS_N_INSNS (speed_p ? extra_cost->mult[0].idiv : 0);
>         else
>   	*cost = LIBCALL_COST (2);
> +
> +      *cost += (code == DIV ? 1 : 0);
>         return false;	/* All arguments must be in registers.  */
> 
> 
> We usually try to avoid adjusting the costs in units other than
> COSTS_N_INSNS.
> Would adding COSTS_N_INSNS (1) here work?
> If so, could you also add a comment here to describe why we're adjusting the
> cost.

It would, I'm just slightly worried it might end up generating different code for DIV then.
The reason I have used a unit smaller than COSTS_N_INSNS it so that it should have any real
Impact on any other optimization as the cost is likely treated as an integer? It's only for things that
Compare the costs values between signed and unsigned would the small unit make a difference.

Since I think the compiler still has some hard coded cost limits somewhere it may be an issue, but I'm not
100% certain. I can make the change though.

> 
>       case MOD:
> @@ -9280,7 +9282,7 @@ arm_rtx_costs_internal (rtx x, enum rtx_code
> code, enum rtx_code outer_code,
> 
>       /* Fall-through.  */
>       case UMOD:
> -      *cost = LIBCALL_COST (2);
> +      *cost = LIBCALL_COST (2) + (code == MOD ? 1 : 0);
> 
> Same here.
> 
> Thanks,
> Kyrill
>
diff mbox

Patch

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index b24143e32e2f100000f3b150f7ed0df4fabb3cc8..ecc7688b1db6309a4dd694a8e254e64abe14d7e3 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -9258,6 +9258,8 @@  arm_rtx_costs_internal (rtx x, enum rtx_code code, enum rtx_code outer_code,
  	*cost += COSTS_N_INSNS (speed_p ? extra_cost->mult[0].idiv : 0);
        else
  	*cost = LIBCALL_COST (2);
+
+      *cost += (code == DIV ? 1 : 0);
        return false;	/* All arguments must be in registers.  */