[Qemu-devel] [PATCH] Fix NaN handling in softfloat

[Qemu-devel] [PATCH] Fix NaN handling in softfloat

[Aurelien Jarno]

Hi all,

The current softfloat implementation changes qNaN into sNaN when 
converting between formats, for no reason. The attached patch fixes
that. It also fixes an off-by-one in the extended double precision
format (aka floatx80), the mantissa is 64-bit long and not 63-bit
long.

With this patch applied all the glibc 2.7 floating point tests
are successfull on MIPS and MIPSEL.

Bye,
Aurelien

Index: fpu/softfloat-specialize.h
===================================================================
RCS file: /sources/qemu/qemu/fpu/softfloat-specialize.h,v
retrieving revision 1.3
diff -u -d -p -r1.3 softfloat-specialize.h
--- fpu/softfloat-specialize.h	11 May 2007 17:10:14 -0000	1.3
+++ fpu/softfloat-specialize.h	3 Nov 2007 17:21:38 -0000
@@ -121,8 +121,7 @@ static commonNaNT float32ToCommonNaN( fl
 static float32 commonNaNToFloat32( commonNaNT a )
 {
 
-    return ( ( (bits32) a.sign )<<31 ) | 0x7FC00000 | ( a.high>>41 );
-
+    return ( ( (bits32) a.sign )<<31 ) | 0x7F800000 | ( a.high>>41 );
 }
 
 /*----------------------------------------------------------------------------
@@ -233,7 +232,7 @@ static float64 commonNaNToFloat64( commo
 
     return
           ( ( (bits64) a.sign )<<63 )
-        | LIT64( 0x7FF8000000000000 )
+        | LIT64( 0x7FF0000000000000 )
         | ( a.high>>12 );
 
 }
@@ -329,7 +328,7 @@ static commonNaNT floatx80ToCommonNaN( f
     if ( floatx80_is_signaling_nan( a ) ) float_raise( float_flag_invalid STATUS_VAR);
     z.sign = a.high>>15;
     z.low = 0;
-    z.high = a.low<<1;
+    z.high = a.low;
     return z;
 
 }
@@ -343,7 +342,7 @@ static floatx80 commonNaNToFloatx80( com
 {
     floatx80 z;
 
-    z.low = LIT64( 0xC000000000000000 ) | ( a.high>>1 );
+    z.low = a.high;
     z.high = ( ( (bits16) a.sign )<<15 ) | 0x7FFF;
     return z;
 
@@ -449,7 +448,7 @@ static float128 commonNaNToFloat128( com
     float128 z;
 
     shift128Right( a.high, a.low, 16, &z.high, &z.low );
-    z.high |= ( ( (bits64) a.sign )<<63 ) | LIT64( 0x7FFF800000000000 );
+    z.high |= ( ( (bits64) a.sign )<<63 ) | LIT64( 0x7FFF000000000000 );
     return z;
 
 }

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[Daniel Jacobowitz]

On Sat, Nov 03, 2007 at 06:35:48PM +0100, Aurelien Jarno wrote:
> Hi all,
> 
> The current softfloat implementation changes qNaN into sNaN when 
> converting between formats, for no reason. The attached patch fixes
> that. It also fixes an off-by-one in the extended double precision
> format (aka floatx80), the mantissa is 64-bit long and not 63-bit
> long.
> 
> With this patch applied all the glibc 2.7 floating point tests
> are successfull on MIPS and MIPSEL.

FYI, I posted a similar patch and haven't had time to get back to it.
Andreas reminded me that we need to make sure at least one mantissa
bit is set.  If we're confident that the common NaN format will
already have some bit other than the qnan/snan bit set, this is fine;
otherwise, we might want to forcibly set some other mantissa bit.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[Aurelien Jarno]

Daniel Jacobowitz a écrit :
> On Sat, Nov 03, 2007 at 06:35:48PM +0100, Aurelien Jarno wrote:
>> Hi all,
>>
>> The current softfloat implementation changes qNaN into sNaN when 
>> converting between formats, for no reason. The attached patch fixes
>> that. It also fixes an off-by-one in the extended double precision
>> format (aka floatx80), the mantissa is 64-bit long and not 63-bit
>> long.
>>
>> With this patch applied all the glibc 2.7 floating point tests
>> are successfull on MIPS and MIPSEL.
> 
> FYI, I posted a similar patch and haven't had time to get back to it.
> Andreas reminded me that we need to make sure at least one mantissa
> bit is set.  If we're confident that the common NaN format will
> already have some bit other than the qnan/snan bit set, this is fine;
> otherwise, we might want to forcibly set some other mantissa bit.
> 

Good point. I will try to improve my patch.

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[Aurelien Jarno]

On Sat, Nov 03, 2007 at 02:06:04PM -0400, Daniel Jacobowitz wrote:
> On Sat, Nov 03, 2007 at 06:35:48PM +0100, Aurelien Jarno wrote:
> > Hi all,
> > 
> > The current softfloat implementation changes qNaN into sNaN when 
> > converting between formats, for no reason. The attached patch fixes
> > that. It also fixes an off-by-one in the extended double precision
> > format (aka floatx80), the mantissa is 64-bit long and not 63-bit
> > long.
> > 
> > With this patch applied all the glibc 2.7 floating point tests
> > are successfull on MIPS and MIPSEL.
> 
> FYI, I posted a similar patch and haven't had time to get back to it.
> Andreas reminded me that we need to make sure at least one mantissa
> bit is set.  If we're confident that the common NaN format will
> already have some bit other than the qnan/snan bit set, this is fine;
> otherwise, we might want to forcibly set some other mantissa bit.
> 

Please find an updated patch below. I have tried to match real x86, MIPS,
HPPA, PowerPC and SPARC hardware when all mantissa bits are cleared.


Index: fpu/softfloat-specialize.h
===================================================================
RCS file: /sources/qemu/qemu/fpu/softfloat-specialize.h,v
retrieving revision 1.3
diff -u -d -p -r1.3 softfloat-specialize.h
--- fpu/softfloat-specialize.h	11 May 2007 17:10:14 -0000	1.3
+++ fpu/softfloat-specialize.h	3 Nov 2007 21:17:57 -0000
@@ -120,9 +120,17 @@ static commonNaNT float32ToCommonNaN( fl
 
 static float32 commonNaNToFloat32( commonNaNT a )
 {
-
-    return ( ( (bits32) a.sign )<<31 ) | 0x7FC00000 | ( a.high>>41 );
-
+    bits32 mantissa = a.high>>41;
+    if (mantissa)
+        return ( ( (bits32) a.sign )<<31 ) | 0x7F800000 | ( mantissa );
+    else
+#if defined(TARGET_MIPS)
+        return ( ( (bits32) a.sign )<<31 ) | 0x7FBFFFFF | ( mantissa );
+#elif defined(TARGET_HPPA)
+        return ( ( (bits32) a.sign )<<31 ) | 0x7FA00000 | ( mantissa );
+#else
+        return ( ( (bits32) a.sign )<<31 ) | 0x7FC00000;
+#endif
 }
 
 /*----------------------------------------------------------------------------
@@ -230,11 +238,15 @@ static commonNaNT float64ToCommonNaN( fl
 
 static float64 commonNaNToFloat64( commonNaNT a )
 {
+    bits64 mantissa = a.high>>12;
 
-    return
-          ( ( (bits64) a.sign )<<63 )
-        | LIT64( 0x7FF8000000000000 )
-        | ( a.high>>12 );
+    if ( mantissa )
+        return ( ( (bits64) a.sign )<<63 )
+                | LIT64( 0x7FF0000000000000 )
+                | ( mantissa );
+    else
+        return ( ( (bits64) a.sign )<<63 )
+                | LIT64( 0x7FF8000000000000 );
 
 }
 
@@ -329,7 +341,7 @@ static commonNaNT floatx80ToCommonNaN( f
     if ( floatx80_is_signaling_nan( a ) ) float_raise( float_flag_invalid STATUS_VAR);
     z.sign = a.high>>15;
     z.low = 0;
-    z.high = a.low<<1;
+    z.high = a.low;
     return z;
 
 }
@@ -343,7 +355,10 @@ static floatx80 commonNaNToFloatx80( com
 {
     floatx80 z;
 
-    z.low = LIT64( 0xC000000000000000 ) | ( a.high>>1 );
+    if (a.high)
+        z.low = a.high;
+    else
+        z.low = LIT64( 0x8000000000000000 );
     z.high = ( ( (bits16) a.sign )<<15 ) | 0x7FFF;
     return z;
 
@@ -449,7 +464,7 @@ static float128 commonNaNToFloat128( com
     float128 z;
 
     shift128Right( a.high, a.low, 16, &z.high, &z.low );
-    z.high |= ( ( (bits64) a.sign )<<63 ) | LIT64( 0x7FFF800000000000 );
+    z.high |= ( ( (bits64) a.sign )<<63 ) | LIT64( 0x7FFF000000000000 );
     return z;
 
 }


-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[J. Mayer]

On Sat, 2007-11-03 at 22:28 +0100, Aurelien Jarno wrote: 
> On Sat, Nov 03, 2007 at 02:06:04PM -0400, Daniel Jacobowitz wrote:
> > On Sat, Nov 03, 2007 at 06:35:48PM +0100, Aurelien Jarno wrote:
> > > Hi all,
> > > 
> > > The current softfloat implementation changes qNaN into sNaN when 
> > > converting between formats, for no reason. The attached patch fixes
> > > that. It also fixes an off-by-one in the extended double precision
> > > format (aka floatx80), the mantissa is 64-bit long and not 63-bit
> > > long.
> > > 
> > > With this patch applied all the glibc 2.7 floating point tests
> > > are successfull on MIPS and MIPSEL.
> > 
> > FYI, I posted a similar patch and haven't had time to get back to it.
> > Andreas reminded me that we need to make sure at least one mantissa
> > bit is set.  If we're confident that the common NaN format will
> > already have some bit other than the qnan/snan bit set, this is fine;
> > otherwise, we might want to forcibly set some other mantissa bit.
> > 
> 
> Please find an updated patch below. I have tried to match real x86, MIPS,
> HPPA, PowerPC and SPARC hardware when all mantissa bits are cleared.

It's a good idea to fix NaN problems here but in my opinion, it's a bad
idea to have target dependant code here. This code should implement IEEE
behavior. Target specific behavior / deviations from the norm has to be
implemented in target specific code. As targets have to check the
presence of a NaN to update the FP flags, it seems that uglyifying this
code with target specific hacks is pointless. If the target code do not
check the presence of a NaN, that means that it does not implement
precise FPU emulation, then there's no need to have specific code to
return a precise value (I mean target dependant) from the generic code,
imho.

[...]

-- 
J. Mayer <l_indien@magic.fr>
Never organized

Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[Thiemo Seufer]

J. Mayer wrote:
> 
> On Sat, 2007-11-03 at 22:28 +0100, Aurelien Jarno wrote: 
> > On Sat, Nov 03, 2007 at 02:06:04PM -0400, Daniel Jacobowitz wrote:
> > > On Sat, Nov 03, 2007 at 06:35:48PM +0100, Aurelien Jarno wrote:
> > > > Hi all,
> > > > 
> > > > The current softfloat implementation changes qNaN into sNaN when 
> > > > converting between formats, for no reason. The attached patch fixes
> > > > that. It also fixes an off-by-one in the extended double precision
> > > > format (aka floatx80), the mantissa is 64-bit long and not 63-bit
> > > > long.
> > > > 
> > > > With this patch applied all the glibc 2.7 floating point tests
> > > > are successfull on MIPS and MIPSEL.
> > > 
> > > FYI, I posted a similar patch and haven't had time to get back to it.
> > > Andreas reminded me that we need to make sure at least one mantissa
> > > bit is set.  If we're confident that the common NaN format will
> > > already have some bit other than the qnan/snan bit set, this is fine;
> > > otherwise, we might want to forcibly set some other mantissa bit.
> > > 
> > 
> > Please find an updated patch below. I have tried to match real x86, MIPS,
> > HPPA, PowerPC and SPARC hardware when all mantissa bits are cleared.
> 
> It's a good idea to fix NaN problems here but in my opinion, it's a bad
> idea to have target dependant code here. This code should implement IEEE
> behavior.

IEEE doesn't prescribe the signalling/non-signalling bit's value.
MIPS FPUs are fully IEEE conformant.


Thiemo

Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[Aurelien Jarno]

On Tue, Nov 06, 2007 at 09:01:13PM +0100, J. Mayer wrote:
> 
> On Sat, 2007-11-03 at 22:28 +0100, Aurelien Jarno wrote: 
> > On Sat, Nov 03, 2007 at 02:06:04PM -0400, Daniel Jacobowitz wrote:
> > > On Sat, Nov 03, 2007 at 06:35:48PM +0100, Aurelien Jarno wrote:
> > > > Hi all,
> > > > 
> > > > The current softfloat implementation changes qNaN into sNaN when 
> > > > converting between formats, for no reason. The attached patch fixes
> > > > that. It also fixes an off-by-one in the extended double precision
> > > > format (aka floatx80), the mantissa is 64-bit long and not 63-bit
> > > > long.
> > > > 
> > > > With this patch applied all the glibc 2.7 floating point tests
> > > > are successfull on MIPS and MIPSEL.
> > > 
> > > FYI, I posted a similar patch and haven't had time to get back to it.
> > > Andreas reminded me that we need to make sure at least one mantissa
> > > bit is set.  If we're confident that the common NaN format will
> > > already have some bit other than the qnan/snan bit set, this is fine;
> > > otherwise, we might want to forcibly set some other mantissa bit.
> > > 
> > 
> > Please find an updated patch below. I have tried to match real x86, MIPS,
> > HPPA, PowerPC and SPARC hardware when all mantissa bits are cleared.
> 
> It's a good idea to fix NaN problems here but in my opinion, it's a bad
> idea to have target dependant code here. This code should implement IEEE
> behavior. Target specific behavior / deviations from the norm has to be

Has Thiemo already said, there is no IEEE behavior. If you look at the
IEEE 754 document you will see that it has requirements on what should
be supported by an IEEE compliant FPU, but has very few requirements on
the implementation.

> implemented in target specific code. As targets have to check the
> presence of a NaN to update the FP flags, it seems that uglyifying this
> code with target specific hacks is pointless. If the target code do not
> check the presence of a NaN, that means that it does not implement
> precise FPU emulation, then there's no need to have specific code to
> return a precise value (I mean target dependant) from the generic code,
> imho.

I actually know very few CPU that check for NaN in general. They check
for sNaN as required by IEEE 754, but rarely for qNaN as their purpose
is exactly to be propagated through all FPU operations as a normal FP 
number would be.

Anyway there is no way to do that in the target specific code *after 
the conversion*, as the detection of a mantissa being nul when 
converting from double to single precision can only be done when both
values are still known. In other words when the value is not fixed 
during the conversion, the value 0x7f800000 can either be infinity or a
conversion of NaN from double to single precision, and thus is it not
possible to fix the value afterwards in the target specific code.

That said, it is possible to check for a NaN before and after the call
to the FP conversion routine, but that will slow down the emulation.


-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[Daniel Jacobowitz]

On Thu, Nov 08, 2007 at 12:05:25AM +0100, Aurelien Jarno wrote:
> Has Thiemo already said, there is no IEEE behavior. If you look at the
> IEEE 754 document you will see that it has requirements on what should
> be supported by an IEEE compliant FPU, but has very few requirements on
> the implementation.

If folks don't like the target conditionals there, I recommend we just
set some low bit to be sure it's a NaN and move on.  The softfloat
implementation is not all that close to matching any one hardware FPU.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[Thiemo Seufer]

Daniel Jacobowitz wrote:
> On Thu, Nov 08, 2007 at 12:05:25AM +0100, Aurelien Jarno wrote:
> > Has Thiemo already said, there is no IEEE behavior. If you look at the
> > IEEE 754 document you will see that it has requirements on what should
> > be supported by an IEEE compliant FPU, but has very few requirements on
> > the implementation.
> 
> If folks don't like the target conditionals there, I recommend we just
> set some low bit to be sure it's a NaN and move on.

I disagree. I chose on purpose the format recommended in the MIPS
architecture specification. The general format seems to be commonly
preferred beyond the MIPS world.

> The softfloat
> implementation is not all that close to matching any one hardware FPU.

At least for MIPS I expect it to be close enough to real hardware.


Thiemo

Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[J. Mayer]

On Thu, 2007-11-08 at 00:05 +0100, Aurelien Jarno wrote:
> On Tue, Nov 06, 2007 at 09:01:13PM +0100, J. Mayer wrote:
> > 
> > On Sat, 2007-11-03 at 22:28 +0100, Aurelien Jarno wrote: 
> > > On Sat, Nov 03, 2007 at 02:06:04PM -0400, Daniel Jacobowitz wrote:
> > > > On Sat, Nov 03, 2007 at 06:35:48PM +0100, Aurelien Jarno wrote:
> > > > > Hi all,
> > > > > 
> > > > > The current softfloat implementation changes qNaN into sNaN when 
> > > > > converting between formats, for no reason. The attached patch fixes
> > > > > that. It also fixes an off-by-one in the extended double precision
> > > > > format (aka floatx80), the mantissa is 64-bit long and not 63-bit
> > > > > long.
> > > > > 
> > > > > With this patch applied all the glibc 2.7 floating point tests
> > > > > are successfull on MIPS and MIPSEL.
> > > > 
> > > > FYI, I posted a similar patch and haven't had time to get back to it.
> > > > Andreas reminded me that we need to make sure at least one mantissa
> > > > bit is set.  If we're confident that the common NaN format will
> > > > already have some bit other than the qnan/snan bit set, this is fine;
> > > > otherwise, we might want to forcibly set some other mantissa bit.
> > > > 
> > > 
> > > Please find an updated patch below. I have tried to match real x86, MIPS,
> > > HPPA, PowerPC and SPARC hardware when all mantissa bits are cleared.
> > 
> > It's a good idea to fix NaN problems here but in my opinion, it's a bad
> > idea to have target dependant code here. This code should implement IEEE
> > behavior. Target specific behavior / deviations from the norm has to be
> 
> Has Thiemo already said, there is no IEEE behavior. If you look at the
> IEEE 754 document you will see that it has requirements on what should
> be supported by an IEEE compliant FPU, but has very few requirements on
> the implementation.
> 

OK.

> > implemented in target specific code. As targets have to check the
> > presence of a NaN to update the FP flags, it seems that uglyifying this
> > code with target specific hacks is pointless. If the target code do not
> > check the presence of a NaN, that means that it does not implement
> > precise FPU emulation, then there's no need to have specific code to
> > return a precise value (I mean target dependant) from the generic code,
> > imho.
> 
> I actually know very few CPU that check for NaN in general. They check
> for sNaN as required by IEEE 754, but rarely for qNaN as their purpose
> is exactly to be propagated through all FPU operations as a normal FP 
> number would be.

CPU do check QNaNs because most of them update a specific flag that can
be checked to know there was a NaN seen during FPU operations. I don't
know for all FPU, but I can see that the PowerPC gives me 4 bits that
give the class of the last FPU result and I guess you have those kind of
flags in most implementations.

> Anyway there is no way to do that in the target specific code *after 
> the conversion*, as the detection of a mantissa being nul when 
> converting from double to single precision can only be done when both
> values are still known. In other words when the value is not fixed 
> during the conversion, the value 0x7f800000 can either be infinity or a
> conversion of NaN from double to single precision, and thus is it not
> possible to fix the value afterwards in the target specific code.

I don't say you have to return an infinity when the argument is a qNaN.
I just say you have to return a qNaN in a generic way.  Just return sign
| 0x7f800000 | mantissa, which is the more generic form and seems to me
to even be OK for sNaNs. It's even needed for some target (not to say
PowerPC) that specify that the result have to be equal to the operand
(in the single precision format, of course) in such a case. This is
simpler, it ensures that any target could then detect the presence of a
NaN, know which one, and can then adjust the value according to its
specification if needed.
I then still can'tl see any reason of having target specific code in
that area.

-- 
J. Mayer <l_indien@magic.fr>
Never organized

Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[Aurelien Jarno]

J. Mayer a écrit :
> On Thu, 2007-11-08 at 00:05 +0100, Aurelien Jarno wrote:
>> On Tue, Nov 06, 2007 at 09:01:13PM +0100, J. Mayer wrote:
>>> On Sat, 2007-11-03 at 22:28 +0100, Aurelien Jarno wrote: 
>>>> On Sat, Nov 03, 2007 at 02:06:04PM -0400, Daniel Jacobowitz wrote:
>>>>> On Sat, Nov 03, 2007 at 06:35:48PM +0100, Aurelien Jarno wrote:
>>>>>> Hi all,
>>>>>>
>>>>>> The current softfloat implementation changes qNaN into sNaN when 
>>>>>> converting between formats, for no reason. The attached patch fixes
>>>>>> that. It also fixes an off-by-one in the extended double precision
>>>>>> format (aka floatx80), the mantissa is 64-bit long and not 63-bit
>>>>>> long.
>>>>>>
>>>>>> With this patch applied all the glibc 2.7 floating point tests
>>>>>> are successfull on MIPS and MIPSEL.
>>>>> FYI, I posted a similar patch and haven't had time to get back to it.
>>>>> Andreas reminded me that we need to make sure at least one mantissa
>>>>> bit is set.  If we're confident that the common NaN format will
>>>>> already have some bit other than the qnan/snan bit set, this is fine;
>>>>> otherwise, we might want to forcibly set some other mantissa bit.
>>>>>
>>>> Please find an updated patch below. I have tried to match real x86, MIPS,
>>>> HPPA, PowerPC and SPARC hardware when all mantissa bits are cleared.
>>> It's a good idea to fix NaN problems here but in my opinion, it's a bad
>>> idea to have target dependant code here. This code should implement IEEE
>>> behavior. Target specific behavior / deviations from the norm has to be
>> Has Thiemo already said, there is no IEEE behavior. If you look at the
>> IEEE 754 document you will see that it has requirements on what should
>> be supported by an IEEE compliant FPU, but has very few requirements on
>> the implementation.
>>
> 
> OK.
> 
>>> implemented in target specific code. As targets have to check the
>>> presence of a NaN to update the FP flags, it seems that uglyifying this
>>> code with target specific hacks is pointless. If the target code do not
>>> check the presence of a NaN, that means that it does not implement
>>> precise FPU emulation, then there's no need to have specific code to
>>> return a precise value (I mean target dependant) from the generic code,
>>> imho.
>> I actually know very few CPU that check for NaN in general. They check
>> for sNaN as required by IEEE 754, but rarely for qNaN as their purpose
>> is exactly to be propagated through all FPU operations as a normal FP 
>> number would be.
> 
> CPU do check QNaNs because most of them update a specific flag that can
> be checked to know there was a NaN seen during FPU operations. I don't
> know for all FPU, but I can see that the PowerPC gives me 4 bits that
> give the class of the last FPU result and I guess you have those kind of
> flags in most implementations.

This is not the case of MIPS for example.

>> Anyway there is no way to do that in the target specific code *after 
>> the conversion*, as the detection of a mantissa being nul when 
>> converting from double to single precision can only be done when both
>> values are still known. In other words when the value is not fixed 
>> during the conversion, the value 0x7f800000 can either be infinity or a
>> conversion of NaN from double to single precision, and thus is it not
>> possible to fix the value afterwards in the target specific code.
> 
> I don't say you have to return an infinity when the argument is a qNaN.
> I just say you have to return a qNaN in a generic way.  Just return sign
> | 0x7f800000 | mantissa, which is the more generic form and seems to me
> to even be OK for sNaNs. It's even needed for some target (not to say

0x7f800000 is actually not a NaN, but infinity.

> PowerPC) that specify that the result have to be equal to the operand
> (in the single precision format, of course) in such a case. This is
> simpler, it ensures that any target could then detect the presence of a
> NaN, know which one, and can then adjust the value according to its
> specification if needed.
> I then still can'tl see any reason of having target specific code in
> that area.

Ok, let's give an example then. On MIPS let's say you want to convert
0x7ff0000000000001 (qNaN) to single precision. The mantissa shifted to
the right become 0, so you have to generate a new value. As you
proposed, let's generate a "generic value" 0x7fc00000 in the softfloat
routines. This value has to be converted to 0x7fbfffff in the MIPS
target code.

However, that doesn't work because 0x7ff8000000000000 (sNaN) will also
be converted to 0x7fc00000 in the softfloat routines. The MIPS target
code will convert that into 0x7bffffff, which is wrong because it is a
qNaN, and because the mantissa has been changed while it should have not.

In short this is not possible to fix a value while it is already
converted to single precision, because all the possible 2^32 values have
a signification.

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[J. Mayer]

On Sat, 2007-11-10 at 10:35 +0100, Aurelien Jarno wrote:
> J. Mayer a écrit :
> > On Thu, 2007-11-08 at 00:05 +0100, Aurelien Jarno wrote:
> >> On Tue, Nov 06, 2007 at 09:01:13PM +0100, J. Mayer wrote:
> >>> On Sat, 2007-11-03 at 22:28 +0100, Aurelien Jarno wrote: 
> >>>> On Sat, Nov 03, 2007 at 02:06:04PM -0400, Daniel Jacobowitz wrote:
> >>>>> On Sat, Nov 03, 2007 at 06:35:48PM +0100, Aurelien Jarno wrote:
> >>>>>> Hi all,
> >>>>>>
> >>>>>> The current softfloat implementation changes qNaN into sNaN when 
> >>>>>> converting between formats, for no reason. The attached patch fixes
> >>>>>> that. It also fixes an off-by-one in the extended double precision
> >>>>>> format (aka floatx80), the mantissa is 64-bit long and not 63-bit
> >>>>>> long.
> >>>>>>
> >>>>>> With this patch applied all the glibc 2.7 floating point tests
> >>>>>> are successfull on MIPS and MIPSEL.
[...]
> >> Anyway there is no way to do that in the target specific code *after 
> >> the conversion*, as the detection of a mantissa being nul when 
> >> converting from double to single precision can only be done when both
> >> values are still known. In other words when the value is not fixed 
> >> during the conversion, the value 0x7f800000 can either be infinity or a
> >> conversion of NaN from double to single precision, and thus is it not
> >> possible to fix the value afterwards in the target specific code.
> > 
> > I don't say you have to return an infinity when the argument is a qNaN.
> > I just say you have to return a qNaN in a generic way.  Just return sign
> > | 0x7f800000 | mantissa, which is the more generic form and seems to me
> > to even be OK for sNaNs. It's even needed for some target (not to say
> 
> 0x7f800000 is actually not a NaN, but infinity.
> 
> > PowerPC) that specify that the result have to be equal to the operand
> > (in the single precision format, of course) in such a case. This is
> > simpler, it ensures that any target could then detect the presence of a
> > NaN, know which one, and can then adjust the value according to its
> > specification if needed.
> > I then still can'tl see any reason of having target specific code in
> > that area.
> 
> Ok, let's give an example then. On MIPS let's say you want to convert
> 0x7ff0000000000001 (qNaN) to single precision. The mantissa shifted to
> the right become 0, so you have to generate a new value. As you
> proposed, let's generate a "generic value" 0x7fc00000 in the softfloat
> routines. This value has to be converted to 0x7fbfffff in the MIPS
> target code.

OK, the values that can cause a problem is all values that would have a
zero mantissa once rounded to sinlge-precision. As the PowerPC requires
that the result would have a zero mantissa (and the result class set to
qNan), I can see no way to handle this case in the generic code. And
even adding a "#ifdef TARGET_PPC" won't solve the problem as the PowerPC
code would not be able to make the distinction between infinity case and
qNaN case. Then, the only solution, as you already mentioned, is to
check for qNaN before calling the rounding function. As the target
emulation code already has to check for sNaN to be able to raise an
exception when it's needed, checking for qNaN would cost nothing more;
just have to change the check if (float64_is_signaling_nan) check with a
check for NaN and handle the two cases by hand. I can see no other way
to have all cases handled for all targets specific cases, do you ?

[...]

-- 
J. Mayer <l_indien@magic.fr>
Never organized

Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[Aurelien Jarno]

J. Mayer a écrit :
> On Sat, 2007-11-10 at 10:35 +0100, Aurelien Jarno wrote:
>> J. Mayer a écrit :
>>> On Thu, 2007-11-08 at 00:05 +0100, Aurelien Jarno wrote:
>>>> On Tue, Nov 06, 2007 at 09:01:13PM +0100, J. Mayer wrote:
>>>>> On Sat, 2007-11-03 at 22:28 +0100, Aurelien Jarno wrote: 
>>>>>> On Sat, Nov 03, 2007 at 02:06:04PM -0400, Daniel Jacobowitz wrote:
>>>>>>> On Sat, Nov 03, 2007 at 06:35:48PM +0100, Aurelien Jarno wrote:
>>>>>>>> Hi all,
>>>>>>>>
>>>>>>>> The current softfloat implementation changes qNaN into sNaN when 
>>>>>>>> converting between formats, for no reason. The attached patch fixes
>>>>>>>> that. It also fixes an off-by-one in the extended double precision
>>>>>>>> format (aka floatx80), the mantissa is 64-bit long and not 63-bit
>>>>>>>> long.
>>>>>>>>
>>>>>>>> With this patch applied all the glibc 2.7 floating point tests
>>>>>>>> are successfull on MIPS and MIPSEL.
> [...]
>>>> Anyway there is no way to do that in the target specific code *after 
>>>> the conversion*, as the detection of a mantissa being nul when 
>>>> converting from double to single precision can only be done when both
>>>> values are still known. In other words when the value is not fixed 
>>>> during the conversion, the value 0x7f800000 can either be infinity or a
>>>> conversion of NaN from double to single precision, and thus is it not
>>>> possible to fix the value afterwards in the target specific code.
>>> I don't say you have to return an infinity when the argument is a qNaN.
>>> I just say you have to return a qNaN in a generic way.  Just return sign
>>> | 0x7f800000 | mantissa, which is the more generic form and seems to me
>>> to even be OK for sNaNs. It's even needed for some target (not to say
>> 0x7f800000 is actually not a NaN, but infinity.
>>
>>> PowerPC) that specify that the result have to be equal to the operand
>>> (in the single precision format, of course) in such a case. This is
>>> simpler, it ensures that any target could then detect the presence of a
>>> NaN, know which one, and can then adjust the value according to its
>>> specification if needed.
>>> I then still can'tl see any reason of having target specific code in
>>> that area.
>> Ok, let's give an example then. On MIPS let's say you want to convert
>> 0x7ff0000000000001 (qNaN) to single precision. The mantissa shifted to
>> the right become 0, so you have to generate a new value. As you
>> proposed, let's generate a "generic value" 0x7fc00000 in the softfloat
>> routines. This value has to be converted to 0x7fbfffff in the MIPS
>> target code.
> 
> OK, the values that can cause a problem is all values that would have a
> zero mantissa once rounded to sinlge-precision. As the PowerPC requires
> that the result would have a zero mantissa (and the result class set to

Are you sure of that? According to IEEE 754 a zero mantissa is not a
NaN. And tests on a real machine shows different results.
0x7ff0000000000001 is converted to 0x7fc00000 on a 740/750 CPU.


> qNan), I can see no way to handle this case in the generic code. And
> even adding a "#ifdef TARGET_PPC" won't solve the problem as the PowerPC
> code would not be able to make the distinction between infinity case and
> qNaN case. Then, the only solution, as you already mentioned, is to
> check for qNaN before calling the rounding function. As the target
> emulation code already has to check for sNaN to be able to raise an
> exception when it's needed, checking for qNaN would cost nothing more;

Except this is currently done *after* the call to the rounding function,
using the flags returned by the softmmu routines. Doing a check before
and after would slow down the emulation.

> just have to change the check if (float64_is_signaling_nan) check with a
> check for NaN and handle the two cases by hand. I can see no other way
> to have all cases handled for all targets specific cases, do you ?
> 

If you can confirm that the all PowerPC CPU are IEEE compliant and
should behave like the 740/750, the patch I proposed is another way to
be correct on all target, including PowerPC. But it seems you don't
really like to add target specific code in the softmmu routines.

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[J. Mayer]

On Sat, 2007-11-10 at 17:15 +0100, Aurelien Jarno wrote:
> J. Mayer a écrit :
> > On Sat, 2007-11-10 at 10:35 +0100, Aurelien Jarno wrote:
> >> J. Mayer a écrit :
> >>> On Thu, 2007-11-08 at 00:05 +0100, Aurelien Jarno wrote:
> >>>> On Tue, Nov 06, 2007 at 09:01:13PM +0100, J. Mayer wrote:
> >>>>> On Sat, 2007-11-03 at 22:28 +0100, Aurelien Jarno wrote: 
> >>>>>> On Sat, Nov 03, 2007 at 02:06:04PM -0400, Daniel Jacobowitz wrote:
> >>>>>>> On Sat, Nov 03, 2007 at 06:35:48PM +0100, Aurelien Jarno wrote:
> >>>>>>>> Hi all,
> >>>>>>>>
> >>>>>>>> The current softfloat implementation changes qNaN into sNaN when 
> >>>>>>>> converting between formats, for no reason. The attached patch fixes
> >>>>>>>> that. It also fixes an off-by-one in the extended double precision
> >>>>>>>> format (aka floatx80), the mantissa is 64-bit long and not 63-bit
> >>>>>>>> long.
> >>>>>>>>
> >>>>>>>> With this patch applied all the glibc 2.7 floating point tests
> >>>>>>>> are successfull on MIPS and MIPSEL.
> > [...]
> >>>> Anyway there is no way to do that in the target specific code *after 
> >>>> the conversion*, as the detection of a mantissa being nul when 
> >>>> converting from double to single precision can only be done when both
> >>>> values are still known. In other words when the value is not fixed 
> >>>> during the conversion, the value 0x7f800000 can either be infinity or a
> >>>> conversion of NaN from double to single precision, and thus is it not
> >>>> possible to fix the value afterwards in the target specific code.
> >>> I don't say you have to return an infinity when the argument is a qNaN.
> >>> I just say you have to return a qNaN in a generic way.  Just return sign
> >>> | 0x7f800000 | mantissa, which is the more generic form and seems to me
> >>> to even be OK for sNaNs. It's even needed for some target (not to say
> >> 0x7f800000 is actually not a NaN, but infinity.
> >>
> >>> PowerPC) that specify that the result have to be equal to the operand
> >>> (in the single precision format, of course) in such a case. This is
> >>> simpler, it ensures that any target could then detect the presence of a
> >>> NaN, know which one, and can then adjust the value according to its
> >>> specification if needed.
> >>> I then still can'tl see any reason of having target specific code in
> >>> that area.
> >> Ok, let's give an example then. On MIPS let's say you want to convert
> >> 0x7ff0000000000001 (qNaN) to single precision. The mantissa shifted to
> >> the right become 0, so you have to generate a new value. As you
> >> proposed, let's generate a "generic value" 0x7fc00000 in the softfloat
> >> routines. This value has to be converted to 0x7fbfffff in the MIPS
> >> target code.
> > 
> > OK, the values that can cause a problem is all values that would have a
> > zero mantissa once rounded to sinlge-precision. As the PowerPC requires
> > that the result would have a zero mantissa (and the result class set to
> 
> Are you sure of that? According to IEEE 754 a zero mantissa is not a
> NaN. And tests on a real machine shows different results.
> 0x7ff0000000000001 is converted to 0x7fc00000 on a 740/750 CPU.

First, please note that a PowerPC do not have any single precision
register nor internal representation. The operation here is "round to
single precision" (frsp) but the result is still a 64 bits float. Then
the result is more likely to be 0x7fc0000000000000.
0x7FF0000000000001 seems to be a SNaN, according to what I see in the
PowerPC specification. Then the result is OK: when no exception is
raised, SNaN is converted to QNaN during rounding to single operation
(please see below).
What about 0x7FF8000000000001, which is a QNaN ? According to the
PowerPC specification, this should be rounded to 0x7FF8000000000000
which is also a QNaN, then is also OK. Then rounding the mantissa and
copying sign || exponent || mantissa would, in fact, always be OK in the
PowerPC case.
What seem to appear to me now is that the problems are due to the fact
Mips have an inverted representation of SNaN / QNaN (if I understood
well) that do not allow distinction between a rounded QNaN and an
infinity...

> > qNan), I can see no way to handle this case in the generic code. And
> > even adding a "#ifdef TARGET_PPC" won't solve the problem as the PowerPC
> > code would not be able to make the distinction between infinity case and
> > qNaN case. Then, the only solution, as you already mentioned, is to
> > check for qNaN before calling the rounding function. As the target
> > emulation code already has to check for sNaN to be able to raise an
> > exception when it's needed, checking for qNaN would cost nothing more;
> 
> Except this is currently done *after* the call to the rounding function,
> using the flags returned by the softmmu routines. Doing a check before
> and after would slow down the emulation.

On PowerPC at least, you have to check operands for sNaN _before_ doing
any floating-point operation and check the result _after_ having done
the floating-point computation in order to set the flags.
The sNaN operand exception must be raised before any computation is
done, because it's related to the operation operands which may be lost
during the operation if the target register is the same than an operand
one.
The other exceptions, based on the result of the operation, must be
raised after the result has been computed and stored.
Then, I can see way not to check for SNaN first. And I guess this is the
case for most FPU...

> > just have to change the check if (float64_is_signaling_nan) check with a
> > check for NaN and handle the two cases by hand. I can see no other way
> > to have all cases handled for all targets specific cases, do you ?
> > 
> 
> If you can confirm that the all PowerPC CPU are IEEE compliant and
> should behave like the 740/750, the patch I proposed is another way to
> be correct on all target, including PowerPC. But it seems you don't
> really like to add target specific code in the softmmu routines.

The code you proposed is not correct for PowerPC.
Here's what the PowerPC specification says:
Just a few precisions before:
* bit 0 is MSB in IBM representation
* notation (XXX)m:n means bits from m to n (included) of XXX
* FRB is the operand register
* FRT is the result register
* FPSCR is the FPU flags register

Floating point round to single-precision model:
(just keeping the NaN parts and adding comments, hope they are
releveant !)
[...]
if ((FRB) 1:11 == 2047) then
    if ((FRB)12:63 == 0) then goto Infinity Operand /* zero mantissa */
    if ((FRB)12 == 1) then goto QNaN Operand           /* mantissa MSB
is one */
   if ((FRB)12 == 0) and ((FRB)13:63 > 0) then goto SNaN Operand /*
mantissa MSB is zero and mantissa != 0 */ 

[...]

QNaN Operand:
  FRT <- (FRB)0:34 || 0 /* Copy the highest 35 bits of the operand to
result */
  (FPSCR)FPRF <- QNaN  /* Set result class to QNaN */
  (FPSCR)FRFI <- 0b00   /* Reset result sign / zero flags */

SNaN Operand:
1 <- (FPSCR)VXSNAN <- 1 /* Set SNaN exception flag */
if ((FPSCR)VE == 0) then /* SNaN exceptions are disabled */
    (FRT)0:11 <- (FRB)0:11 /* Copy sign / exponent from the operand to
result */
    (FRT)12 <- 1 /* Ensure the the highest bit of mantissa is 1 */
    (FRT)13:63 <- (FRB)13:34 || 0 /* Copy the bits 13:34 from the
operand to result */
    (FPSCR)FPRF <- QNaN /* set the result class to QNaN */
(FPSCR)FRFI <- 0b00 /* Reset result sign / zero flags */

Then, as mentionned previously, there is no problem in this model,
except the fact that it keeps more bits than the ones that would fit in
a single-precision float in case of NaN: it keeps 35 bits of the
operand, then this cannot be handled by the generic routine that would
just keep 32 bits... Then I will still have to check this case at the
same place I check for sNaN in order to always have the correct result.
If the SNaN / QNaN representation is inverted, compared to this model,
then yes, you have a problem with this method. But I still don't think
adding target specific code in generic code that should not be target
dependant is the solution. Having a define signaling that QNaN is 1/0 //
SNaN is 0/1 would be more generic and checking that flag (and not
TARGET_XXX) in that code would be more acceptable. But I keep thinking
that a check on the operand is needed before computation for most CPUs,
then this hack should not be needed...

-- 
J. Mayer <l_indien@magic.fr>
Never organized

Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[Aurelien Jarno]

J. Mayer a écrit :
> On Sat, 2007-11-10 at 17:15 +0100, Aurelien Jarno wrote:
>> J. Mayer a écrit :
>>> On Sat, 2007-11-10 at 10:35 +0100, Aurelien Jarno wrote:
>>>> J. Mayer a écrit :
>>>>> On Thu, 2007-11-08 at 00:05 +0100, Aurelien Jarno wrote:
>>>>>> On Tue, Nov 06, 2007 at 09:01:13PM +0100, J. Mayer wrote:
>>>>>>> On Sat, 2007-11-03 at 22:28 +0100, Aurelien Jarno wrote: 
>>>>>>>> On Sat, Nov 03, 2007 at 02:06:04PM -0400, Daniel Jacobowitz wrote:
>>>>>>>>> On Sat, Nov 03, 2007 at 06:35:48PM +0100, Aurelien Jarno wrote:
>>>>>>>>>> Hi all,
>>>>>>>>>>
>>>>>>>>>> The current softfloat implementation changes qNaN into sNaN when 
>>>>>>>>>> converting between formats, for no reason. The attached patch fixes
>>>>>>>>>> that. It also fixes an off-by-one in the extended double precision
>>>>>>>>>> format (aka floatx80), the mantissa is 64-bit long and not 63-bit
>>>>>>>>>> long.
>>>>>>>>>>
>>>>>>>>>> With this patch applied all the glibc 2.7 floating point tests
>>>>>>>>>> are successfull on MIPS and MIPSEL.
>>> [...]
>>>>>> Anyway there is no way to do that in the target specific code *after 
>>>>>> the conversion*, as the detection of a mantissa being nul when 
>>>>>> converting from double to single precision can only be done when both
>>>>>> values are still known. In other words when the value is not fixed 
>>>>>> during the conversion, the value 0x7f800000 can either be infinity or a
>>>>>> conversion of NaN from double to single precision, and thus is it not
>>>>>> possible to fix the value afterwards in the target specific code.
>>>>> I don't say you have to return an infinity when the argument is a qNaN.
>>>>> I just say you have to return a qNaN in a generic way.  Just return sign
>>>>> | 0x7f800000 | mantissa, which is the more generic form and seems to me
>>>>> to even be OK for sNaNs. It's even needed for some target (not to say
>>>> 0x7f800000 is actually not a NaN, but infinity.
>>>>
>>>>> PowerPC) that specify that the result have to be equal to the operand
>>>>> (in the single precision format, of course) in such a case. This is
>>>>> simpler, it ensures that any target could then detect the presence of a
>>>>> NaN, know which one, and can then adjust the value according to its
>>>>> specification if needed.
>>>>> I then still can'tl see any reason of having target specific code in
>>>>> that area.
>>>> Ok, let's give an example then. On MIPS let's say you want to convert
>>>> 0x7ff0000000000001 (qNaN) to single precision. The mantissa shifted to
>>>> the right become 0, so you have to generate a new value. As you
>>>> proposed, let's generate a "generic value" 0x7fc00000 in the softfloat
>>>> routines. This value has to be converted to 0x7fbfffff in the MIPS
>>>> target code.
>>> OK, the values that can cause a problem is all values that would have a
>>> zero mantissa once rounded to sinlge-precision. As the PowerPC requires
>>> that the result would have a zero mantissa (and the result class set to
>> Are you sure of that? According to IEEE 754 a zero mantissa is not a
>> NaN. And tests on a real machine shows different results.
>> 0x7ff0000000000001 is converted to 0x7fc00000 on a 740/750 CPU.
> 
> First, please note that a PowerPC do not have any single precision
> register nor internal representation. The operation here is "round to
> single precision" (frsp) but the result is still a 64 bits float. Then
> the result is more likely to be 0x7fc0000000000000.
> 0x7FF0000000000001 seems to be a SNaN, according to what I see in the
> PowerPC specification. Then the result is OK: when no exception is
> raised, SNaN is converted to QNaN during rounding to single operation
> (please see below).
> What about 0x7FF8000000000001, which is a QNaN ? According to the
> PowerPC specification, this should be rounded to 0x7FF8000000000000
> which is also a QNaN, then is also OK. Then rounding the mantissa and
> copying sign || exponent || mantissa would, in fact, always be OK in the
> PowerPC case.
> What seem to appear to me now is that the problems are due to the fact
> Mips have an inverted representation of SNaN / QNaN (if I understood
> well) that do not allow distinction between a rounded QNaN and an
> infinity...

Nope it is not due to the fact that MIPS uses an "inverted"
representation. It is the same problem on x86 or other target, except
that they can allow the distinction between a rounded SNaN and an
infinity. The problem is present on all targets that can represent a
single precision FP value.


>>> qNan), I can see no way to handle this case in the generic code. And
>>> even adding a "#ifdef TARGET_PPC" won't solve the problem as the PowerPC
>>> code would not be able to make the distinction between infinity case and
>>> qNaN case. Then, the only solution, as you already mentioned, is to
>>> check for qNaN before calling the rounding function. As the target
>>> emulation code already has to check for sNaN to be able to raise an
>>> exception when it's needed, checking for qNaN would cost nothing more;
>> Except this is currently done *after* the call to the rounding function,
>> using the flags returned by the softmmu routines. Doing a check before
>> and after would slow down the emulation.
> 
> On PowerPC at least, you have to check operands for sNaN _before_ doing
> any floating-point operation and check the result _after_ having done
> the floating-point computation in order to set the flags.
>
> The sNaN operand exception must be raised before any computation is
> done, because it's related to the operation operands which may be lost
> during the operation if the target register is the same than an operand
> one.

I don't really understand why this is necessary to do it before *and*
after. The softmmu routines already does that job, and compute the
correct flags according to the operand. The only remaining operation for
the target specific code is to copy the softmmu flags to the target FP
status register and trigger exception if necessary.

Anyway that's not the point here, what to remember is that most target
only check operand after the softmmu routine

> The other exceptions, based on the result of the operation, must be
> raised after the result has been computed and stored.
> Then, I can see way not to check for SNaN first. And I guess this is the
> case for most FPU...
> 
>>> just have to change the check if (float64_is_signaling_nan) check with a
>>> check for NaN and handle the two cases by hand. I can see no other way
>>> to have all cases handled for all targets specific cases, do you ?
>>>
>> If you can confirm that the all PowerPC CPU are IEEE compliant and
>> should behave like the 740/750, the patch I proposed is another way to
>> be correct on all target, including PowerPC. But it seems you don't
>> really like to add target specific code in the softmmu routines.
> 
> The code you proposed is not correct for PowerPC.
> Here's what the PowerPC specification says:
> Just a few precisions before:
> * bit 0 is MSB in IBM representation
> * notation (XXX)m:n means bits from m to n (included) of XXX
> * FRB is the operand register
> * FRT is the result register
> * FPSCR is the FPU flags register
> 
> Floating point round to single-precision model:
> (just keeping the NaN parts and adding comments, hope they are
> releveant !)
> [...]
> if ((FRB) 1:11 == 2047) then
>     if ((FRB)12:63 == 0) then goto Infinity Operand /* zero mantissa */
>     if ((FRB)12 == 1) then goto QNaN Operand           /* mantissa MSB
> is one */
>    if ((FRB)12 == 0) and ((FRB)13:63 > 0) then goto SNaN Operand /*
> mantissa MSB is zero and mantissa != 0 */ 
> 
> [...]
> 
> QNaN Operand:
>   FRT <- (FRB)0:34 || 0 /* Copy the highest 35 bits of the operand to
> result */
>   (FPSCR)FPRF <- QNaN  /* Set result class to QNaN */
>   (FPSCR)FRFI <- 0b00   /* Reset result sign / zero flags */
> 
> SNaN Operand:
> 1 <- (FPSCR)VXSNAN <- 1 /* Set SNaN exception flag */
> if ((FPSCR)VE == 0) then /* SNaN exceptions are disabled */
>     (FRT)0:11 <- (FRB)0:11 /* Copy sign / exponent from the operand to
> result */
>     (FRT)12 <- 1 /* Ensure the the highest bit of mantissa is 1 */
>     (FRT)13:63 <- (FRB)13:34 || 0 /* Copy the bits 13:34 from the
> operand to result */
>     (FPSCR)FPRF <- QNaN /* set the result class to QNaN */
> (FPSCR)FRFI <- 0b00 /* Reset result sign / zero flags */
> 
> Then, as mentionned previously, there is no problem in this model,
> except the fact that it keeps more bits than the ones that would fit in
> a single-precision float in case of NaN: it keeps 35 bits of the
> operand, then this cannot be handled by the generic routine that would
> just keep 32 bits... Then I will still have to check this case at the
> same place I check for sNaN in order to always have the correct result.
> If the SNaN / QNaN representation is inverted, compared to this model,
> then yes, you have a problem with this method. But I still don't think
> adding target specific code in generic code that should not be target
> dependant is the solution. Having a define signaling that QNaN is 1/0 //
> SNaN is 0/1 would be more generic and checking that flag (and not
> TARGET_XXX) in that code would be more acceptable. But I keep thinking
> that a check on the operand is needed before computation for most CPUs,
> then this hack should not be needed...
> 

It looks like the problem is different on PowerPC, because it does not
know about single precision FP. Then I wonder why to use "double
precision to single precision conversion" softmmu routine on PowerPC.
This routine is working correctly (with the patch I submitted) for all
target, but PowerPC.

>From my point of view the best is to create a "round to single
precision" softmmu subroutine that takes a double precision for both
input and output for PowerPC.

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[J. Mayer]

On Sat, 2007-11-10 at 19:09 +0100, Aurelien Jarno wrote:
> J. Mayer a écrit :
> > On Sat, 2007-11-10 at 17:15 +0100, Aurelien Jarno wrote:
> >> J. Mayer a écrit :
> >>> On Sat, 2007-11-10 at 10:35 +0100, Aurelien Jarno wrote:
> >>>> J. Mayer a écrit :
> >>>>> On Thu, 2007-11-08 at 00:05 +0100, Aurelien Jarno wrote:
> >>>>>> On Tue, Nov 06, 2007 at 09:01:13PM +0100, J. Mayer wrote:
> >>>>>>> On Sat, 2007-11-03 at 22:28 +0100, Aurelien Jarno wrote: 
> >>>>>>>> On Sat, Nov 03, 2007 at 02:06:04PM -0400, Daniel Jacobowitz wrote:
> >>>>>>>>> On Sat, Nov 03, 2007 at 06:35:48PM +0100, Aurelien Jarno wrote:
> >>>>>>>>>> Hi all,
> >>>>>>>>>>
> >>>>>>>>>> The current softfloat implementation changes qNaN into sNaN when 
> >>>>>>>>>> converting between formats, for no reason. The attached patch fixes
> >>>>>>>>>> that. It also fixes an off-by-one in the extended double precision
> >>>>>>>>>> format (aka floatx80), the mantissa is 64-bit long and not 63-bit
> >>>>>>>>>> long.
> >>>>>>>>>>
> >>>>>>>>>> With this patch applied all the glibc 2.7 floating point tests
> >>>>>>>>>> are successfull on MIPS and MIPSEL.
> >>> [...]
> >>>>>> Anyway there is no way to do that in the target specific code *after 
> >>>>>> the conversion*, as the detection of a mantissa being nul when 
> >>>>>> converting from double to single precision can only be done when both
> >>>>>> values are still known. In other words when the value is not fixed 
> >>>>>> during the conversion, the value 0x7f800000 can either be infinity or a
> >>>>>> conversion of NaN from double to single precision, and thus is it not
> >>>>>> possible to fix the value afterwards in the target specific code.
> >>>>> I don't say you have to return an infinity when the argument is a qNaN.
> >>>>> I just say you have to return a qNaN in a generic way.  Just return sign
> >>>>> | 0x7f800000 | mantissa, which is the more generic form and seems to me
> >>>>> to even be OK for sNaNs. It's even needed for some target (not to say
> >>>> 0x7f800000 is actually not a NaN, but infinity.
> >>>>
> >>>>> PowerPC) that specify that the result have to be equal to the operand
> >>>>> (in the single precision format, of course) in such a case. This is
> >>>>> simpler, it ensures that any target could then detect the presence of a
> >>>>> NaN, know which one, and can then adjust the value according to its
> >>>>> specification if needed.
> >>>>> I then still can'tl see any reason of having target specific code in
> >>>>> that area.
> >>>> Ok, let's give an example then. On MIPS let's say you want to convert
> >>>> 0x7ff0000000000001 (qNaN) to single precision. The mantissa shifted to
> >>>> the right become 0, so you have to generate a new value. As you
> >>>> proposed, let's generate a "generic value" 0x7fc00000 in the softfloat
> >>>> routines. This value has to be converted to 0x7fbfffff in the MIPS
> >>>> target code.
> >>> OK, the values that can cause a problem is all values that would have a
> >>> zero mantissa once rounded to sinlge-precision. As the PowerPC requires
> >>> that the result would have a zero mantissa (and the result class set to
> >> Are you sure of that? According to IEEE 754 a zero mantissa is not a
> >> NaN. And tests on a real machine shows different results.
> >> 0x7ff0000000000001 is converted to 0x7fc00000 on a 740/750 CPU.
> > 
> > First, please note that a PowerPC do not have any single precision
> > register nor internal representation. The operation here is "round to
> > single precision" (frsp) but the result is still a 64 bits float. Then
> > the result is more likely to be 0x7fc0000000000000.
> > 0x7FF0000000000001 seems to be a SNaN, according to what I see in the
> > PowerPC specification. Then the result is OK: when no exception is
> > raised, SNaN is converted to QNaN during rounding to single operation
> > (please see below).
> > What about 0x7FF8000000000001, which is a QNaN ? According to the
> > PowerPC specification, this should be rounded to 0x7FF8000000000000
> > which is also a QNaN, then is also OK. Then rounding the mantissa and
> > copying sign || exponent || mantissa would, in fact, always be OK in the
> > PowerPC case.
> > What seem to appear to me now is that the problems are due to the fact
> > Mips have an inverted representation of SNaN / QNaN (if I understood
> > well) that do not allow distinction between a rounded QNaN and an
> > infinity...
> 
> Nope it is not due to the fact that MIPS uses an "inverted"
> representation. It is the same problem on x86 or other target, except
> that they can allow the distinction between a rounded SNaN and an
> infinity. The problem is present on all targets that can represent a
> single precision FP 

For those targets that can make the distinction between rounded sNaN and
infinite case, I don't see why there should be a problem.
But, I realize now, as you, that PowerPC is really not a good example
for comparison as it does not know about 32 bits floats. And I also
realize it cannot use the softmmu routines to round 64 bits floats to
single precision.

> >>> qNan), I can see no way to handle this case in the generic code. And
> >>> even adding a "#ifdef TARGET_PPC" won't solve the problem as the PowerPC
> >>> code would not be able to make the distinction between infinity case and
> >>> qNaN case. Then, the only solution, as you already mentioned, is to
> >>> check for qNaN before calling the rounding function. As the target
> >>> emulation code already has to check for sNaN to be able to raise an
> >>> exception when it's needed, checking for qNaN would cost nothing more;
> >> Except this is currently done *after* the call to the rounding function,
> >> using the flags returned by the softmmu routines. Doing a check before
> >> and after would slow down the emulation.
> > 
> > On PowerPC at least, you have to check operands for sNaN _before_ doing
> > any floating-point operation and check the result _after_ having done
> > the floating-point computation in order to set the flags.
> >
> > The sNaN operand exception must be raised before any computation is
> > done, because it's related to the operation operands which may be lost
> > during the operation if the target register is the same than an operand
> > one.
> 
> I don't really understand why this is necessary to do it before *and*
> after. The softmmu routines already does that job, and compute the
> correct flags according to the operand. The only remaining operation for
> the target specific code is to copy the softmmu flags to the target FP
> status register and trigger exception if necessary.

When using the native softfloat implementation, you don't have any flags
set then cannot raise any sNaN exception. This check allow use of the
native softfloat.
I also checked the softfloat implementation (not the native one) for
fmul and fdiv and it appears that it does not give me the needed
informations to raise the correct exceptions for the PowerPC targets.
For example, it does not seem to me that the 0 x infinity case is
properly trapped
And it does not seem to me to make a distinction between NaN operands
and invalid operations or between different "classes" of invalid
operations then do not allow a proper update of the PowerPC FPU flags.
Then, for PowerPC, checking the precise case "by hand" and using the
native softfloat implementation seems more accurate if I want to have
precise and correct results (this is sure not the case, due to all the
bugs I must have accumulated in that code !).

> Anyway that's not the point here, what to remember is that most target
> only check operand after the softmmu routine
> 
> > The other exceptions, based on the result of the operation, must be
> > raised after the result has been computed and stored.
> > Then, I can see way not to check for SNaN first. And I guess this is the
> > case for most FPU...
> > 
> >>> just have to change the check if (float64_is_signaling_nan) check with a
> >>> check for NaN and handle the two cases by hand. I can see no other way
> >>> to have all cases handled for all targets specific cases, do you ?
> >>>
> >> If you can confirm that the all PowerPC CPU are IEEE compliant and
> >> should behave like the 740/750, the patch I proposed is another way to
> >> be correct on all target, including PowerPC. But it seems you don't
> >> really like to add target specific code in the softmmu routines.
> > 
> > The code you proposed is not correct for PowerPC.
[...]

> It looks like the problem is different on PowerPC, because it does not
> know about single precision FP. Then I wonder why to use "double
> precision to single precision conversion" softmmu routine on PowerPC.
> This routine is working correctly (with the patch I submitted) for all
> target, but PowerPC.
> 
> >From my point of view the best is to create a "round to single
> precision" softmmu subroutine that takes a double precision for both
> input and output for PowerPC.

Yes, you're right, PowerPC have to use its own conversion routine. And
it seems it also needs specific routines to load and store single
precision floats to / from double precision registers.

-- 
J. Mayer <l_indien@magic.fr>
Never organized

Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[Thiemo Seufer]

Aurelien Jarno wrote:
> On Sat, Nov 03, 2007 at 02:06:04PM -0400, Daniel Jacobowitz wrote:
> > On Sat, Nov 03, 2007 at 06:35:48PM +0100, Aurelien Jarno wrote:
> > > Hi all,
> > > 
> > > The current softfloat implementation changes qNaN into sNaN when 
> > > converting between formats, for no reason. The attached patch fixes
> > > that. It also fixes an off-by-one in the extended double precision
> > > format (aka floatx80), the mantissa is 64-bit long and not 63-bit
> > > long.
> > > 
> > > With this patch applied all the glibc 2.7 floating point tests
> > > are successfull on MIPS and MIPSEL.
> > 
> > FYI, I posted a similar patch and haven't had time to get back to it.
> > Andreas reminded me that we need to make sure at least one mantissa
> > bit is set.  If we're confident that the common NaN format will
> > already have some bit other than the qnan/snan bit set, this is fine;
> > otherwise, we might want to forcibly set some other mantissa bit.
> > 
> 
> Please find an updated patch below. I have tried to match real x86, MIPS,
> HPPA, PowerPC and SPARC hardware when all mantissa bits are cleared.

The default NaN pattern for MIPS and HPPA were broken. (Fabrice spotted
this.) I currently have the appended patch, it assumes HPPA is similiar
to MIPS, which is probably incorrect.

> Index: fpu/softfloat-specialize.h
> ===================================================================
> RCS file: /sources/qemu/qemu/fpu/softfloat-specialize.h,v
> retrieving revision 1.3
> diff -u -d -p -r1.3 softfloat-specialize.h
> --- fpu/softfloat-specialize.h	11 May 2007 17:10:14 -0000	1.3
> +++ fpu/softfloat-specialize.h	3 Nov 2007 21:17:57 -0000
> @@ -120,9 +120,17 @@ static commonNaNT float32ToCommonNaN( fl
>  
>  static float32 commonNaNToFloat32( commonNaNT a )
>  {
> -
> -    return ( ( (bits32) a.sign )<<31 ) | 0x7FC00000 | ( a.high>>41 );
> -
> +    bits32 mantissa = a.high>>41;
> +    if (mantissa)
> +        return ( ( (bits32) a.sign )<<31 ) | 0x7F800000 | ( mantissa );
> +    else
> +#if defined(TARGET_MIPS)
> +        return ( ( (bits32) a.sign )<<31 ) | 0x7FBFFFFF | ( mantissa );
> +#elif defined(TARGET_HPPA)
> +        return ( ( (bits32) a.sign )<<31 ) | 0x7FA00000 | ( mantissa );
> +#else
> +        return ( ( (bits32) a.sign )<<31 ) | 0x7FC00000;
> +#endif

This looks odd. Do MIPS and HPPA really need a specialcase here but none
for doubles? Could you re-check with my patch applied?


Thiemo


Index: qemu-work/fpu/softfloat-specialize.h
===================================================================
--- qemu-work.orig/fpu/softfloat-specialize.h	2007-06-26 21:22:50.000000000 +0100
+++ qemu-work/fpu/softfloat-specialize.h	2007-11-10 19:49:37.000000000 +0000
@@ -30,6 +30,12 @@
 
 =============================================================================*/
 
+#if defined(TARGET_MIPS) || defined(TARGET_HPPA)
+#define SNAN_BIT_IS_ONE		1
+#else
+#define SNAN_BIT_IS_ONE		0
+#endif
+
 /*----------------------------------------------------------------------------
 | Underflow tininess-detection mode, statically initialized to default value.
 | (The declaration in `softfloat.h' must match the `int8' type here.)
@@ -45,9 +51,7 @@
 
 void float_raise( int8 flags STATUS_PARAM )
 {
-
     STATUS(float_exception_flags) |= flags;
-
 }
 
 /*----------------------------------------------------------------------------
@@ -61,20 +65,20 @@
 /*----------------------------------------------------------------------------
 | The pattern for a default generated single-precision NaN.
 *----------------------------------------------------------------------------*/
-#if defined(TARGET_MIPS) || defined(TARGET_HPPA)
-#define float32_default_nan 0xFF800000
+#if SNAN_BIT_IS_ONE
+#define float32_default_nan 0x7FBFFFFF
 #else
 #define float32_default_nan 0xFFC00000
 #endif
 
 /*----------------------------------------------------------------------------
-| Returns 1 if the single-precision floating-point value `a' is a NaN;
-| otherwise returns 0.
+| Returns 1 if the single-precision floating-point value `a' is a quiet
+| NaN; otherwise returns 0.
 *----------------------------------------------------------------------------*/
 
 int float32_is_nan( float32 a )
 {
-#if defined(TARGET_MIPS) || defined(TARGET_HPPA)
+#if SNAN_BIT_IS_ONE
     return ( ( ( a>>22 ) & 0x1FF ) == 0x1FE ) && ( a & 0x003FFFFF );
 #else
     return ( 0xFF800000 <= (bits32) ( a<<1 ) );
@@ -88,7 +92,7 @@
 
 int float32_is_signaling_nan( float32 a )
 {
-#if defined(TARGET_MIPS) || defined(TARGET_HPPA)
+#if SNAN_BIT_IS_ONE
     return ( 0xFF800000 <= (bits32) ( a<<1 ) );
 #else
     return ( ( ( a>>22 ) & 0x1FF ) == 0x1FE ) && ( a & 0x003FFFFF );
@@ -110,7 +114,6 @@
     z.low = 0;
     z.high = ( (bits64) a )<<41;
     return z;
-
 }
 
 /*----------------------------------------------------------------------------
@@ -120,9 +123,7 @@
 
 static float32 commonNaNToFloat32( commonNaNT a )
 {
-
     return ( ( (bits32) a.sign )<<31 ) | 0x7FC00000 | ( a.high>>41 );
-
 }
 
 /*----------------------------------------------------------------------------
@@ -139,7 +140,7 @@
     aIsSignalingNaN = float32_is_signaling_nan( a );
     bIsNaN = float32_is_nan( b );
     bIsSignalingNaN = float32_is_signaling_nan( b );
-#if defined(TARGET_MIPS) || defined(TARGET_HPPA)
+#if SNAN_BIT_IS_ONE
     a &= ~0x00400000;
     b &= ~0x00400000;
 #else
@@ -161,26 +162,25 @@
     else {
         return b;
     }
-
 }
 
 /*----------------------------------------------------------------------------
 | The pattern for a default generated double-precision NaN.
 *----------------------------------------------------------------------------*/
-#if defined(TARGET_MIPS) || defined(TARGET_HPPA)
-#define float64_default_nan LIT64( 0xFFF0000000000000 )
+#if SNAN_BIT_IS_ONE
+#define float64_default_nan LIT64( 0x7FF7FFFFFFFFFFFF )
 #else
 #define float64_default_nan LIT64( 0xFFF8000000000000 )
 #endif
 
 /*----------------------------------------------------------------------------
-| Returns 1 if the double-precision floating-point value `a' is a NaN;
-| otherwise returns 0.
+| Returns 1 if the double-precision floating-point value `a' is a quiet
+| NaN; otherwise returns 0.
 *----------------------------------------------------------------------------*/
 
 int float64_is_nan( float64 a )
 {
-#if defined(TARGET_MIPS) || defined(TARGET_HPPA)
+#if SNAN_BIT_IS_ONE
     return
            ( ( ( a>>51 ) & 0xFFF ) == 0xFFE )
         && ( a & LIT64( 0x0007FFFFFFFFFFFF ) );
@@ -196,7 +196,7 @@
 
 int float64_is_signaling_nan( float64 a )
 {
-#if defined(TARGET_MIPS) || defined(TARGET_HPPA)
+#if SNAN_BIT_IS_ONE
     return ( LIT64( 0xFFF0000000000000 ) <= (bits64) ( a<<1 ) );
 #else
     return
@@ -220,7 +220,6 @@
     z.low = 0;
     z.high = a<<12;
     return z;
-
 }
 
 /*----------------------------------------------------------------------------
@@ -230,12 +229,10 @@
 
 static float64 commonNaNToFloat64( commonNaNT a )
 {
-
     return
           ( ( (bits64) a.sign )<<63 )
         | LIT64( 0x7FF8000000000000 )
         | ( a.high>>12 );
-
 }
 
 /*----------------------------------------------------------------------------
@@ -252,7 +249,7 @@
     aIsSignalingNaN = float64_is_signaling_nan( a );
     bIsNaN = float64_is_nan( b );
     bIsSignalingNaN = float64_is_signaling_nan( b );
-#if defined(TARGET_MIPS) || defined(TARGET_HPPA)
+#if SNAN_BIT_IS_ONE
     a &= ~LIT64( 0x0008000000000000 );
     b &= ~LIT64( 0x0008000000000000 );
 #else
@@ -274,7 +271,6 @@
     else {
         return b;
     }
-
 }
 
 #ifdef FLOATX80
@@ -284,19 +280,32 @@
 | `high' and `low' values hold the most- and least-significant bits,
 | respectively.
 *----------------------------------------------------------------------------*/
+#if SNAN_BIT_IS_ONE
+#define floatx80_default_nan_high 0x7FFF
+#define floatx80_default_nan_low  LIT64( 0xBFFFFFFFFFFFFFFF )
+#else
 #define floatx80_default_nan_high 0xFFFF
 #define floatx80_default_nan_low  LIT64( 0xC000000000000000 )
+#endif
 
 /*----------------------------------------------------------------------------
 | Returns 1 if the extended double-precision floating-point value `a' is a
-| NaN; otherwise returns 0.
+| quiet NaN; otherwise returns 0.
 *----------------------------------------------------------------------------*/
 
 int floatx80_is_nan( floatx80 a )
 {
+#if SNAN_BIT_IS_ONE
+    bits64 aLow;
 
+    aLow = a.low & ~ LIT64( 0x4000000000000000 );
+    return
+           ( ( a.high & 0x7FFF ) == 0x7FFF )
+        && (bits64) ( aLow<<1 )
+        && ( a.low == aLow );
+#else
     return ( ( a.high & 0x7FFF ) == 0x7FFF ) && (bits64) ( a.low<<1 );
-
+#endif
 }
 
 /*----------------------------------------------------------------------------
@@ -306,6 +315,9 @@
 
 int floatx80_is_signaling_nan( floatx80 a )
 {
+#if SNAN_BIT_IS_ONE
+    return ( ( a.high & 0x7FFF ) == 0x7FFF ) && (bits64) ( a.low<<1 );
+#else
     bits64 aLow;
 
     aLow = a.low & ~ LIT64( 0x4000000000000000 );
@@ -313,7 +325,7 @@
            ( ( a.high & 0x7FFF ) == 0x7FFF )
         && (bits64) ( aLow<<1 )
         && ( a.low == aLow );
-
+#endif
 }
 
 /*----------------------------------------------------------------------------
@@ -331,7 +343,6 @@
     z.low = 0;
     z.high = a.low<<1;
     return z;
-
 }
 
 /*----------------------------------------------------------------------------
@@ -346,7 +357,6 @@
     z.low = LIT64( 0xC000000000000000 ) | ( a.high>>1 );
     z.high = ( ( (bits16) a.sign )<<15 ) | 0x7FFF;
     return z;
-
 }
 
 /*----------------------------------------------------------------------------
@@ -363,8 +373,13 @@
     aIsSignalingNaN = floatx80_is_signaling_nan( a );
     bIsNaN = floatx80_is_nan( b );
     bIsSignalingNaN = floatx80_is_signaling_nan( b );
+#if SNAN_BIT_IS_ONE
+    a.low &= ~LIT64( 0xC000000000000000 );
+    b.low &= ~LIT64( 0xC000000000000000 );
+#else
     a.low |= LIT64( 0xC000000000000000 );
     b.low |= LIT64( 0xC000000000000000 );
+#endif
     if ( aIsSignalingNaN | bIsSignalingNaN ) float_raise( float_flag_invalid STATUS_VAR);
     if ( aIsSignalingNaN ) {
         if ( bIsSignalingNaN ) goto returnLargerSignificand;
@@ -380,7 +395,6 @@
     else {
         return b;
     }
-
 }
 
 #endif
@@ -391,21 +405,30 @@
 | The pattern for a default generated quadruple-precision NaN.  The `high' and
 | `low' values hold the most- and least-significant bits, respectively.
 *----------------------------------------------------------------------------*/
+#if SNAN_BIT_IS_ONE
+#define float128_default_nan_high LIT64( 0x7FFF7FFFFFFFFFFF )
+#define float128_default_nan_low  LIT64( 0xFFFFFFFFFFFFFFFF )
+#else
 #define float128_default_nan_high LIT64( 0xFFFF800000000000 )
 #define float128_default_nan_low  LIT64( 0x0000000000000000 )
+#endif
 
 /*----------------------------------------------------------------------------
-| Returns 1 if the quadruple-precision floating-point value `a' is a NaN;
-| otherwise returns 0.
+| Returns 1 if the quadruple-precision floating-point value `a' is a quiet
+| NaN; otherwise returns 0.
 *----------------------------------------------------------------------------*/
 
 int float128_is_nan( float128 a )
 {
-
+#if SNAN_BIT_IS_ONE
+    return
+           ( ( ( a.high>>47 ) & 0xFFFF ) == 0xFFFE )
+        && ( a.low || ( a.high & LIT64( 0x00007FFFFFFFFFFF ) ) );
+#else
     return
            ( LIT64( 0xFFFE000000000000 ) <= (bits64) ( a.high<<1 ) )
         && ( a.low || ( a.high & LIT64( 0x0000FFFFFFFFFFFF ) ) );
-
+#endif
 }
 
 /*----------------------------------------------------------------------------
@@ -415,11 +438,15 @@
 
 int float128_is_signaling_nan( float128 a )
 {
-
+#if SNAN_BIT_IS_ONE
+    return
+           ( LIT64( 0xFFFE000000000000 ) <= (bits64) ( a.high<<1 ) )
+        && ( a.low || ( a.high & LIT64( 0x0000FFFFFFFFFFFF ) ) );
+#else
     return
            ( ( ( a.high>>47 ) & 0xFFFF ) == 0xFFFE )
         && ( a.low || ( a.high & LIT64( 0x00007FFFFFFFFFFF ) ) );
-
+#endif
 }
 
 /*----------------------------------------------------------------------------
@@ -436,7 +463,6 @@
     z.sign = a.high>>63;
     shortShift128Left( a.high, a.low, 16, &z.high, &z.low );
     return z;
-
 }
 
 /*----------------------------------------------------------------------------
@@ -451,7 +477,6 @@
     shift128Right( a.high, a.low, 16, &z.high, &z.low );
     z.high |= ( ( (bits64) a.sign )<<63 ) | LIT64( 0x7FFF800000000000 );
     return z;
-
 }
 
 /*----------------------------------------------------------------------------
@@ -468,8 +493,13 @@
     aIsSignalingNaN = float128_is_signaling_nan( a );
     bIsNaN = float128_is_nan( b );
     bIsSignalingNaN = float128_is_signaling_nan( b );
+#if SNAN_BIT_IS_ONE
+    a.high &= ~LIT64( 0x0000800000000000 );
+    b.high &= ~LIT64( 0x0000800000000000 );
+#else
     a.high |= LIT64( 0x0000800000000000 );
     b.high |= LIT64( 0x0000800000000000 );
+#endif
     if ( aIsSignalingNaN | bIsSignalingNaN ) float_raise( float_flag_invalid STATUS_VAR);
     if ( aIsSignalingNaN ) {
         if ( bIsSignalingNaN ) goto returnLargerSignificand;
@@ -485,8 +515,6 @@
     else {
         return b;
     }
-
 }
 
 #endif
-

Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[Aurelien Jarno]

Thiemo Seufer a écrit :
> Aurelien Jarno wrote:
>> On Sat, Nov 03, 2007 at 02:06:04PM -0400, Daniel Jacobowitz wrote:
>>> On Sat, Nov 03, 2007 at 06:35:48PM +0100, Aurelien Jarno wrote:
>>>> Hi all,
>>>>
>>>> The current softfloat implementation changes qNaN into sNaN when 
>>>> converting between formats, for no reason. The attached patch fixes
>>>> that. It also fixes an off-by-one in the extended double precision
>>>> format (aka floatx80), the mantissa is 64-bit long and not 63-bit
>>>> long.
>>>>
>>>> With this patch applied all the glibc 2.7 floating point tests
>>>> are successfull on MIPS and MIPSEL.
>>> FYI, I posted a similar patch and haven't had time to get back to it.
>>> Andreas reminded me that we need to make sure at least one mantissa
>>> bit is set.  If we're confident that the common NaN format will
>>> already have some bit other than the qnan/snan bit set, this is fine;
>>> otherwise, we might want to forcibly set some other mantissa bit.
>>>
>> Please find an updated patch below. I have tried to match real x86, MIPS,
>> HPPA, PowerPC and SPARC hardware when all mantissa bits are cleared.
> 
> The default NaN pattern for MIPS and HPPA were broken. (Fabrice spotted
> this.) I currently have the appended patch, it assumes HPPA is similiar
> to MIPS, which is probably incorrect.

HPPA only generates different NaN values, appart from that it behaves
the same as MIPS. On the other side, target-hppa is not yet implemented
(even if some patches could be found on the web).

>> Index: fpu/softfloat-specialize.h
>> ===================================================================
>> RCS file: /sources/qemu/qemu/fpu/softfloat-specialize.h,v
>> retrieving revision 1.3
>> diff -u -d -p -r1.3 softfloat-specialize.h
>> --- fpu/softfloat-specialize.h	11 May 2007 17:10:14 -0000	1.3
>> +++ fpu/softfloat-specialize.h	3 Nov 2007 21:17:57 -0000
>> @@ -120,9 +120,17 @@ static commonNaNT float32ToCommonNaN( fl
>>  
>>  static float32 commonNaNToFloat32( commonNaNT a )
>>  {
>> -
>> -    return ( ( (bits32) a.sign )<<31 ) | 0x7FC00000 | ( a.high>>41 );
>> -
>> +    bits32 mantissa = a.high>>41;
>> +    if (mantissa)
>> +        return ( ( (bits32) a.sign )<<31 ) | 0x7F800000 | ( mantissa );
>> +    else
>> +#if defined(TARGET_MIPS)
>> +        return ( ( (bits32) a.sign )<<31 ) | 0x7FBFFFFF | ( mantissa );
>> +#elif defined(TARGET_HPPA)
>> +        return ( ( (bits32) a.sign )<<31 ) | 0x7FA00000 | ( mantissa );
>> +#else
>> +        return ( ( (bits32) a.sign )<<31 ) | 0x7FC00000;
>> +#endif
> 
> This looks odd. Do MIPS and HPPA really need a specialcase here but none
> for doubles? Could you re-check with my patch applied?

That's because there is no greater precision than double precision on
MIPS and HPPA (for the latter I am not 100% sure), so you can't loose
precision on the mantissa when converting to double precision.

I have looked quickly at your patch, it looks like the right approach to
define target specific things at only one place in the file. I will get
a closer tomorrow.

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[Aurelien Jarno]

> I have looked quickly at your patch, it looks like the right approach to
> define target specific things at only one place in the file. I will get
> a closer tomorrow.
> 

Sorry I have been busy with real life stuff, and haven't looked at your
patch. I have seen you have committed it.

I have rebased the patch I already posted against the current CVS, it is
less intrusive now. It defines float32_default_nan and 
float64_default_nan for the various targets, as it varies even within 
targets which have the same convention for the SNAN bit.

It also keeps the mantissa when converting FP numbers, so that qNaN are
not changed into sNaN on MIPS and HPPA. If the mantissa becames zero a
new NaN number is generated.

It finally fixes an off-by-one in the extended double precision
format (aka floatx80), the mantissa is 64-bit long and not 63-bit
long.

With this patch applied all the glibc 2.7 floating point tests
are successfull on MIPS and MIPSEL.


Index: fpu/softfloat-specialize.h
===================================================================
RCS file: /sources/qemu/qemu/fpu/softfloat-specialize.h,v
retrieving revision 1.5
diff -u -d -p -r1.5 softfloat-specialize.h
--- fpu/softfloat-specialize.h	18 Nov 2007 14:33:23 -0000	1.5
+++ fpu/softfloat-specialize.h	20 Nov 2007 23:18:44 -0000
@@ -65,7 +65,13 @@ typedef struct {
 /*----------------------------------------------------------------------------
 | The pattern for a default generated single-precision NaN.
 *----------------------------------------------------------------------------*/
-#if SNAN_BIT_IS_ONE
+#if defined(TARGET_SPARC)
+#define float32_default_nan make_float32(0x7FFFFFFF)
+#elif defined(TARGET_POWERPC)
+#define float32_default_nan make_float32(0x7FC00000)
+#elif defined(TARGET_HPPA)
+#define float32_default_nan make_float32(0x7FA00000)
+#elif SNAN_BIT_IS_ONE
 #define float32_default_nan make_float32(0x7FBFFFFF)
 #else
 #define float32_default_nan make_float32(0xFFC00000)
@@ -125,8 +131,12 @@ static commonNaNT float32ToCommonNaN( fl
 
 static float32 commonNaNToFloat32( commonNaNT a )
 {
-    return make_float32(
-        ( ( (bits32) a.sign )<<31 ) | 0x7FC00000 | ( a.high>>41 ) );
+    bits32 mantissa = a.high>>41;
+    if ( mantissa )
+        return make_float32(
+            ( ( (bits32) a.sign )<<31 ) | 0x7F800000 | ( a.high>>41 ) );
+    else
+        return float32_default_nan;
 }
 
 /*----------------------------------------------------------------------------
@@ -180,7 +190,13 @@ static float32 propagateFloat32NaN( floa
 /*----------------------------------------------------------------------------
 | The pattern for a default generated double-precision NaN.
 *----------------------------------------------------------------------------*/
-#if SNAN_BIT_IS_ONE
+#if defined(TARGET_SPARC)
+#define float64_default_nan make_float64(LIT64( 0x7FFFFFFFFFFFFFFF ))
+#elif defined(TARGET_POWERPC)
+#define float64_default_nan make_float64(LIT64( 0x7FF8000000000000 ))
+#elif defined(TARGET_HPPA)
+#define float64_default_nan make_float64(LIT64( 0x7FF4000000000000 ))
+#elif SNAN_BIT_IS_ONE
 #define float64_default_nan make_float64(LIT64( 0x7FF7FFFFFFFFFFFF ))
 #else
 #define float64_default_nan make_float64(LIT64( 0xFFF8000000000000 ))
@@ -244,10 +260,15 @@ static commonNaNT float64ToCommonNaN( fl
 
 static float64 commonNaNToFloat64( commonNaNT a )
 {
-    return make_float64(
-          ( ( (bits64) a.sign )<<63 )
-        | LIT64( 0x7FF8000000000000 )
-        | ( a.high>>12 ));
+    bits64 mantissa = a.high>>12;
+
+    if ( mantissa )
+        return make_float64(
+              ( ( (bits64) a.sign )<<63 )
+            | LIT64( 0x7FF0000000000000 )
+            | ( a.high>>12 ));
+    else
+        return float64_default_nan;
 }
 
 /*----------------------------------------------------------------------------
@@ -366,7 +387,7 @@ static commonNaNT floatx80ToCommonNaN( f
     if ( floatx80_is_signaling_nan( a ) ) float_raise( float_flag_invalid STATUS_VAR);
     z.sign = a.high>>15;
     z.low = 0;
-    z.high = a.low<<1;
+    z.high = a.low;
     return z;
 }
 
@@ -379,7 +400,10 @@ static floatx80 commonNaNToFloatx80( com
 {
     floatx80 z;
 
-    z.low = LIT64( 0xC000000000000000 ) | ( a.high>>1 );
+    if (a.high)
+        z.low = a.high;
+    else
+        z.low = floatx80_default_nan_low;
     z.high = ( ( (bits16) a.sign )<<15 ) | 0x7FFF;
     return z;
 }
@@ -500,7 +524,7 @@ static float128 commonNaNToFloat128( com
     float128 z;
 
     shift128Right( a.high, a.low, 16, &z.high, &z.low );
-    z.high |= ( ( (bits64) a.sign )<<63 ) | LIT64( 0x7FFF800000000000 );
+    z.high |= ( ( (bits64) a.sign )<<63 ) | LIT64( 0x7FFF000000000000 );
     return z;
 }
 

-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[Aurelien Jarno]

Aurelien Jarno a écrit :
>> I have looked quickly at your patch, it looks like the right approach to
>> define target specific things at only one place in the file. I will get
>> a closer tomorrow.
>>
> 
> Sorry I have been busy with real life stuff, and haven't looked at your
> patch. I have seen you have committed it.
> 
> I have rebased the patch I already posted against the current CVS, it is
> less intrusive now. It defines float32_default_nan and 
> float64_default_nan for the various targets, as it varies even within 
> targets which have the same convention for the SNAN bit.
> 
> It also keeps the mantissa when converting FP numbers, so that qNaN are
> not changed into sNaN on MIPS and HPPA. If the mantissa becames zero a
> new NaN number is generated.
> 
> It finally fixes an off-by-one in the extended double precision
> format (aka floatx80), the mantissa is 64-bit long and not 63-bit
> long.
> 
> With this patch applied all the glibc 2.7 floating point tests
> are successfull on MIPS and MIPSEL.


Any news about this patch?



> Index: fpu/softfloat-specialize.h
> ===================================================================
> RCS file: /sources/qemu/qemu/fpu/softfloat-specialize.h,v
> retrieving revision 1.5
> diff -u -d -p -r1.5 softfloat-specialize.h
> --- fpu/softfloat-specialize.h	18 Nov 2007 14:33:23 -0000	1.5
> +++ fpu/softfloat-specialize.h	20 Nov 2007 23:18:44 -0000
> @@ -65,7 +65,13 @@ typedef struct {
>  /*----------------------------------------------------------------------------
>  | The pattern for a default generated single-precision NaN.
>  *----------------------------------------------------------------------------*/
> -#if SNAN_BIT_IS_ONE
> +#if defined(TARGET_SPARC)
> +#define float32_default_nan make_float32(0x7FFFFFFF)
> +#elif defined(TARGET_POWERPC)
> +#define float32_default_nan make_float32(0x7FC00000)
> +#elif defined(TARGET_HPPA)
> +#define float32_default_nan make_float32(0x7FA00000)
> +#elif SNAN_BIT_IS_ONE
>  #define float32_default_nan make_float32(0x7FBFFFFF)
>  #else
>  #define float32_default_nan make_float32(0xFFC00000)
> @@ -125,8 +131,12 @@ static commonNaNT float32ToCommonNaN( fl
>  
>  static float32 commonNaNToFloat32( commonNaNT a )
>  {
> -    return make_float32(
> -        ( ( (bits32) a.sign )<<31 ) | 0x7FC00000 | ( a.high>>41 ) );
> +    bits32 mantissa = a.high>>41;
> +    if ( mantissa )
> +        return make_float32(
> +            ( ( (bits32) a.sign )<<31 ) | 0x7F800000 | ( a.high>>41 ) );
> +    else
> +        return float32_default_nan;
>  }
>  
>  /*----------------------------------------------------------------------------
> @@ -180,7 +190,13 @@ static float32 propagateFloat32NaN( floa
>  /*----------------------------------------------------------------------------
>  | The pattern for a default generated double-precision NaN.
>  *----------------------------------------------------------------------------*/
> -#if SNAN_BIT_IS_ONE
> +#if defined(TARGET_SPARC)
> +#define float64_default_nan make_float64(LIT64( 0x7FFFFFFFFFFFFFFF ))
> +#elif defined(TARGET_POWERPC)
> +#define float64_default_nan make_float64(LIT64( 0x7FF8000000000000 ))
> +#elif defined(TARGET_HPPA)
> +#define float64_default_nan make_float64(LIT64( 0x7FF4000000000000 ))
> +#elif SNAN_BIT_IS_ONE
>  #define float64_default_nan make_float64(LIT64( 0x7FF7FFFFFFFFFFFF ))
>  #else
>  #define float64_default_nan make_float64(LIT64( 0xFFF8000000000000 ))
> @@ -244,10 +260,15 @@ static commonNaNT float64ToCommonNaN( fl
>  
>  static float64 commonNaNToFloat64( commonNaNT a )
>  {
> -    return make_float64(
> -          ( ( (bits64) a.sign )<<63 )
> -        | LIT64( 0x7FF8000000000000 )
> -        | ( a.high>>12 ));
> +    bits64 mantissa = a.high>>12;
> +
> +    if ( mantissa )
> +        return make_float64(
> +              ( ( (bits64) a.sign )<<63 )
> +            | LIT64( 0x7FF0000000000000 )
> +            | ( a.high>>12 ));
> +    else
> +        return float64_default_nan;
>  }
>  
>  /*----------------------------------------------------------------------------
> @@ -366,7 +387,7 @@ static commonNaNT floatx80ToCommonNaN( f
>      if ( floatx80_is_signaling_nan( a ) ) float_raise( float_flag_invalid STATUS_VAR);
>      z.sign = a.high>>15;
>      z.low = 0;
> -    z.high = a.low<<1;
> +    z.high = a.low;
>      return z;
>  }
>  
> @@ -379,7 +400,10 @@ static floatx80 commonNaNToFloatx80( com
>  {
>      floatx80 z;
>  
> -    z.low = LIT64( 0xC000000000000000 ) | ( a.high>>1 );
> +    if (a.high)
> +        z.low = a.high;
> +    else
> +        z.low = floatx80_default_nan_low;
>      z.high = ( ( (bits16) a.sign )<<15 ) | 0x7FFF;
>      return z;
>  }
> @@ -500,7 +524,7 @@ static float128 commonNaNToFloat128( com
>      float128 z;
>  
>      shift128Right( a.high, a.low, 16, &z.high, &z.low );
> -    z.high |= ( ( (bits64) a.sign )<<63 ) | LIT64( 0x7FFF800000000000 );
> +    z.high |= ( ( (bits64) a.sign )<<63 ) | LIT64( 0x7FFF000000000000 );
>      return z;
>  }
>  
> 


-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net

Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[Thiemo Seufer]

Aurelien Jarno wrote:
> Hi all,
> 
> The current softfloat implementation changes qNaN into sNaN when 
> converting between formats, for no reason. The attached patch fixes
> that.

Did you take into account that MIPS and PA-RISC have the signalling
bit inverted to the rest of the world?

> It also fixes an off-by-one in the extended double precision
> format (aka floatx80), the mantissa is 64-bit long and not 63-bit
> long.
> 
> With this patch applied all the glibc 2.7 floating point tests
> are successfull on MIPS and MIPSEL.
> 
> Bye,
> Aurelien
> 
> Index: fpu/softfloat-specialize.h
> ===================================================================
> RCS file: /sources/qemu/qemu/fpu/softfloat-specialize.h,v
> retrieving revision 1.3
> diff -u -d -p -r1.3 softfloat-specialize.h
> --- fpu/softfloat-specialize.h	11 May 2007 17:10:14 -0000	1.3
> +++ fpu/softfloat-specialize.h	3 Nov 2007 17:21:38 -0000
> @@ -121,8 +121,7 @@ static commonNaNT float32ToCommonNaN( fl
>  static float32 commonNaNToFloat32( commonNaNT a )
>  {
>  
> -    return ( ( (bits32) a.sign )<<31 ) | 0x7FC00000 | ( a.high>>41 );
> -
> +    return ( ( (bits32) a.sign )<<31 ) | 0x7F800000 | ( a.high>>41 );
>  }
>  
>  /*----------------------------------------------------------------------------
> @@ -233,7 +232,7 @@ static float64 commonNaNToFloat64( commo
>  
>      return
>            ( ( (bits64) a.sign )<<63 )
> -        | LIT64( 0x7FF8000000000000 )
> +        | LIT64( 0x7FF0000000000000 )
>          | ( a.high>>12 );
>  
>  }
> @@ -329,7 +328,7 @@ static commonNaNT floatx80ToCommonNaN( f
>      if ( floatx80_is_signaling_nan( a ) ) float_raise( float_flag_invalid STATUS_VAR);
>      z.sign = a.high>>15;
>      z.low = 0;
> -    z.high = a.low<<1;
> +    z.high = a.low;
>      return z;
>  
>  }
> @@ -343,7 +342,7 @@ static floatx80 commonNaNToFloatx80( com
>  {
>      floatx80 z;
>  
> -    z.low = LIT64( 0xC000000000000000 ) | ( a.high>>1 );
> +    z.low = a.high;
>      z.high = ( ( (bits16) a.sign )<<15 ) | 0x7FFF;
>      return z;
>  
> @@ -449,7 +448,7 @@ static float128 commonNaNToFloat128( com
>      float128 z;
>  
>      shift128Right( a.high, a.low, 16, &z.high, &z.low );
> -    z.high |= ( ( (bits64) a.sign )<<63 ) | LIT64( 0x7FFF800000000000 );
> +    z.high |= ( ( (bits64) a.sign )<<63 ) | LIT64( 0x7FFF000000000000 );
>      return z;
>  
>  }
> 
> -- 
>   .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
>  : :' :  Debian developer           | Electrical Engineer
>  `. `'   aurel32@debian.org         | aurelien@aurel32.net
>    `-    people.debian.org/~aurel32 | www.aurel32.net
> 
> 
> 

Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat

[Aurelien Jarno]

Thiemo Seufer a écrit :
> Aurelien Jarno wrote:
>> Hi all,
>>
>> The current softfloat implementation changes qNaN into sNaN when 
>> converting between formats, for no reason. The attached patch fixes
>> that.
> 
> Did you take into account that MIPS and PA-RISC have the signalling
> bit inverted to the rest of the world?

Oh didn't know about that! Anyway that doesn't change anything, as the
patch simply copy parts of the mantissa from one format to another.

That just means on other architectures, sNaN are changed into qNaN.


-- 
  .''`.  Aurelien Jarno	            | GPG: 1024D/F1BCDB73
 : :' :  Debian developer           | Electrical Engineer
 `. `'   aurel32@debian.org         | aurelien@aurel32.net
   `-    people.debian.org/~aurel32 | www.aurel32.net