Re: [Qemu-devel] [ARM] Problem in a NEON instruction

["Laurent Desnogues" <laurent.desnogues@gmail.com>]

On Tue, Oct 14, 2008 at 10:23 AM, Aurelien Jarno <aurelien@aurel32.net> wrote:
> On Tue, Jun 10, 2008 at 12:32:17PM +0200, Laurent Desnogues wrote:
>> The rshl instruction is faulty.  Note I did not check the correctness
>> of that instruction in general, I only made a change that looked logical.
>>
>>
>> Laurent
>
>> --- svn-ref/target-arm/neon_helper.c  2008-06-09 08:52:48.000000000 +0200
>> +++ svn/target-arm/neon_helper.c      2008-06-10 12:27:38.000000000 +0200
>> @@ -456,7 +456,7 @@
>>      if (tmp >= sizeof(src1) * 8) { \
>>          dest = 0; \
>>      } else if (tmp < -sizeof(src1) * 8) { \
>> -        dest >>= sizeof(src1) * 8 - 1; \
>> +        dest = src1 >> (sizeof(src1) * 8 - 1); \
>>      } else if (tmp == -sizeof(src1) * 8) { \
>>          dest = src1 >> (tmp - 1); \
>>          dest++; \
>
> Your patch is indeed correct. It seems there is another mistake (at
> least compared to the 64-bit version) in this instruction, please find
> an updated patch below.
>
> diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
> index 4ee5658..35fbaf5 100644
> --- a/target-arm/neon_helper.c
> +++ b/target-arm/neon_helper.c
> @@ -456,11 +456,11 @@ uint64_t HELPER(neon_shl_s64)(uint64_t valop, uint64_t shiftop)
>     if (tmp >= sizeof(src1) * 8) { \
>         dest = 0; \
>     } else if (tmp < -sizeof(src1) * 8) { \
> -        dest >>= sizeof(src1) * 8 - 1; \
> +        dest = src1 >> (sizeof(src1) * 8 - 1); \
>     } else if (tmp == -sizeof(src1) * 8) { \
>         dest = src1 >> (tmp - 1); \
>         dest++; \
> -        src2 >>= 1; \
> +        dest >>= 1; \
>     } else if (tmp < 0) { \
>         dest = (src1 + (1 << (-1 - tmp))) >> -tmp; \
>     } else { \

Can someone consider that patch especially now that gcc 4
displays some well deserved warnings about the code :-)


Laurent