| Message ID | 1348845423-20085-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Accepted |
| Commit | e744c06fca438dc08271e626034e632a270c91c8 |
| Headers | show |
On 28 September 2012 16:17, Peter Maydell <peter.maydell@linaro.org> wrote: > The uint64_to_float32() conversion function was incorrectly always > returning numbers with the sign bit set (ie negative numbers). Correct > this so we return positive numbers instead. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > As far as I can see we use this function only in the three PPC SPE > insns efscfuf, efsctsf, efsctuf. It is therefore untested(!); if > anybody with PPC hw to test against could check the results of > those functions that would be cool. ...incidentally in two of those uses we're operating on a constant: tmp = uint64_to_float64(1ULL << 32, &env->vec_status); and it would probably be better to use make_float64() instead. -- PMM
On Fri, Sep 28, 2012 at 04:17:03PM +0100, Peter Maydell wrote: > The uint64_to_float32() conversion function was incorrectly always > returning numbers with the sign bit set (ie negative numbers). Correct > this so we return positive numbers instead. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > As far as I can see we use this function only in the three PPC SPE > insns efscfuf, efsctsf, efsctuf. It is therefore untested(!); if > anybody with PPC hw to test against could check the results of > those functions that would be cool. > > fpu/softfloat.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fpu/softfloat.c b/fpu/softfloat.c > index b29256a..91497e8 100644 > --- a/fpu/softfloat.c > +++ b/fpu/softfloat.c > @@ -1238,7 +1238,7 @@ float32 uint64_to_float32( uint64 a STATUS_PARAM ) > if ( a == 0 ) return float32_zero; > shiftCount = countLeadingZeros64( a ) - 40; > if ( 0 <= shiftCount ) { > - return packFloat32( 1 > 0, 0x95 - shiftCount, a<<shiftCount ); > + return packFloat32(0, 0x95 - shiftCount, a<<shiftCount); > } > else { > shiftCount += 7; > @@ -1248,7 +1248,7 @@ float32 uint64_to_float32( uint64 a STATUS_PARAM ) > else { > a <<= shiftCount; > } > - return roundAndPackFloat32( 1 > 0, 0x9C - shiftCount, a STATUS_VAR ); > + return roundAndPackFloat32(0, 0x9C - shiftCount, a STATUS_VAR); > } > } > Reviewed-by: Aurelien Jarno <aurelien@aurel32.net>
On Fri, Sep 28, 2012 at 04:42:16PM +0100, Peter Maydell wrote: > On 28 September 2012 16:17, Peter Maydell <peter.maydell@linaro.org> wrote: > > The uint64_to_float32() conversion function was incorrectly always > > returning numbers with the sign bit set (ie negative numbers). Correct > > this so we return positive numbers instead. > > > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > --- > > As far as I can see we use this function only in the three PPC SPE > > insns efscfuf, efsctsf, efsctuf. It is therefore untested(!); if > > anybody with PPC hw to test against could check the results of > > those functions that would be cool. SPE instructions are not common on the non-embedded world, as far as I know it should be tested using the e500 CPU. That said I don't have a kernel nor an image for such a machine. > ...incidentally in two of those uses we're operating on a constant: > tmp = uint64_to_float64(1ULL << 32, &env->vec_status); > and it would probably be better to use make_float64() instead. > Agreed.
On Fri, Sep 28, 2012 at 04:17:03PM +0100, Peter Maydell wrote: > The uint64_to_float32() conversion function was incorrectly always > returning numbers with the sign bit set (ie negative numbers). Correct > this so we return positive numbers instead. > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > As far as I can see we use this function only in the three PPC SPE > insns efscfuf, efsctsf, efsctuf. It is therefore untested(!); if > anybody with PPC hw to test against could check the results of > those functions that would be cool. > > fpu/softfloat.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/fpu/softfloat.c b/fpu/softfloat.c > index b29256a..91497e8 100644 > --- a/fpu/softfloat.c > +++ b/fpu/softfloat.c > @@ -1238,7 +1238,7 @@ float32 uint64_to_float32( uint64 a STATUS_PARAM ) > if ( a == 0 ) return float32_zero; > shiftCount = countLeadingZeros64( a ) - 40; > if ( 0 <= shiftCount ) { > - return packFloat32( 1 > 0, 0x95 - shiftCount, a<<shiftCount ); > + return packFloat32(0, 0x95 - shiftCount, a<<shiftCount); > } > else { > shiftCount += 7; > @@ -1248,7 +1248,7 @@ float32 uint64_to_float32( uint64 a STATUS_PARAM ) > else { > a <<= shiftCount; > } > - return roundAndPackFloat32( 1 > 0, 0x9C - shiftCount, a STATUS_VAR ); > + return roundAndPackFloat32(0, 0x9C - shiftCount, a STATUS_VAR); > } > } > Thanks, applied.
diff --git a/fpu/softfloat.c b/fpu/softfloat.c index b29256a..91497e8 100644 --- a/fpu/softfloat.c +++ b/fpu/softfloat.c @@ -1238,7 +1238,7 @@ float32 uint64_to_float32( uint64 a STATUS_PARAM ) if ( a == 0 ) return float32_zero; shiftCount = countLeadingZeros64( a ) - 40; if ( 0 <= shiftCount ) { - return packFloat32( 1 > 0, 0x95 - shiftCount, a<<shiftCount ); + return packFloat32(0, 0x95 - shiftCount, a<<shiftCount); } else { shiftCount += 7; @@ -1248,7 +1248,7 @@ float32 uint64_to_float32( uint64 a STATUS_PARAM ) else { a <<= shiftCount; } - return roundAndPackFloat32( 1 > 0, 0x9C - shiftCount, a STATUS_VAR ); + return roundAndPackFloat32(0, 0x9C - shiftCount, a STATUS_VAR); } }
The uint64_to_float32() conversion function was incorrectly always returning numbers with the sign bit set (ie negative numbers). Correct this so we return positive numbers instead. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- As far as I can see we use this function only in the three PPC SPE insns efscfuf, efsctsf, efsctuf. It is therefore untested(!); if anybody with PPC hw to test against could check the results of those functions that would be cool. fpu/softfloat.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)