Date: Sat, 3 Nov 2007 22:28:16 +0100
From: Aurelien Jarno <aurelien@aurel32.net>
Subject: Re: [Qemu-devel] [PATCH] Fix NaN handling in softfloat
Message-ID: <20071103212816.GA31686@hall.aurel32.net>
References: <20071103173548.GA16847@hall.aurel32.net>
	<20071103180604.GA14403@caradoc.them.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-15
Content-Disposition: inline
In-Reply-To: <20071103180604.GA14403@caradoc.them.org>
Sender: "aurel32,,," <aurel32@hall.aurel32.net>
Reply-To: qemu-devel@nongnu.org
To: qemu-devel@nongnu.org

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