| Message ID | 20170207005930.28327-3-laurent@vivier.eu |
|---|---|
| State | New |
| Headers | show |
On 02/06/2017 04:59 PM, Laurent Vivier wrote: > According to the comment, this definition of invalid encoding is given > by intel developer's manual, and doesn't work with the behavior > of 680x0 FPU. > > CC: Andreas Schwab <schwab@linux-m68k.org> > Signed-off-by: Laurent Vivier <laurent@vivier.eu> > --- Reviewed-by: Richard Henderson <rth@twiddle.net> r~
On 7 February 2017 at 00:59, Laurent Vivier <laurent@vivier.eu> wrote: > According to the comment, this definition of invalid encoding is given > by intel developer's manual, and doesn't work with the behavior > of 680x0 FPU. > > CC: Andreas Schwab <schwab@linux-m68k.org> > Signed-off-by: Laurent Vivier <laurent@vivier.eu> > --- > fpu/softfloat.c | 31 +++++++++++++++++++++++++++++++ > include/fpu/softfloat.h | 15 --------------- > 2 files changed, 31 insertions(+), 15 deletions(-) > > diff --git a/fpu/softfloat.c b/fpu/softfloat.c > index c295f31..f95b19f 100644 > --- a/fpu/softfloat.c > +++ b/fpu/softfloat.c > @@ -4799,6 +4799,37 @@ int float64_unordered_quiet(float64 a, float64 b, float_status *status) > } > > /*---------------------------------------------------------------------------- > +| Return whether the given value is an invalid floatx80 encoding. > +| Invalid floatx80 encodings arise when the integer bit is not set, but > +| the exponent is not zero. The only times the integer bit is permitted to > +| be zero is in subnormal numbers and the value zero. > +| This includes what the Intel software developer's manual calls pseudo-NaNs, > +| pseudo-infinities and un-normal numbers. It does not include > +| pseudo-denormals, which must still be correctly handled as inputs even > +| if they are never generated as outputs. > +*----------------------------------------------------------------------------*/ > +static inline bool floatx80_invalid_encoding(floatx80 a) > +{ > +#if defined(TARGET_M68K) > + /*------------------------------------------------------------------------- > + | M68000 FAMILY PROGRAMMER’S REFERENCE MANUAL > + | 1.6.2 Denormalized Numbers > + | Since the extended-precision data format has an explicit integer bit, > + | a number can be formatted with a nonzero exponent, less than the maximum > + | value, and a zero integer bit. The IEEE 754 standard does not define a > + | zero integer bit. Such a number is an unnormalized number. Hardware does > + | not directly support denormalized and unnormalized numbers, but > + | implicitly supports them by trapping them as unimplemented data types, > + | allowing efficient conversion in software. > + *------------------------------------------------------------------------*/ This comment says that numbers of the form that this function identifies (zero integer bit and nonzero exponent) are supposed to trap on m68k. That suggests that the target/m68k code is going to need to catch this case before it calls softfloat code. So (a) we don't care what the softfloat code does with these representations because they'll never get to it and (b) the m68k code could probably use floatx80_invalid_encoding() as a function to identify where it needs to trap, which it can't do if it's moved out of softfloat.h. Either way this change doesn't seem right given what the comment describes... thanks -- PMM
Le 08/02/2017 à 23:58, Peter Maydell a écrit : > On 7 February 2017 at 00:59, Laurent Vivier <laurent@vivier.eu> wrote: >> According to the comment, this definition of invalid encoding is given >> by intel developer's manual, and doesn't work with the behavior >> of 680x0 FPU. >> >> CC: Andreas Schwab <schwab@linux-m68k.org> >> Signed-off-by: Laurent Vivier <laurent@vivier.eu> >> --- >> fpu/softfloat.c | 31 +++++++++++++++++++++++++++++++ >> include/fpu/softfloat.h | 15 --------------- >> 2 files changed, 31 insertions(+), 15 deletions(-) >> >> diff --git a/fpu/softfloat.c b/fpu/softfloat.c >> index c295f31..f95b19f 100644 >> --- a/fpu/softfloat.c >> +++ b/fpu/softfloat.c >> @@ -4799,6 +4799,37 @@ int float64_unordered_quiet(float64 a, float64 b, float_status *status) >> } >> >> /*---------------------------------------------------------------------------- >> +| Return whether the given value is an invalid floatx80 encoding. >> +| Invalid floatx80 encodings arise when the integer bit is not set, but >> +| the exponent is not zero. The only times the integer bit is permitted to >> +| be zero is in subnormal numbers and the value zero. >> +| This includes what the Intel software developer's manual calls pseudo-NaNs, >> +| pseudo-infinities and un-normal numbers. It does not include >> +| pseudo-denormals, which must still be correctly handled as inputs even >> +| if they are never generated as outputs. >> +*----------------------------------------------------------------------------*/ >> +static inline bool floatx80_invalid_encoding(floatx80 a) >> +{ >> +#if defined(TARGET_M68K) >> + /*------------------------------------------------------------------------- >> + | M68000 FAMILY PROGRAMMER’S REFERENCE MANUAL >> + | 1.6.2 Denormalized Numbers >> + | Since the extended-precision data format has an explicit integer bit, >> + | a number can be formatted with a nonzero exponent, less than the maximum >> + | value, and a zero integer bit. The IEEE 754 standard does not define a >> + | zero integer bit. Such a number is an unnormalized number. Hardware does >> + | not directly support denormalized and unnormalized numbers, but >> + | implicitly supports them by trapping them as unimplemented data types, >> + | allowing efficient conversion in software. >> + *------------------------------------------------------------------------*/ > > This comment says that numbers of the form that this function > identifies (zero integer bit and nonzero exponent) are > supposed to trap on m68k. That suggests that the target/m68k code > is going to need to catch this case before it calls softfloat > code. So (a) we don't care what the softfloat code does > with these representations because they'll never get to it > and (b) the m68k code could probably use floatx80_invalid_encoding() > as a function to identify where it needs to trap, which it > can't do if it's moved out of softfloat.h. > > Either way this change doesn't seem right given what the > comment describes... Ok, so I think this functionality needs more work inside the 680x0 FPU emulation. As the emulation can work without it, I'm going to remove it from this series to rework it later. Thanks, Laurent
diff --git a/fpu/softfloat.c b/fpu/softfloat.c index c295f31..f95b19f 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -4799,6 +4799,37 @@ int float64_unordered_quiet(float64 a, float64 b, float_status *status) } /*---------------------------------------------------------------------------- +| Return whether the given value is an invalid floatx80 encoding. +| Invalid floatx80 encodings arise when the integer bit is not set, but +| the exponent is not zero. The only times the integer bit is permitted to +| be zero is in subnormal numbers and the value zero. +| This includes what the Intel software developer's manual calls pseudo-NaNs, +| pseudo-infinities and un-normal numbers. It does not include +| pseudo-denormals, which must still be correctly handled as inputs even +| if they are never generated as outputs. +*----------------------------------------------------------------------------*/ +static inline bool floatx80_invalid_encoding(floatx80 a) +{ +#if defined(TARGET_M68K) + /*------------------------------------------------------------------------- + | M68000 FAMILY PROGRAMMER’S REFERENCE MANUAL + | 1.6.2 Denormalized Numbers + | Since the extended-precision data format has an explicit integer bit, + | a number can be formatted with a nonzero exponent, less than the maximum + | value, and a zero integer bit. The IEEE 754 standard does not define a + | zero integer bit. Such a number is an unnormalized number. Hardware does + | not directly support denormalized and unnormalized numbers, but + | implicitly supports them by trapping them as unimplemented data types, + | allowing efficient conversion in software. + *------------------------------------------------------------------------*/ + return 0; +#else + return (a.low & (1ULL << 63)) == 0 && (a.high & 0x7FFF) != 0; +#endif +} + + +/*---------------------------------------------------------------------------- | Returns the result of converting the extended double-precision floating- | point value `a' to the 32-bit two's complement integer format. The | conversion is performed according to the IEC/IEEE Standard for Binary diff --git a/include/fpu/softfloat.h b/include/fpu/softfloat.h index 842ec6b..3920c0a 100644 --- a/include/fpu/softfloat.h +++ b/include/fpu/softfloat.h @@ -678,21 +678,6 @@ static inline int floatx80_is_any_nan(floatx80 a) return ((a.high & 0x7fff) == 0x7fff) && (a.low<<1); } -/*---------------------------------------------------------------------------- -| Return whether the given value is an invalid floatx80 encoding. -| Invalid floatx80 encodings arise when the integer bit is not set, but -| the exponent is not zero. The only times the integer bit is permitted to -| be zero is in subnormal numbers and the value zero. -| This includes what the Intel software developer's manual calls pseudo-NaNs, -| pseudo-infinities and un-normal numbers. It does not include -| pseudo-denormals, which must still be correctly handled as inputs even -| if they are never generated as outputs. -*----------------------------------------------------------------------------*/ -static inline bool floatx80_invalid_encoding(floatx80 a) -{ - return (a.low & (1ULL << 63)) == 0 && (a.high & 0x7FFF) != 0; -} - #define floatx80_zero make_floatx80(0x0000, 0x0000000000000000LL) #define floatx80_one make_floatx80(0x3fff, 0x8000000000000000LL) #define floatx80_ln2 make_floatx80(0x3ffe, 0xb17217f7d1cf79acLL)
According to the comment, this definition of invalid encoding is given by intel developer's manual, and doesn't work with the behavior of 680x0 FPU. CC: Andreas Schwab <schwab@linux-m68k.org> Signed-off-by: Laurent Vivier <laurent@vivier.eu> --- fpu/softfloat.c | 31 +++++++++++++++++++++++++++++++ include/fpu/softfloat.h | 15 --------------- 2 files changed, 31 insertions(+), 15 deletions(-)