On Thu, Jan 06, 2011 at 03:34:38PM +0000, Peter Maydell wrote: > On 6 January 2011 14:35, Aurelien Jarno <aurel...@aurel32.net> 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.
Correct. > 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. After reading the manual again, it seems float64_is_nan() should be used here. The idea is that fnmadd returns the negated value of fmadd, and that NaN should be propagated as fnmadd was a single instruction. In QEMU chs changes the sign bit even if the value is NaN. Quoting the manual: | This instruction produces the same result as would be obtained by | using the Floating Multiply-Add instruction and then negating the | result, with the following exceptions. | * QNaNs propagate with no effect on their "sign" bit. | * QNaNs that are generated as the result of a disabled Invalid | Operation Exception have a "sign" bit of 0. | * SNaNs that are converted to QNaNs as the result of a disabled | Invalid Operation Exception retain the "sign”"bit of the SNaN. > Similarly I'm dubious about uses in helper_fsel, helper_fcmpu > and helper_fcmpo, I confirm the issue for this three helpers, tested on real hardware. > 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. > Also correct. Looks like I have read the code a bit quickly... Thanks for looking at it, I will send a patch later. -- Aurelien Jarno GPG: 1024D/F1BCDB73 aurel...@aurel32.net http://www.aurel32.net