Re: [Qemu-devel] [PATCH 2/6] target-arm: fix Neon right shifts with shift amount == input width.

[Peter Maydell]

On 11 February 2011 15:10,  <address@hidden> wrote:
> From: Christophe Lyon <address@hidden>
>
> Fix rshl helpers (s8, s16, s64, u8, u16)
>
> Signed-off-by: Christophe Lyon <address@hidden>
> ---
>  target-arm/neon_helper.c |    6 +++---
>  1 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/target-arm/neon_helper.c b/target-arm/neon_helper.c
> index 3f1f3d4..1ac362f 100644
> --- a/target-arm/neon_helper.c
> +++ b/target-arm/neon_helper.c
> @@ -548,7 +548,7 @@ uint64_t HELPER(neon_shl_s64)(uint64_t valop, uint64_t 
> shiftop)
>     } else if (tmp < -(ssize_t)sizeof(src1) * 8) { \
>         dest = src1 >> (sizeof(src1) * 8 - 1); \
>     } else if (tmp == -(ssize_t)sizeof(src1) * 8) { \
> -        dest = src1 >> (tmp - 1); \
> +        dest = src1 >> (-tmp - 1); \
>         dest++; \
>         dest >>= 1; \

Again, these three lines have the same effect as dest = 0,
so we can fold into the previous if().

>     } else if (tmp < 0) { \
> @@ -594,7 +594,7 @@ uint64_t HELPER(neon_rshl_s64)(uint64_t valop, uint64_t 
> shiftop)
>         val = 0;
>     } else if (shift < -64) {
>         val >>= 63;

You didn't change this case, but it is the wrong answer:
should be 0.

> -    } else if (shift == -63) {
> +    } else if (shift == -64) {
>         val >>= 63;
>         val++;
>         val >>= 1;

Always results in 0.

-- PMM