| Message ID | 4D4C06CD.4060900@st.com |
|---|---|
| State | New |
| Headers | show |
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
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.
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.
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
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 ); }
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(-)