On Sun, Apr 10, 2011 at 08:59:04PM +0100, Peter Maydell wrote:
> On 10 April 2011 20:13, Aurelien Jarno <aurel...@aurel32.net> wrote:
> > Add float{32,64,x80,128}_unordered() functions to softfloat, matching
> > the softfloat-native ones. This allow target-i386/ops_sse.h to be
> > compiled with softfloat.
>
> I guess you could have made the x86 target use float*_compare()
> instead, but I agree that it makes sense to have the unordered()
> comparison to match the other specific-comparison ops.
Given it's used in the same macro which also handle float*_le, _ge and
so on, it was easier that way. Also float*_compare() is probably a bit
slower as it does a bit more stuff.
> > /*----------------------------------------------------------------------------
> > +| Returns 1 if the single-precision floating-point values `a' and `b'
> > cannot
> > +| be compared, and 0 otherwise. The comparison is performed according to
> > the
> > +| IEC/IEEE Standard for Binary Floating-Point Arithmetic.
> > +*----------------------------------------------------------------------------*/
> > +
> > +int float32_unordered( float32 a, float32 b STATUS_PARAM )
> > +{
> > + a = float32_squash_input_denormal(a STATUS_VAR);
> > + b = float32_squash_input_denormal(b STATUS_VAR);
> > +
> > + if ( ( ( extractFloat32Exp( a ) == 0xFF ) && extractFloat32Frac( a
> > ) )
> > + || ( ( extractFloat32Exp( b ) == 0xFF ) && extractFloat32Frac( b
> > ) )
> > + ) {
> > + if ( float32_is_signaling_nan( a ) || float32_is_signaling_nan( b
> > ) ) {
> > + float_raise( float_flag_invalid STATUS_VAR);
> > + }
> > + return 1;
> > + }
> > +
> > + return 0;
> > +}
>
> So the NaN signalling semantics here are that we raise Invalid
> for an SNaN but not for a QNaN. That's correct for the x86 op
> we're implementing, but the float*_lt, _le and _compare functions
> use the _quiet suffix for these semantics (with plain float*_lt
> etc being "raise Invalid for both QNaN and SNaN"). So I think
> these functions should be float*_unordered_quiet().
Ok, will change that.
> Annoyingly for eq the two versions use a different convention,
> so we have float*_eq [raise Invalid only if SNaN] and
> float*_eq_signaling [for any NaN] -- ideally that inconsistency
> should be fixed...
I'll try to send a patch for that in my next version of the series.
> > +int float64_unordered( float64 a, float64 b STATUS_PARAM )
> > +{
> > + a = float64_squash_input_denormal(a STATUS_VAR);
> > + b = float64_squash_input_denormal(b STATUS_VAR);
> > +
> > + if ( ( ( extractFloat64Exp( a ) == 0x7FF ) && extractFloat64Frac( a
> > ) )
> > + || ( ( extractFloat64Exp( b ) == 0x7FF ) && extractFloat64Frac( b
> > ) )
> > + ) {
> > + if ( float64_is_signaling_nan( a ) || float64_is_signaling_nan( b
> > ) ) {
> > + float_raise( float_flag_invalid STATUS_VAR);
> > + }
> > + return 0;
> > + }
> > + return 1;
> > +}
>
> You've got the sense the wrong way round on this one, I think.
Yup, good catch.
> I note that target-mips has a private float32_is_unordered()
> and float64_is_unordered() which could probably be cleaned
> up to use these instead. You'd need to implement both the
> float*_unordered() and float*_unordered_quiet() versions.
>
I missed that when running grep. I'll also add that in my next version
of the series (so that will be x86 + mips at the end).
Thanks for the review!
--
Aurelien Jarno GPG: 1024D/F1BCDB73
aurel...@aurel32.net http://www.aurel32.net
