From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=47571 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1Q91jr-0002qB-J9 for qemu-devel@nongnu.org; Sun, 10 Apr 2011 17:00:08 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Q91jq-0001ea-Eb for qemu-devel@nongnu.org; Sun, 10 Apr 2011 17:00:07 -0400 Received: from hall.aurel32.net ([88.191.126.93]:43577) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Q91jq-0001eR-3M for qemu-devel@nongnu.org; Sun, 10 Apr 2011 17:00:06 -0400 Date: Sun, 10 Apr 2011 23:00:04 +0200 From: Aurelien Jarno Subject: Re: [Qemu-devel] Re: [PATCH 4/5] softfloat: add float{32, 64, x80, 128}_unordered() functions Message-ID: <20110410210004.GN28617@hall.aurel32.net> References: <1302462807-8795-1-git-send-email-aurelien@aurel32.net> <1302462807-8795-4-git-send-email-aurelien@aurel32.net> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-15 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: Sender: Aurelien Jarno List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Peter Maydell Cc: qemu-devel@nongnu.org On Sun, Apr 10, 2011 at 08:59:04PM +0100, Peter Maydell wrote: > On 10 April 2011 20:13, Aurelien Jarno 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 aurelien@aurel32.net http://www.aurel32.net