Regression caused by r218653 - [FastISel][AArch64] Fold sign-/zero-extends into the load instruction.

[Juergen Ributzka]

Great!

Sorry about that stupid bug.

Cheers,
Juergen

> On Oct 9, 2014, at 1:59 AM, Richard Barton <richard.barton at arm.com> wrote:
> 
> Hi Juergen
> 
> That appears to have fixed the regressions we are seeing.
> 
> Thanks for the quick fix!
> 
> Regards,
> Rich
> 
>> -----Original Message-----
>> From: Juergen Ributzka [mailto:juergen at apple.com]
>> Sent: 07 October 2014 04:57
>> To: Richard Barton
>> Cc: LLVM Commits
>> Subject: Re: Regression caused by r218653 - [FastISel][AArch64] Fold
> sign-/zero-
>> extends into the load instruction.
>> 
>> Hi Richard,
>> 
>> I committed a fix in r219185. Please let me know if that worked.
>> 
>> Thanks
>> 
>> Cheers,
>> Juergen
>> 
>>> On Oct 6, 2014, at 8:01 AM, Richard Barton <richard.barton at arm.com>
>> wrote:
>>> 
>>> Hi Juergen
>>> 
>>> This commit introduces a bug for the following (also attached) c-code.
>>> 
>>> The c code:
>>> int a = INT_MIN;
>>> long b = INT_MIN;
>>> if (a != b) {
>>>    printf("FAIL - a = %d, b = %ld (should compare equal)\n", a, b);
>>> }
>>> 
>>> The transformed assembly sequences look like:
>>> ldur    w9, [blah]      ldursw  x8, [blah]
>>> mov     w8, w9         mov     w9, w8
>>> sxtw    x8, w8           mov     w8, x9
>>> ldr     x10 [blah2]     ldr     x10 [blah2]
>>> cmp     x8, x10         cmp     x8, x10
>>> 
>>> In your transformed sequence, the load instruction sign-extends to 64
> bits
>>> in x8, but the subsequent 32-bit mov to w9 disregards the top bits so
> the
>>> subsequent 64-bit value in x8 has lost its signedness.
>>> 
>>> A second example is the c code:
>>> 
>>> signed char a = -99;
>>> unsigned long b = a;
>>> unsigned long c = ULONG_MAX - 98;
>>> if (b != c) {
>>>   printf("Conversion Fails\n");
>>> }
>>> 
>>> The generated assembly sequences are:
>>> ldurb	w9, [x29, #-5]  	ldursb	w9, [x29, #-5]
>>> mov	 w10, w9             mov	 w10, w9
>>> sxtb	x10, w10
>>> str	x10, [sp, #16]     str	x10, [sp, #16]
>>> 
>>> The extending load does not extend up to 64-bits in w9, so the
> subsequent
>>> store of x9 is a positive value.
>>> 
>>> I guess that your patch needs to also take into account that the
> sign-extend
>>> instruction could convert the value from a 32-bit one to a 64-bit one
> and so
>>> your new load instruction must also preserve this to consumers of the
> old
>>> sxt value.
>>> 
>>> Regards
>>> Rich<example1.c><example2.c>
>> 
>> 
> 
> 
> 
>