[Qemu-devel] [PATCH] Softfloat: Add support to softfloat to return floatxx_default_nan when the corresponding target status flag is set.

[Qemu-devel] [PATCH] Softfloat: Add support to softfloat to return floatxx_default_nan when the corresponding target status flag is set.

[Christophe Lyon]

Some CPUs have a status flag imposing to return floatxx_default_nan
whatever the input value, when converting from one FP format to
another. Implement this, using the already existing default_nan_mode
status flag, only for ARM at the moment, though other architectures
may have the same feature. This patch only modifies the
commonNaNToFloat32 and commonNaNToFloat64 conversion functions, as ARM
only uses these.

Signed-off-by: Christophe Lyon <christophe.lyon@st.com>
---
checkpatch complains in this patch, but I have kept the style already present in the involved files.

 fpu/softfloat-specialize.h |   15 +++++++++++++--
 fpu/softfloat.c            |   12 ++++++------
 2 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h
index 11521ce..1a618bd 100644
--- a/fpu/softfloat-specialize.h
+++ b/fpu/softfloat-specialize.h
@@ -144,9 +144,15 @@ static commonNaNT float32ToCommonNaN( float32 a STATUS_PARAM )
 | precision floating-point format.
 *----------------------------------------------------------------------------*/
 
-static float32 commonNaNToFloat32( commonNaNT a )
+static float32 commonNaNToFloat32( commonNaNT a STATUS_PARAM )
 {
     bits32 mantissa = a.high>>41;
+
+#if defined(TARGET_ARM)
+    if ( STATUS(default_nan_mode) ) {
+        return float32_default_nan;
+    }
+#endif
     if ( mantissa )
         return make_float32(
             ( ( (bits32) a.sign )<<31 ) | 0x7F800000 | ( a.high>>41 ) );
@@ -398,10 +404,15 @@ static commonNaNT float64ToCommonNaN( float64 a STATUS_PARAM)
 | precision floating-point format.
 *----------------------------------------------------------------------------*/
 
-static float64 commonNaNToFloat64( commonNaNT a )
+static float64 commonNaNToFloat64( commonNaNT a STATUS_PARAM )
 {
     bits64 mantissa = a.high>>12;
 
+#if defined(TARGET_ARM)
+    if ( STATUS(default_nan_mode) ) {
+        return float64_default_nan;
+    }
+#endif
     if ( mantissa )
         return make_float64(
               ( ( (bits64) a.sign )<<63 )
diff --git a/fpu/softfloat.c b/fpu/softfloat.c
index 17842f4..4674d37 100644
--- a/fpu/softfloat.c
+++ b/fpu/softfloat.c
@@ -1534,7 +1534,7 @@ float64 float32_to_float64( float32 a STATUS_PARAM )
     aExp = extractFloat32Exp( a );
     aSign = extractFloat32Sign( a );
     if ( aExp == 0xFF ) {
-        if ( aSig ) return commonNaNToFloat64( float32ToCommonNaN( a STATUS_VAR ));
+        if ( aSig ) return commonNaNToFloat64( float32ToCommonNaN( a STATUS_VAR ) STATUS_VAR );
         return packFloat64( aSign, 0x7FF, 0 );
     }
     if ( aExp == 0 ) {
@@ -2689,7 +2689,7 @@ float32 float64_to_float32( float64 a STATUS_PARAM )
     aExp = extractFloat64Exp( a );
     aSign = extractFloat64Sign( a );
     if ( aExp == 0x7FF ) {
-        if ( aSig ) return commonNaNToFloat32( float64ToCommonNaN( a STATUS_VAR ) );
+        if ( aSig ) return commonNaNToFloat32( float64ToCommonNaN( a STATUS_VAR ) STATUS_VAR );
         return packFloat32( aSign, 0xFF, 0 );
     }
     shift64RightJamming( aSig, 22, &aSig );
@@ -3843,7 +3843,7 @@ float32 floatx80_to_float32( floatx80 a STATUS_PARAM )
     aSign = extractFloatx80Sign( a );
     if ( aExp == 0x7FFF ) {
         if ( (bits64) ( aSig<<1 ) ) {
-            return commonNaNToFloat32( floatx80ToCommonNaN( a STATUS_VAR ) );
+            return commonNaNToFloat32( floatx80ToCommonNaN( a STATUS_VAR ) STATUS_VAR );
         }
         return packFloat32( aSign, 0xFF, 0 );
     }
@@ -3871,7 +3871,7 @@ float64 floatx80_to_float64( floatx80 a STATUS_PARAM )
     aSign = extractFloatx80Sign( a );
     if ( aExp == 0x7FFF ) {
         if ( (bits64) ( aSig<<1 ) ) {
-            return commonNaNToFloat64( floatx80ToCommonNaN( a STATUS_VAR ) );
+            return commonNaNToFloat64( floatx80ToCommonNaN( a STATUS_VAR ) STATUS_VAR );
         }
         return packFloat64( aSign, 0x7FF, 0 );
     }
@@ -4863,7 +4863,7 @@ float32 float128_to_float32( float128 a STATUS_PARAM )
     aSign = extractFloat128Sign( a );
     if ( aExp == 0x7FFF ) {
         if ( aSig0 | aSig1 ) {
-            return commonNaNToFloat32( float128ToCommonNaN( a STATUS_VAR ) );
+            return commonNaNToFloat32( float128ToCommonNaN( a STATUS_VAR ) STATUS_VAR );
         }
         return packFloat32( aSign, 0xFF, 0 );
     }
@@ -4897,7 +4897,7 @@ float64 float128_to_float64( float128 a STATUS_PARAM )
     aSign = extractFloat128Sign( a );
     if ( aExp == 0x7FFF ) {
         if ( aSig0 | aSig1 ) {
-            return commonNaNToFloat64( float128ToCommonNaN( a STATUS_VAR ) );
+            return commonNaNToFloat64( float128ToCommonNaN( a STATUS_VAR ) STATUS_VAR );
         }
         return packFloat64( aSign, 0x7FF, 0 );
     }
-- 
1.7.2.3

Re: [Qemu-devel] [PATCH] Softfloat: Add support to softfloat to return floatxx_default_nan when the corresponding target status flag is set.

[Peter Maydell]

On 4 February 2011 14:01, Christophe Lyon <christophe.lyon@st.com> wrote:
> Some CPUs have a status flag imposing to return floatxx_default_nan
> whatever the input value, when converting from one FP format to
> another. Implement this, using the already existing default_nan_mode
> status flag, only for ARM at the moment, though other architectures
> may have the same feature. This patch only modifies the
> commonNaNToFloat32 and commonNaNToFloat64 conversion functions, as ARM
> only uses these.

> -static float32 commonNaNToFloat32( commonNaNT a )
> +static float32 commonNaNToFloat32( commonNaNT a STATUS_PARAM )
>  {
>     bits32 mantissa = a.high>>41;
> +
> +#if defined(TARGET_ARM)
> +    if ( STATUS(default_nan_mode) ) {
> +        return float32_default_nan;
> +    }
> +#endif

The target-specific #ifdef is pretty ugly.
Fortunately the only architecture except ARM that sets
default_nan_mode is sh4, so all we have to do is find out
what it does with its float-to-float conversions. It looks
to me from
http://documentation.renesas.com/eng/products/mpumcu/rej09b0318_sh_4sm.pdf
that it also returns the default NaN for converting a NaN
from one float format to another. So you can just drop the
#ifdef.

(cc'ing Aurelien to check since he's the SH4 maintainer.)

I also think the change is simple enough that we ought to do
it consistently for the floatx80 and float128 functions even if
neither ARM nor sh4 use them.

-- PMM

Re: [Qemu-devel] [PATCH] Softfloat: Add support to softfloat to return floatxx_default_nan when the corresponding target status flag is set.

[Aurelien Jarno]

On Fri, Feb 04, 2011 at 02:44:48PM +0000, Peter Maydell wrote:
> On 4 February 2011 14:01, Christophe Lyon <christophe.lyon@st.com> wrote:
> > Some CPUs have a status flag imposing to return floatxx_default_nan
> > whatever the input value, when converting from one FP format to
> > another. Implement this, using the already existing default_nan_mode
> > status flag, only for ARM at the moment, though other architectures
> > may have the same feature. This patch only modifies the
> > commonNaNToFloat32 and commonNaNToFloat64 conversion functions, as ARM
> > only uses these.
> 
> > -static float32 commonNaNToFloat32( commonNaNT a )
> > +static float32 commonNaNToFloat32( commonNaNT a STATUS_PARAM )
> >  {
> >     bits32 mantissa = a.high>>41;
> > +
> > +#if defined(TARGET_ARM)
> > +    if ( STATUS(default_nan_mode) ) {
> > +        return float32_default_nan;
> > +    }
> > +#endif
> 
> The target-specific #ifdef is pretty ugly.
> Fortunately the only architecture except ARM that sets
> default_nan_mode is sh4, so all we have to do is find out
> what it does with its float-to-float conversions. It looks
> to me from
> http://documentation.renesas.com/eng/products/mpumcu/rej09b0318_sh_4sm.pdf
> that it also returns the default NaN for converting a NaN
> from one float format to another. So you can just drop the
> #ifdef.

I confirm that the same bug is present on SH4 (tested on real hardware),
so the same fix is needed there. Thanks for catching that. Can you
please resend your patch without the #ifdef?

> (cc'ing Aurelien to check since he's the SH4 maintainer.)
> 
> I also think the change is simple enough that we ought to do
> it consistently for the floatx80 and float128 functions even if
> neither ARM nor sh4 use them.
> 

Agreed.


-- 
Aurelien Jarno                          GPG: 1024D/F1BCDB73
aurelien@aurel32.net                 http://www.aurel32.net

Re: [Qemu-devel] [PATCH] Softfloat: Add support to softfloat to return floatxx_default_nan when the corresponding target status flag is set.

[Christophe Lyon]

On 04.02.2011 20:47, Aurelien Jarno wrote:
> On Fri, Feb 04, 2011 at 02:44:48PM +0000, Peter Maydell wrote:
>> On 4 February 2011 14:01, Christophe Lyon <christophe.lyon@st.com> wrote:
>>
>> The target-specific #ifdef is pretty ugly.
I wanted to be safe (not breaking other targets), and as there are already some #ifdef in the same file, it seemed acceptable ;-)


> 
> I confirm that the same bug is present on SH4 (tested on real hardware),
> so the same fix is needed there. Thanks for catching that. Can you
> please resend your patch without the #ifdef?
> 
OK

>> (cc'ing Aurelien to check since he's the SH4 maintainer.)
>>
>> I also think the change is simple enough that we ought to do
>> it consistently for the floatx80 and float128 functions even if
>> neither ARM nor sh4 use them.
>>
> 
> Agreed.
> 
OK.

I was also wondering about the float16 case? The ARM ARM describes it uses default_nan too, but I couldn't find that right piece of code to patch in softfloat. Any idea? It seems to behave a bit differently from other FP formats.

Christophe.

 

Re: [Qemu-devel] [PATCH] Softfloat: Add support to softfloat to return floatxx_default_nan when the corresponding target status flag is set.

[Peter Maydell]

On 7 February 2011 10:17, Christophe Lyon <christophe.lyon@st.com> wrote:
> I was also wondering about the float16 case? The ARM ARM describes it
> uses default_nan too, but I couldn't find that right piece of code to patch
> in softfloat. Any idea? It seems to behave a bit differently from other FP
> formats.

Hm. My suggestion is that the bit of code in float32_to_float16() that does:
        if (aSig) {
            /* Make sure correct exceptions are raised.  */
            float32ToCommonNaN(a STATUS_VAR);
            aSig |= 0x00400000;
        }
        return packFloat16(aSign, 0x1f, aSig >> 13);

should be brought into line with the other float*_to_float* functions
by having it use a new commonNaNToFloat16() function rather
than hand-coding the conversion.

AFAICT only ARM uses float16, and that only for float16<->float32
conversions, so the softfloat code support for it is limited to only
what was needed for that.

-- PMM