Re: [Qemu-devel] [PATCH 1/6] softfloat: remove HPPA specific code

[Peter Maydell]

On 6 January 2011 14:35, Aurelien Jarno <address@hidden> wrote:
> On Wed, Jan 05, 2011 at 11:13:06PM +0000, Stuart Brady wrote:
>>    Is there any plan to deal with use of float*_is_quiet_nan(), where
>>    float*_is_any_nan() was intended?  These should really either be
>>    fixed (and tested), or if not, a FIXME should be added.
>
> The problem with these functions is that they are used in target-*/
> and not directly part of the softfloat code.
>
> I have reviewed MIPS and PowerPC targets, they both use these functions
> correctly.

MIPS looks OK. However PPC has this in helper_compute_fprf():

    if (unlikely(float64_is_quiet_nan(farg.d))) {
        if (float64_is_signaling_nan(farg.d)) {
            /* Signaling NaN: flags are undefined */
            ret = 0x00;
        } else {
            /* Quiet NaN */
            ret = 0x11;
        }

which is definitely wrong because the first case can't be reached.
The outer if should be is_any_nan(), I think.

In helper_fnmadd() and helper_fnmsub():
        if (likely(!float64_is_quiet_nan(farg1.d)))
            farg1.d = float64_chs(farg1.d);

is I think OK but somebody else might like to check.

Similarly I'm dubious about uses in helper_fsel, helper_fcmpu
and helper_fcmpo, efsctsi, efsctui, efsctsiz, efsctuiz, efsctsf,
efsctuf and all the helper_efd* functions. I haven't actually
checked them all, but for instance efdctsi in the Power ISA
2.03 spec says "NaNs are converted as though they were zero",
but qemu's code says:
    /* NaN are not treated the same way IEEE 754 does */
    if (unlikely(float64_is_quiet_nan(u.d)))
        return 0;

which is not going to do the right thing for signaling NaNs.

-- PMM