qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH 08/12] ARM: Return correct result for single<->d


From: Peter Maydell
Subject: Re: [Qemu-devel] [PATCH 08/12] ARM: Return correct result for single<->double conversion of NaN
Date: Tue, 30 Nov 2010 11:15:56 +0000

On 29 November 2010 19:54, Nathan Froyd <address@hidden> wrote:
> Yes, this is ugly.  Are you up for running:
>
>  perl -p -i -e 's/float(\d+)_is_nan/float\1_is_quiet_nan/g' target-*/*.c
>
> (and also carefully in fpu/*) or similar and moving the bit-twiddling
> float_is_nan into fpu/?

I'm just compiling this up now. While I was eyeballing the results of
the substitution, I noticed that there are some places which are almost
certainly bugs introduced by other people not noticing that float*_is_nan()
doesn't do what it says on the tin. Three at random:

In target-ppc/op_helper.c:helper_compute_fprf():
    if (unlikely(float64_is_nan(farg.d))) {
        if (float64_is_signaling_nan(farg.d)) {
            /* Signaling NaN: flags are undefined */
            ret = 0x00;
        } else {
            /* Quiet NaN */
            ret = 0x11;
        }
    } else ...

the 'signalling NaN' case can never be taken.

In target-alpha/op_helper.c:helper_cmptun():
    if (float64_is_nan(fa) || float64_is_nan(fb))
        return 0x4000000000000000ULL;
    else
        return 0;

...but the web suggests this is supposed to return non-zero for any
unordered comparison, not just ones with quiet NaNs.

In target-m68k/helper.c:sub_cmp_f64:
    res = float64_sub(a, b, &env->fp_status);
    if (float64_is_nan(res)) {
        /* +/-inf compares equal against itself, but sub returns nan.  */
        if (!float64_is_nan(a)
            && !float64_is_nan(b)) {
            res = float64_zero;
            if (float64_lt_quiet(a, res, &env->fp_status))
                res = float64_chs(res);
        }
    }

judging from the comments the author expected is_nan() to
be true for all NaNs.

I'm not sure what we should do about these -- they look wrong
but I don't have any of the setup to actually test whether they're
wrong. (The only ARM ones are in the Linux user nwfpe emulation
which is pretty much dead by now IMHO.)

-- PMM



reply via email to

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