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

[Aurelien Jarno]

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


-- 
  .''`.  Aurelien Jarno             | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   address@hidden         | address@hidden
   `-    people.debian.org/~aurel32 | www.aurel32.net