[LLVMdev] [ARM backend] SMLAL issue

[James Molloy]

Hi Jyoti,

The first version doesn’t need a SMUL_LOHI, as it’s only doing a 32-bit multiply. The results of that 32-bit multiply are fed into the 64-bit ADD.

The second one is doing a 64-bit multiply. This is expanded to a SMUL_LOHI, and is peepholed by ISelLowering.

It looks simply like whoever wrote this peephole only considered that one version and didn’t consider the more canonical pattern (with a 32-bit multiply).

The place to fix seems to be ARMISelLowering::7949 (AddCombineTo64BitMLAL) - adding a new pattern for that to detect.

Cheers,

James

On 14 May 2015, at 16:36, Jyoti Rajendra Allur <jyoti.allur at samsung.com> wrote:

> 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


-- 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