[PATCH][ARM][2/4] Fix operand costing logic for SMUL[TB][TB]

[PATCH][ARM][2/4] Fix operand costing logic for SMUL[TB][TB]

[Kyrill Tkachov]

[-- Attachment #1: Type: text/plain, Size: 741 bytes --]

Hi all,

As part of investigating the codegen effects of a fix for PR 65932 I found we assign
too high a cost for the sign-extending multiply instruction SMULBB.
This is because we add the cost of a multiply-extend but then also recurse into the
SIGN_EXTEND sub-expressions rather than the registers (or subregs) being sign-extended.

This patch is a simple fix. The fix is right by itself, but in combination with patch 3
fix the gcc.target/arm/wmul-2.c testcase.

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

Ok for trunk?

Thanks,
Kyrill

2016-01-22  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     * config/arm/arm.c (arm_new_rtx_costs, MULT case): Properly extract
     the operands of the SIGN_EXTENDs from a SMUL[TB][TB] rtx.

[-- Attachment #2: arm-smulbb-costs.patch --]
[-- Type: text/x-patch, Size: 760 bytes --]

diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 17f00b5a1de21de35366b82040a7ad46d65f899e..ee3ebe4561ea3d9791fabdfcec8b16af63bd4d20 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -10321,8 +10321,10 @@ arm_new_rtx_costs (rtx x, enum rtx_code code, enum rtx_code outer_code,
 	      /* SMUL[TB][TB].  */
 	      if (speed_p)
 		*cost += extra_cost->mult[0].extend;
-	      *cost += rtx_cost (XEXP (x, 0), mode, SIGN_EXTEND, 0, speed_p);
-	      *cost += rtx_cost (XEXP (x, 1), mode, SIGN_EXTEND, 1, speed_p);
+	      *cost += rtx_cost (XEXP (XEXP (x, 0), 0), mode,
+				 SIGN_EXTEND, 0, speed_p);
+	      *cost += rtx_cost (XEXP (XEXP (x, 1), 0), mode,
+				 SIGN_EXTEND, 1, speed_p);
 	      return true;
 	    }
 	  if (speed_p)

Re: [PATCH][ARM][2/4] Fix operand costing logic for SMUL[TB][TB]

[Ramana Radhakrishnan]

On Fri, Jan 22, 2016 at 9:52 AM, Kyrill Tkachov
<kyrylo.tkachov@foss.arm.com> wrote:
> Hi all,
>
> As part of investigating the codegen effects of a fix for PR 65932 I found
> we assign
> too high a cost for the sign-extending multiply instruction SMULBB.
> This is because we add the cost of a multiply-extend but then also recurse
> into the
> SIGN_EXTEND sub-expressions rather than the registers (or subregs) being
> sign-extended.
>
> This patch is a simple fix. The fix is right by itself, but in combination
> with patch 3
> fix the gcc.target/arm/wmul-2.c testcase.
>
> Bootstrapped and tested on arm-none-linux-gnueabihf.
>
> Ok for trunk?
>

OK.

Thanks,
Ramana
> Thanks,
> Kyrill
>
> 2016-01-22  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     * config/arm/arm.c (arm_new_rtx_costs, MULT case): Properly extract
>     the operands of the SIGN_EXTENDs from a SMUL[TB][TB] rtx.

Re: [PATCH][ARM][2/4] Fix operand costing logic for SMUL[TB][TB]

[Kyrill Tkachov]

On 04/02/16 09:00, Ramana Radhakrishnan wrote:
> On Fri, Jan 22, 2016 at 9:52 AM, Kyrill Tkachov
> <kyrylo.tkachov@foss.arm.com> wrote:
>> Hi all,
>>
>> As part of investigating the codegen effects of a fix for PR 65932 I found
>> we assign
>> too high a cost for the sign-extending multiply instruction SMULBB.
>> This is because we add the cost of a multiply-extend but then also recurse
>> into the
>> SIGN_EXTEND sub-expressions rather than the registers (or subregs) being
>> sign-extended.
>>
>> This patch is a simple fix. The fix is right by itself, but in combination
>> with patch 3
>> fix the gcc.target/arm/wmul-2.c testcase.
>>
>> Bootstrapped and tested on arm-none-linux-gnueabihf.
>>
>> Ok for trunk?
>>
> OK.

Is it ok to backport this to the GCC 5 branch?
It fixes a testcase with cortex-a5 tuning and was tested by Christophe:
https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01238.html

Thanks,
Kyrill

> Thanks,
> Ramana
>> Thanks,
>> Kyrill
>>
>> 2016-01-22  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>>
>>      * config/arm/arm.c (arm_new_rtx_costs, MULT case): Properly extract
>>      the operands of the SIGN_EXTENDs from a SMUL[TB][TB] rtx.

Re: [PATCH][ARM][2/4] Fix operand costing logic for SMUL[TB][TB]

[Nick Clifton]

Hi Kyrill,

> 2016-01-22  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
> 
>     * config/arm/arm.c (arm_new_rtx_costs, MULT case): Properly extract
>     the operands of the SIGN_EXTENDs from a SMUL[TB][TB] rtx.

Approved - please apply.

Cheers
  Nick