mbox series

[for-3.1,0/7] target/ppc fp cleanups

Message ID 20180703151732.29843-1-richard.henderson@linaro.org
Headers show
Series target/ppc fp cleanups | expand

Message

Richard Henderson July 3, 2018, 3:17 p.m. UTC
Beginning with John Arbuckle's fdiv test case, clean up some
of the fp helpers.  As with fdiv, fre and fresqrt are missing
divide-by-zero exceptions.

I've also noticed that load/store were using arithmetic conversions
to/from float32.  These should be using the non-arithmetic algorithms
listed in the manual.

Aside from cleaning up all of the rest of the helpers, I believe that
the implementation of the single-precision operations are incorrect.
They are currently implemented with the double-precision operation
followed by a round-to-single.  This causes incorrect results via
double rounding.  I believe better results could be had by using
these non-arithmetic converters to produce float32 operands, use the
proper float32 softfloat operations, and then convert back.

Anyway, all of this has been broken long enough that it'll need to
wait til next devel cycle before anything further gets done.


r~


Richard Henderson (7):
  target/ppc: Enable fp exceptions for user-only
  target/ppc: Honor fpscr_ze semantics and tidy fdiv
  target/ppc: Tidy helper_fmul
  target/ppc: Tidy helper_fadd, helper_fsub
  target/ppc: Tidy helper_fsqrt
  target/ppc: Honor fpscr_ze semantics and tidy fre, fresqrt
  target/ppc: Use non-arithmetic conversions for fp load/store

 target/ppc/helper.h                |  14 +-
 target/ppc/fpu_helper.c            | 294 +++++++++++++++++------------
 target/ppc/translate/fp-impl.inc.c |  26 +--
 target/ppc/translate_init.inc.c    |   2 +
 4 files changed, 189 insertions(+), 147 deletions(-)

-- 
2.17.1

Comments

David Gibson July 4, 2018, 3:31 a.m. UTC | #1
On Tue, Jul 03, 2018 at 08:17:25AM -0700, Richard Henderson wrote:
> Beginning with John Arbuckle's fdiv test case, clean up some

> of the fp helpers.  As with fdiv, fre and fresqrt are missing

> divide-by-zero exceptions.

> 

> I've also noticed that load/store were using arithmetic conversions

> to/from float32.  These should be using the non-arithmetic algorithms

> listed in the manual.

> 

> Aside from cleaning up all of the rest of the helpers, I believe that

> the implementation of the single-precision operations are incorrect.

> They are currently implemented with the double-precision operation

> followed by a round-to-single.  This causes incorrect results via

> double rounding.  I believe better results could be had by using

> these non-arithmetic converters to produce float32 operands, use the

> proper float32 softfloat operations, and then convert back.

> 

> Anyway, all of this has been broken long enough that it'll need to

> wait til next devel cycle before anything further gets done.


Applied to ppc-for-3.1 (newly created).

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
G 3 July 4, 2018, 1:42 p.m. UTC | #2
> On Jul 3, 2018, at 11:31 PM, David Gibson <david@gibson.dropbear.id.au> wrote:

> 

> On Tue, Jul 03, 2018 at 08:17:25AM -0700, Richard Henderson wrote:

>> Beginning with John Arbuckle's fdiv test case, clean up some

>> of the fp helpers.  As with fdiv, fre and fresqrt are missing

>> divide-by-zero exceptions.

>> 

>> I've also noticed that load/store were using arithmetic conversions

>> to/from float32.  These should be using the non-arithmetic algorithms

>> listed in the manual.

>> 

>> Aside from cleaning up all of the rest of the helpers, I believe that

>> the implementation of the single-precision operations are incorrect.

>> They are currently implemented with the double-precision operation

>> followed by a round-to-single.  This causes incorrect results via

>> double rounding.  I believe better results could be had by using

>> these non-arithmetic converters to produce float32 operands, use the

>> proper float32 softfloat operations, and then convert back.

>> 

>> Anyway, all of this has been broken long enough that it'll need to

>> wait til next devel cycle before anything further gets done.

> 

> Applied to ppc-for-3.1 (newly created)


Could I have the address for your ppc-for-3.1 repo please? Would you like future floating point patches to be based on this repo?
Alex Bennée July 4, 2018, 2:15 p.m. UTC | #3
Richard Henderson <richard.henderson@linaro.org> writes:

>

> Anyway, all of this has been broken long enough that it'll need to

> wait til next devel cycle before anything further gets done.


When it gets done should we add some explicit ppc tests to tests/tcg/ppc
like we did with arm64 fcvt or would it be worth spending more time on
the general purpose FP test suite Emilio was working on as part of his
hard-float series?

>

>

> r~

>

>

> Richard Henderson (7):

>   target/ppc: Enable fp exceptions for user-only

>   target/ppc: Honor fpscr_ze semantics and tidy fdiv

>   target/ppc: Tidy helper_fmul

>   target/ppc: Tidy helper_fadd, helper_fsub

>   target/ppc: Tidy helper_fsqrt

>   target/ppc: Honor fpscr_ze semantics and tidy fre, fresqrt

>   target/ppc: Use non-arithmetic conversions for fp load/store

>

>  target/ppc/helper.h                |  14 +-

>  target/ppc/fpu_helper.c            | 294 +++++++++++++++++------------

>  target/ppc/translate/fp-impl.inc.c |  26 +--

>  target/ppc/translate_init.inc.c    |   2 +

>  4 files changed, 189 insertions(+), 147 deletions(-)



--
Alex Bennée
G 3 July 4, 2018, 2:49 p.m. UTC | #4
> On Jul 4, 2018, at 10:15 AM, Alex Bennée <alex.bennee@linaro.org> wrote:

> 

> 

> Richard Henderson <richard.henderson@linaro.org> writes:

> 

>> 

>> Anyway, all of this has been broken long enough that it'll need to

>> wait til next devel cycle before anything further gets done.

> 

> When it gets done should we add some explicit ppc tests to tests/tcg/ppc

> like we did with arm64 fcvt or would it be worth spending more time on

> the general purpose FP test suite Emilio was working on as part of his

> hard-float series?


My vote goes to adding PowerPC specific tests. I already made some and sent them to the list a couple of days ago. A general purpose floating point test suite would probably not be able to test things like PowerPC floating point flags. 

I'm not certain how we would add floating point tests to QEMU. I'm guessing it would involve some kind of bootable image file that would load and run the tests. Hopefully there is an easier way to implement testing.
Alex Bennée July 4, 2018, 3:39 p.m. UTC | #5
Programmingkid <programmingkidx@gmail.com> writes:

>> On Jul 4, 2018, at 10:15 AM, Alex Bennée <alex.bennee@linaro.org> wrote:

>>

>>

>> Richard Henderson <richard.henderson@linaro.org> writes:

>>

>>>

>>> Anyway, all of this has been broken long enough that it'll need to

>>> wait til next devel cycle before anything further gets done.

>>

>> When it gets done should we add some explicit ppc tests to tests/tcg/ppc

>> like we did with arm64 fcvt or would it be worth spending more time on

>> the general purpose FP test suite Emilio was working on as part of his

>> hard-float series?

>

> My vote goes to adding PowerPC specific tests. I already made some and sent them to the list a couple of days ago. A general purpose floating point test suite would probably not be able to test things like PowerPC floating point flags.

>

> I'm not certain how we would add floating point tests to QEMU. I'm

> guessing it would involve some kind of bootable image file that would

> load and run the tests. Hopefully there is an easier way to implement

> testing.


You don't need system tests - this is all testable from linux-user. See
tests/tcg/arm/fcvt.c and the .ref files in arm and aarch64 directories.
Assuming the test isn't insane (like test-i386-fprem) we can just add a
reference output recorded on know good hardware.

--
Alex Bennée
G 3 July 4, 2018, 4:23 p.m. UTC | #6
> On Jul 4, 2018, at 11:39 AM, Alex Bennée <alex.bennee@linaro.org> wrote:

> 

> 

> Programmingkid <programmingkidx@gmail.com> writes:

> 

>>> On Jul 4, 2018, at 10:15 AM, Alex Bennée <alex.bennee@linaro.org> wrote:

>>> 

>>> 

>>> Richard Henderson <richard.henderson@linaro.org> writes:

>>> 

>>>> 

>>>> Anyway, all of this has been broken long enough that it'll need to

>>>> wait til next devel cycle before anything further gets done.

>>> 

>>> When it gets done should we add some explicit ppc tests to tests/tcg/ppc

>>> like we did with arm64 fcvt or would it be worth spending more time on

>>> the general purpose FP test suite Emilio was working on as part of his

>>> hard-float series?

>> 

>> My vote goes to adding PowerPC specific tests. I already made some and sent them to the list a couple of days ago. A general purpose floating point test suite would probably not be able to test things like PowerPC floating point flags.

>> 

>> I'm not certain how we would add floating point tests to QEMU. I'm

>> guessing it would involve some kind of bootable image file that would

>> load and run the tests. Hopefully there is an easier way to implement

>> testing.

> 

> You don't need system tests - this is all testable from linux-user. See

> tests/tcg/arm/fcvt.c and the .ref files in arm and aarch64 directories.

> Assuming the test isn't insane (like test-i386-fprem) we can just add a

> reference output recorded on know good hardware.


My hope is to make testing the floating point unit as easy as 'make test' or maybe 'make fp-test'.
Alex Bennée July 4, 2018, 4:48 p.m. UTC | #7
Programmingkid <programmingkidx@gmail.com> writes:

>> On Jul 4, 2018, at 11:39 AM, Alex Bennée <alex.bennee@linaro.org> wrote:

>>

>>

>> Programmingkid <programmingkidx@gmail.com> writes:

>>

>>>> On Jul 4, 2018, at 10:15 AM, Alex Bennée <alex.bennee@linaro.org> wrote:

>>>>

>>>>

>>>> Richard Henderson <richard.henderson@linaro.org> writes:

>>>>

>>>>>

>>>>> Anyway, all of this has been broken long enough that it'll need to

>>>>> wait til next devel cycle before anything further gets done.

>>>>

>>>> When it gets done should we add some explicit ppc tests to tests/tcg/ppc

>>>> like we did with arm64 fcvt or would it be worth spending more time on

>>>> the general purpose FP test suite Emilio was working on as part of his

>>>> hard-float series?

>>>

>>> My vote goes to adding PowerPC specific tests. I already made some and sent them to the list a couple of days ago. A general purpose floating point test suite would probably not be able to test things like PowerPC floating point flags.

>>>

>>> I'm not certain how we would add floating point tests to QEMU. I'm

>>> guessing it would involve some kind of bootable image file that would

>>> load and run the tests. Hopefully there is an easier way to implement

>>> testing.

>>

>> You don't need system tests - this is all testable from linux-user. See

>> tests/tcg/arm/fcvt.c and the .ref files in arm and aarch64 directories.

>> Assuming the test isn't insane (like test-i386-fprem) we can just add a

>> reference output recorded on know good hardware.

>

> My hope is to make testing the floating point unit as easy as 'make

> test' or maybe 'make fp-test'.


That would be "make check-tcg"


--
Alex Bennée
David Gibson July 5, 2018, 12:09 a.m. UTC | #8
On Wed, Jul 04, 2018 at 09:42:16AM -0400, Programmingkid wrote:
> 

> > On Jul 3, 2018, at 11:31 PM, David Gibson <david@gibson.dropbear.id.au> wrote:

> > 

> > On Tue, Jul 03, 2018 at 08:17:25AM -0700, Richard Henderson wrote:

> >> Beginning with John Arbuckle's fdiv test case, clean up some

> >> of the fp helpers.  As with fdiv, fre and fresqrt are missing

> >> divide-by-zero exceptions.

> >> 

> >> I've also noticed that load/store were using arithmetic conversions

> >> to/from float32.  These should be using the non-arithmetic algorithms

> >> listed in the manual.

> >> 

> >> Aside from cleaning up all of the rest of the helpers, I believe that

> >> the implementation of the single-precision operations are incorrect.

> >> They are currently implemented with the double-precision operation

> >> followed by a round-to-single.  This causes incorrect results via

> >> double rounding.  I believe better results could be had by using

> >> these non-arithmetic converters to produce float32 operands, use the

> >> proper float32 softfloat operations, and then convert back.

> >> 

> >> Anyway, all of this has been broken long enough that it'll need to

> >> wait til next devel cycle before anything further gets done.

> > 

> > Applied to ppc-for-3.1 (newly created)

> 

> Could I have the address for your ppc-for-3.1 repo please? Would you

> like future floating point patches to be based on this repo?


See https://github.com/dgibson/qemu/tree/ppc-for-3.1

Same repo as my existing ppc-for-3.0 tree, different branch.

And yes, patches should be based on this tree, unless they're
important bugfixes that are aimed at 3.0.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson