qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 3/3] target-arm: fix support for vrsqrte.


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH 3/3] target-arm: fix support for vrsqrte.
Date: Thu, 17 Feb 2011 12:49:52 +0000

On 16 February 2011 17:59,  <address@hidden> wrote:
> From: Christophe Lyon <address@hidden>
>
> Now use the same algorithm as described in the ARM ARM.

This doesn't pass random testing.

> +static float64 recip_sqrt_estimate(float64 a, CPUState *env)
> +{
> +    float_status *s = &env->vfp.standard_fp_status;
> +    float64 one = int64_to_float64(1, s);
> +    float64 half = float64_div(one, int64_to_float64(2, s), s);

Same remarks about one and half apply as for 2/3.
Maybe we should put a float64_half in softfloat.h.

>  float32 HELPER(rsqrte_f32)(float32 a, CPUState *env)
>  {
> -    float_status *s = &env->vfp.fp_status;
> -    float32 one = int32_to_float32(1, s);

Use float32_one.

> +    if (float32_is_any_nan(a)) {
> +        return float32_default_nan;

Should raise InvalidOp for an SNaN.

> +    } else if (float32_is_zero(a)) {
> +        float_raise(float_flag_divbyzero, s);
> +        return float32_set_sign(float32_infinity, float32_is_neg(a));

Should use float32_is_zero_or_denormal()
so we handle denormals properly.

> +    } else if (val < 0) {

Use if (float32_is_neg(a)). Then you can make
val and val64 bit uint32_t,uint64_t rather than
the signed types, which makes more sense given
these are really just bit patterns.

> +    if ((val & 0x800000) == 0) {
> +        f64 = make_float64(((uint64_t)(val & 0x80000000) << 32)
> +                           | (0x3feULL << 52)
> +                           | ((uint64_t)(val & 0x7ffffff) << 29));
> +    } else {
> +        f64 = make_float64(((uint64_t)(val & 0x80000000) << 32)
> +                           | (0x3fdULL << 52)
> +                           | ((uint64_t)(val & 0x7ffffff) << 29));
> +    }

These are both wrong -- the ARM ARM says f64
gets operand<31> : ‘01111111110’ : operand<22:0> : Zeros(29);
(or similar for the other case), which means you
need to mask with 0x7fffff. (Count the 'f's !)

If you fix this and the denormal case then it passes
random testing.

-- PMM



reply via email to

[Prev in Thread] Current Thread [Next in Thread]