| Message ID | 1294065273-30274-3-git-send-email-aurelien@aurel32.net |
|---|---|
| State | New |
| Headers | show |
On 3 January 2011 14:34, Aurelien Jarno <aurelien@aurel32.net> wrote: > On targets that define sNaN with the sNaN bit as one, simply clearing > this bit may correspond to an infinite value. > > Convert it to a default NaN if SNAN_BIT_IS_ONE, as it corresponds to > the MIPS implementation, the only emulated CPU with SNAN_BIT_IS_ONE. > When other CPU of this type are added, this might be updated to include > more cases. This patch doesn't apply to master: > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> > --- > fpu/softfloat-specialize.h | 12 ++++++------ > 1 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h > index f23bd6a..31481e7 100644 > --- a/fpu/softfloat-specialize.h > +++ b/fpu/softfloat-specialize.h > @@ -107,13 +107,13 @@ int float32_is_signaling_nan( float32 a_ ) > float32 float32_maybe_silence_nan( float32 a_ ) > { > if (float32_is_signaling_nan(a_)) { > - bits32 a = float32_val(a_); ...on master this line is uint32_t a = float32_val(a_); (different type) so the patch doesn't apply. Other than that, looks OK. I think I'd like a comment somewhere along the lines of /* Rules for silencing a signaling NaN are target-specific. Typically * targets with !SNAN_BIT_IS_ONE use the rule that the NaN * is silenced by setting the bit. Targets where SNAN_BIT_IS_ONE * must do something more complicated, because clearing the * bit might turn a NaN into an infinity. This code is correct for * MIPS but new targets might need something different. */ Or you could have the #ifdefs be on TARGET_whatever so that it's clear (because it won't compile) that adding a new TARGET_FOO means you have to check behaviour in this area. But I don't feel very strongly about that. -- PMM
Peter Maydell a écrit : > On 3 January 2011 14:34, Aurelien Jarno <aurelien@aurel32.net> wrote: >> On targets that define sNaN with the sNaN bit as one, simply clearing >> this bit may correspond to an infinite value. >> >> Convert it to a default NaN if SNAN_BIT_IS_ONE, as it corresponds to >> the MIPS implementation, the only emulated CPU with SNAN_BIT_IS_ONE. >> When other CPU of this type are added, this might be updated to include >> more cases. > > This patch doesn't apply to master: > >> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> >> --- >> fpu/softfloat-specialize.h | 12 ++++++------ >> 1 files changed, 6 insertions(+), 6 deletions(-) >> >> diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h >> index f23bd6a..31481e7 100644 >> --- a/fpu/softfloat-specialize.h >> +++ b/fpu/softfloat-specialize.h >> @@ -107,13 +107,13 @@ int float32_is_signaling_nan( float32 a_ ) >> float32 float32_maybe_silence_nan( float32 a_ ) >> { >> if (float32_is_signaling_nan(a_)) { >> - bits32 a = float32_val(a_); > > ...on master this line is > uint32_t a = float32_val(a_); > > (different type) so the patch doesn't apply. Oops, yes, my patch series should have started by a patch fixing types, but i made a mistake selecting the commits to send. Will fix that in a v2. > Other than that, looks OK. I think I'd like a comment somewhere > along the lines of > /* Rules for silencing a signaling NaN are target-specific. Typically > * targets with !SNAN_BIT_IS_ONE use the rule that the NaN > * is silenced by setting the bit. Targets where SNAN_BIT_IS_ONE > * must do something more complicated, because clearing the > * bit might turn a NaN into an infinity. This code is correct for > * MIPS but new targets might need something different. > */ > > Or you could have the #ifdefs be on TARGET_whatever so > that it's clear (because it won't compile) that adding a new > TARGET_FOO means you have to check behaviour in this > area. But I don't feel very strongly about that. > Ok, thanks for the review, will fix that.
diff --git a/fpu/softfloat-specialize.h b/fpu/softfloat-specialize.h index f23bd6a..31481e7 100644 --- a/fpu/softfloat-specialize.h +++ b/fpu/softfloat-specialize.h @@ -107,13 +107,13 @@ int float32_is_signaling_nan( float32 a_ ) float32 float32_maybe_silence_nan( float32 a_ ) { if (float32_is_signaling_nan(a_)) { - bits32 a = float32_val(a_); #if SNAN_BIT_IS_ONE - a &= ~(1 << 22); + return float32_default_nan; #else + bits32 a = float32_val(a_); a |= (1 << 22); -#endif return make_float32(a); +#endif } return a_; } @@ -321,13 +321,13 @@ int float64_is_signaling_nan( float64 a_ ) float64 float64_maybe_silence_nan( float64 a_ ) { if (float64_is_signaling_nan(a_)) { - bits64 a = float64_val(a_); #if SNAN_BIT_IS_ONE - a &= ~LIT64( 0x0008000000000000 ); + return float64_default_nan; #else + bits64 a = float64_val(a_); a |= LIT64( 0x0008000000000000 ); -#endif return make_float64(a); +#endif } return a_; }
On targets that define sNaN with the sNaN bit as one, simply clearing this bit may correspond to an infinite value. Convert it to a default NaN if SNAN_BIT_IS_ONE, as it corresponds to the MIPS implementation, the only emulated CPU with SNAN_BIT_IS_ONE. When other CPU of this type are added, this might be updated to include more cases. Signed-off-by: Aurelien Jarno <aurelien@aurel32.net> --- fpu/softfloat-specialize.h | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)