llvm dev - May 2015 - [LLVMdev] Fwd: [ARM backend] SMLAL issue

Hi James,
Patch for this is almost ready, I am now considering some cases around SMLALBB.

I came across one the gcc test cases and was a but puzzeled and hoped you would
clarify it.

/* { dg-do compile } */
/* { dg-options "-O2" } */
/* { dg-require-effective-target arm_dsp } */

long long
foo (long long a, char *b, char *c)
{
  return a + *b * *c;
}

/* { dg-final { scan-assembler "umlal" } } */

For this test case, why is gcc toolchain expecting UMLAL instead of SMLAL,
because the multiplication is being done on signed char values?

Regards,
Jyoti Allur

------- Original Message -------
Sender : Jyoti Rajendra Allur<jyoti.allur at samsung.com>  Technical
Manager/SRI-Bangalore-Tizen Core Platform/Samsung Electronics
Date   : May 14, 2015 21:06 (GMT+05:30)
Title  : [ARM backend] SMLAL issue

Hi Tim/James,
In the following code, MUL, ADDS, ADC are combined to SMLAL only if 'b'
and 'c' are promoted to long long type during multiplication.
Shouldn't they be allowed to be combined to SMLAL
even in the code as is without promoting b and c to long long type?

I found the reason for not combining to SMAL to be this, ADDS operand 0 has
opcode not set as ISD::SMUL_LOHI even though operand 0 ie., R2 is a result of
SMULBB as seen in the assembly.

long long
foo (long long a, int b, int c)
{
       return a + b * c;    ===> return a + (long long)b * (long long)c;

}
ldrb    r2, [r2]
ldrb    r3, [r3]
smulbb  r2, r3, r2
adds    r0, r2, r0
adc     r1, r1, #0
bx      lr

I have already checked in combine function in DAG combiner and visitADDC, ADDS
node operand0 's opcode is not set to ISD::SMUL_LOHI. Only when b and c are
typecasted to long long, operand0 opcode is set to  ISD::SMUL_LOHI
Is this a likely bug, if not is there a specific reason for designing it this
way ?


Regards,
Jyoti Allur<p>&nbsp;</p><p>&nbsp;</p>

Hi Jyoti,
>From memory, char is unsigned on ARM platforms. So the multiplication needs
to be done unsigned and the add doesn’t care, so UMLAL would be right.
Cheers,

James

On 19 May 2015, at 06:21, Jyoti Rajendra Allur <jyoti.allur at
samsung.com> wrote:
> Hi James,
> Patch for this is almost ready, I am now considering some cases around
SMLALBB.
>
> I came across one the gcc test cases and was a but puzzeled and hoped you
would clarify it.
>
> /* { dg-do compile } */
> /* { dg-options "-O2" } */
> /* { dg-require-effective-target arm_dsp } */
>
> long long
> foo (long long a, char *b, char *c)
> {
>  return a + *b * *c;
> }
>
> /* { dg-final { scan-assembler "umlal" } } */
>
> For this test case, why is gcc toolchain expecting UMLAL instead of SMLAL,
because the multiplication is being done on signed char values?
>
> Regards,
> Jyoti Allur
>
> ------- Original Message -------
> Sender : Jyoti Rajendra Allur<jyoti.allur at samsung.com>  Technical
Manager/SRI-Bangalore-Tizen Core Platform/Samsung Electronics
> Date   : May 14, 2015 21:06 (GMT+05:30)
> Title  : [ARM backend] SMLAL issue
>
> Hi Tim/James,
> In the following code, MUL, ADDS, ADC are combined to SMLAL only if
'b' and 'c' are promoted to long long type during
multiplication. Shouldn't they be allowed to be combined to SMLAL
> even in the code as is without promoting b and c to long long type?
>
> I found the reason for not combining to SMAL to be this, ADDS operand 0 has
opcode not set as ISD::SMUL_LOHI even though operand 0 ie., R2 is a result of
SMULBB as seen in the assembly.
>
> long long
> foo (long long a, int b, int c)
> {
>       return a + b * c;    ===> return a + (long long)b * (long long)c;
>
> }
> ldrb    r2, [r2]
> ldrb    r3, [r3]
> smulbb  r2, r3, r2
> adds    r0, r2, r0
> adc     r1, r1, #0
> bx      lr
>
> I have already checked in combine function in DAG combiner and visitADDC,
ADDS node operand0 's opcode is not set to ISD::SMUL_LOHI. Only when b and c
are typecasted to long long, operand0 opcode is set to  ISD::SMUL_LOHI
> Is this a likely bug, if not is there a specific reason for designing it
this way ?
>
>
> Regards,
> Jyoti Allur<p>&nbsp;</p><p>&nbsp;</p>

-- IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium.  Thank you.

ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered
in England & Wales, Company No:  2557590
ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ,
Registered in England & Wales, Company No:  2548782