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

[Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>]

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.

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.  */
 

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