[Qemu-devel] [PATCH v2 0/2] softfloat tests based on berkeley's testfloat

Emilio G. Cota posted 2 patches 7 years, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20180908191735.22861-1-cota@braap.org
Test docker-clang@ubuntu failed
Test checkpatch failed
There is a newer version of this series
[Qemu-devel] [PATCH v2 0/2] softfloat tests based on berkeley's testfloat
Posted by Emilio G. Cota 7 years, 1 month ago
A few fixes since yesterday's v1:
  https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg00884.html

- Convert copy_qemu_to_soft80 to qemu_to_soft80, just like the other
  conversion functions
- Set fp-test as the program name as reported by itself
- Fix Makefile to include .d files so that dependencies are
  properly tracked
- Update commit log

Grab this from:
  https://github.com/cota/qemu/tree/fp-test-v2

Thanks,

		Emilio



Re: [Qemu-devel] [PATCH v2 0/2] softfloat tests based on berkeley's testfloat
Posted by Alex Bennée 7 years, 1 month ago
Emilio G. Cota <cota@braap.org> writes:

> A few fixes since yesterday's v1:
>   https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg00884.html
>
> - Convert copy_qemu_to_soft80 to qemu_to_soft80, just like the other
>   conversion functions
> - Set fp-test as the program name as reported by itself
> - Fix Makefile to include .d files so that dependencies are
>   properly tracked
> - Update commit log

Just some general comments:

 - I think this is a better way to go than the IBM test suite
 - I'm ambivalent about maintaining our fp-test.c in close alignment to
   softfloat unless we expect much upstreaming of changes.
 - the coverage seems a bit low. Rebuilding everything with
   --enable-gcov and running -all1 -all2 I get:

  tests/fp/fp-test.c - 53.5 % coverage 43.3 % branch coverage
  fpu/softfloat.c - 32.5 % coverage 25.1 % branch coverage

But maybe I didn't pass enough options to fp-test? I could really do
with a --just-run-everything-and-summarise-failing-functions option so I
can then go through in more detail with fp-test fFOO_BAR.

>
> Grab this from:
>   https://github.com/cota/qemu/tree/fp-test-v2
>
> Thanks,
>
> 		Emilio


--
Alex Bennée

Re: [Qemu-devel] [PATCH v2 0/2] softfloat tests based on berkeley's testfloat
Posted by Emilio G. Cota 7 years, 1 month ago
On Mon, Sep 10, 2018 at 12:26:58 +0100, Alex Bennée wrote:
> 
> Emilio G. Cota <cota@braap.org> writes:
> 
> > A few fixes since yesterday's v1:
> >   https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg00884.html
> >
> > - Convert copy_qemu_to_soft80 to qemu_to_soft80, just like the other
> >   conversion functions
> > - Set fp-test as the program name as reported by itself
> > - Fix Makefile to include .d files so that dependencies are
> >   properly tracked
> > - Update commit log
> 
> Just some general comments:
> 
>  - I think this is a better way to go than the IBM test suite
>  - I'm ambivalent about maintaining our fp-test.c in close alignment to
>    softfloat unless we expect much upstreaming of changes.

I don't think there'll be any, to be honest. They have a git repo but
I doubt they'll take any patches. This was just a minimum attempt to
get some tests working (I don't have a lot of time to work on this)

>  - the coverage seems a bit low. Rebuilding everything with
>    --enable-gcov and running -all1 -all2 I get:
> 
>   tests/fp/fp-test.c - 53.5 % coverage 43.3 % branch coverage
>   fpu/softfloat.c - 32.5 % coverage 25.1 % branch coverage
> 
> But maybe I didn't pass enough options to fp-test? I could really do
> with a --just-run-everything-and-summarise-failing-functions option so I
> can then go through in more detail with fp-test fFOO_BAR.

Yes right now to get better coverage you need to run it several times.
After a few runs I got it to 58%, but still without testing some
rounding modes. So considering we're coming from 0% coverage, I'd
say coverage is pretty good!

But yes, we should add a flag to just test -all.

There are some functions that we are not testing yet though, but
that can be fixed over time (we can add more tests to fp-test even
though they're not in testfloat, such as testing flush-to-zero/
denormals-are-zero, or the muladd variants that we have).

I have basically no time left to work on this. What do you think about
the following plan?

1. Have our own clones (forks) of testfloat/softfloat in qemu servers.
2. Add those as submodules
3. Add fp-test with as you said an -all flag that reports all
   errors to get decent coverage.
4. Add hardfloat patches, with tests. This requires a small
   change to testfloat:
   https://github.com/cota/berkeley-testfloat-3/commit/ca9fa2ba05

I'd then leave adding further tests to increase coverage and fixing
the existing bugs (prior to hardfloat) to someone else with
more time/resources.

Thanks,

		Emilio

Re: [Qemu-devel] [PATCH v2 0/2] softfloat tests based on berkeley's testfloat
Posted by Alex Bennée 7 years, 1 month ago
Emilio G. Cota <cota@braap.org> writes:

> On Mon, Sep 10, 2018 at 12:26:58 +0100, Alex Bennée wrote:
>>
>> Emilio G. Cota <cota@braap.org> writes:
>>
>> > A few fixes since yesterday's v1:
>> >   https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg00884.html
>> >
>> > - Convert copy_qemu_to_soft80 to qemu_to_soft80, just like the other
>> >   conversion functions
>> > - Set fp-test as the program name as reported by itself
>> > - Fix Makefile to include .d files so that dependencies are
>> >   properly tracked
>> > - Update commit log
>>
>> Just some general comments:
>>
>>  - I think this is a better way to go than the IBM test suite
>>  - I'm ambivalent about maintaining our fp-test.c in close alignment to
>>    softfloat unless we expect much upstreaming of changes.
>
> I don't think there'll be any, to be honest. They have a git repo but
> I doubt they'll take any patches. This was just a minimum attempt to
> get some tests working (I don't have a lot of time to work on this)
>
>>  - the coverage seems a bit low. Rebuilding everything with
>>    --enable-gcov and running -all1 -all2 I get:
>>
>>   tests/fp/fp-test.c - 53.5 % coverage 43.3 % branch coverage
>>   fpu/softfloat.c - 32.5 % coverage 25.1 % branch coverage
>>
>> But maybe I didn't pass enough options to fp-test? I could really do
>> with a --just-run-everything-and-summarise-failing-functions option so I
>> can then go through in more detail with fp-test fFOO_BAR.
>
> Yes right now to get better coverage you need to run it several times.
> After a few runs I got it to 58%, but still without testing some
> rounding modes. So considering we're coming from 0% coverage, I'd
> say coverage is pretty good!

I agree ;-)

>
> But yes, we should add a flag to just test -all.
>
> There are some functions that we are not testing yet though, but
> that can be fixed over time (we can add more tests to fp-test even
> though they're not in testfloat, such as testing flush-to-zero/
> denormals-are-zero, or the muladd variants that we have).
>
> I have basically no time left to work on this. What do you think about
> the following plan?
>
> 1. Have our own clones (forks) of testfloat/softfloat in qemu servers.
> 2. Add those as submodules
> 3. Add fp-test with as you said an -all flag that reports all
>    errors to get decent coverage.

Works for me.

> 4. Add hardfloat patches, with tests. This requires a small
>    change to testfloat:
>    https://github.com/cota/berkeley-testfloat-3/commit/ca9fa2ba05

It seems a shame to have our mirror of testfloat diverge but if it can't
be avoided I guess it will have to do. Couldn't we just zero the HW
flags in the wrapped up function? I may be misremembering but don't we
fall-back to softfloat code anyway if we have reached any limits that
trigger exception flags?

> I'd then leave adding further tests to increase coverage and fixing
> the existing bugs (prior to hardfloat) to someone else with
> more time/resources.

Agreed. I should have time to chase down the softfloat regressions once
I'm back from Connect after next week.

>
> Thanks,
>
> 		Emilio


--
Alex Bennée

Re: [Qemu-devel] [PATCH v2 0/2] softfloat tests based on berkeley's testfloat
Posted by Emilio G. Cota 7 years, 1 month ago
On Mon, Sep 10, 2018 at 16:41:21 +0100, Alex Bennée wrote:
> 
> Emilio G. Cota <cota@braap.org> writes:
> 
> > On Mon, Sep 10, 2018 at 12:26:58 +0100, Alex Bennée wrote:
> >>
> >> Emilio G. Cota <cota@braap.org> writes:
> >>
> >> > A few fixes since yesterday's v1:
> >> >   https://lists.gnu.org/archive/html/qemu-devel/2018-09/msg00884.html
> >> >
> >> > - Convert copy_qemu_to_soft80 to qemu_to_soft80, just like the other
> >> >   conversion functions
> >> > - Set fp-test as the program name as reported by itself
> >> > - Fix Makefile to include .d files so that dependencies are
> >> >   properly tracked
> >> > - Update commit log
> >>
> >> Just some general comments:
> >>
> >>  - I think this is a better way to go than the IBM test suite
> >>  - I'm ambivalent about maintaining our fp-test.c in close alignment to
> >>    softfloat unless we expect much upstreaming of changes.
> >
> > I don't think there'll be any, to be honest. They have a git repo but
> > I doubt they'll take any patches. This was just a minimum attempt to
> > get some tests working (I don't have a lot of time to work on this)
> >
> >>  - the coverage seems a bit low. Rebuilding everything with
> >>    --enable-gcov and running -all1 -all2 I get:
> >>
> >>   tests/fp/fp-test.c - 53.5 % coverage 43.3 % branch coverage
> >>   fpu/softfloat.c - 32.5 % coverage 25.1 % branch coverage
> >>
> >> But maybe I didn't pass enough options to fp-test? I could really do
> >> with a --just-run-everything-and-summarise-failing-functions option so I
> >> can then go through in more detail with fp-test fFOO_BAR.
> >
> > Yes right now to get better coverage you need to run it several times.
> > After a few runs I got it to 58%, but still without testing some
> > rounding modes. So considering we're coming from 0% coverage, I'd
> > say coverage is pretty good!
> 
> I agree ;-)
> 
> >
> > But yes, we should add a flag to just test -all.
> >
> > There are some functions that we are not testing yet though, but
> > that can be fixed over time (we can add more tests to fp-test even
> > though they're not in testfloat, such as testing flush-to-zero/
> > denormals-are-zero, or the muladd variants that we have).
> >
> > I have basically no time left to work on this. What do you think about
> > the following plan?
> >
> > 1. Have our own clones (forks) of testfloat/softfloat in qemu servers.
> > 2. Add those as submodules
> > 3. Add fp-test with as you said an -all flag that reports all
> >    errors to get decent coverage.
> 
> Works for me.
> 
> > 4. Add hardfloat patches, with tests. This requires a small
> >    change to testfloat:
> >    https://github.com/cota/berkeley-testfloat-3/commit/ca9fa2ba05
> 
> It seems a shame to have our mirror of testfloat diverge but if it can't
> be avoided I guess it will have to do. Couldn't we just zero the HW
> flags in the wrapped up function? I may be misremembering but don't we
> fall-back to softfloat code anyway if we have reached any limits that
> trigger exception flags?

testfloat compares the result of the ops as well as the resulting flags.
It zeroes the exception flags register before computing the
"golden" result/flags. The hardfloat code only kicks in when inexact
is set, so we need to add the above patch to testfloat so that we can
control the initial flags from outside. With this patch + a patch
on the fp-test side to set the initial flags, we can choose whether
to test hardfloat or not.

About your second question: yes, for corner cases we defer to softfloat.
But to reach those corner cases we first have to reach the hardfloat
code, which requires inexact to be set.

> > I'd then leave adding further tests to increase coverage and fixing
> > the existing bugs (prior to hardfloat) to someone else with
> > more time/resources.
> 
> Agreed. I should have time to chase down the softfloat regressions once
> I'm back from Connect after next week.

Nice! I'll try to have something ready later today.

		Emilio