Re: [Qemu-devel] [PATCH 4/6] target-arm: fix saturated values for Neon right shifts.

[Peter Maydell]

On 11 February 2011 15:11,  <address@hidden> wrote:

> --- a/target-arm/neon_helper.c
> +++ b/target-arm/neon_helper.c
> @@ -903,7 +903,7 @@ uint64_t HELPER(neon_qrshl_u64)(CPUState *env, uint64_t 
> val, uint64_t shiftop)
>         dest = src1 << tmp; \
>         if ((dest >> tmp) != src1) { \
>             SET_QC(); \
> -            dest = src1 >> 31; \
> +            dest = (uint32_t)(1 << (sizeof(src1) * 8 - 1)) - (src1 > 0 ? 1 : 
> 0); \
>         } \
>     }} while (0)

This gives the right values but is a pretty long expression (among
other things it is more than 80 chars and breaks the QEMU coding
style). I'd rather do the same as the existing qshl_s* helper:

            dest = (uint32_t)(1 << (sizeof(src1) * 8 - 1)); \
            if (src1 > 0) { \
                dest--; \
            } \

>  NEON_VOP_ENV(qrshl_s8, neon_s8, 4)
> @@ -924,7 +924,11 @@ uint32_t HELPER(neon_qrshl_s32)(CPUState *env, uint32_t 
> valop, uint32_t shiftop)
>         dest = val << shift;
>         if ((dest >> shift) != val) {
>             SET_QC();
> -            dest = (uint32_t)(1 << (sizeof(val) * 8 - 1)) - (val > 0 ? 1 : 
> 0);
> +            if (val < 0) {
> +                dest = INT32_MIN;
> +            } else {
> +                dest = INT32_MAX;
> +            }

Again, right answers but the way most of the rest of the code
forces a 32 bit value to signed saturation is
   dest = (val >> 31) ^ ~SIGNBIT;

-- PMM