On 29 November 2010 19:54, Nathan Froyd <froy...@codesourcery.com> 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
