[PATCH] Fix remainder calculating bug in single floating point division

[PATCH] Fix remainder calculating bug in single floating point division

[Liu Yu]

This bug exists in the emulation of floating point division for powerpc.

The original code cannot count the remainder correctly. 
I can provide a test case to trigger this bug. 
When use fdiv to count 1.1754941e-38f / 0.9999999f, 
the result is expected to be 1.175494e-38f, 
but we will get 1.174921e-38f in the original case.

Comments are always welcomed!

Signed-off-by: Liu Yu <Yu.Liu@freescale.com>
---
 arch/powerpc/math-emu/sfp-machine.h |   11 +++++------
 1 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/math-emu/sfp-machine.h b/arch/powerpc/math-emu/sfp-machine.h
index 4b17d83..644cee2 100644
--- a/arch/powerpc/math-emu/sfp-machine.h
+++ b/arch/powerpc/math-emu/sfp-machine.h
@@ -324,10 +324,10 @@ extern int  fp_pack_ds(void *, long, unsigned long, unsigned long, long, long);
     __r1 = __r1 * __ll_B | __ll_highpart (n0);				\
     if (__r1 < __m)							\
       {									\
-	__q1--, __r1 += (d);						\
-	if (__r1 >= (d)) /* we didn't get carry when adding to __r1 */	\
-	  if (__r1 < __m)						\
+	do {								\
 	    __q1--, __r1 += (d);					\
+	  /* we didn't get carry when adding to __r1 */			\
+	} while (__r1 >= (d) && __r1 < __m);				\
       }									\
     __r1 -= __m;							\
 									\
@@ -337,10 +337,9 @@ extern int  fp_pack_ds(void *, long, unsigned long, unsigned long, long, long);
     __r0 = __r0 * __ll_B | __ll_lowpart (n0);				\
     if (__r0 < __m)							\
       {									\
-	__q0--, __r0 += (d);						\
-	if (__r0 >= (d))						\
-	  if (__r0 < __m)						\
+	do {								\
 	    __q0--, __r0 += (d);					\
+	} while (__r0 >= (d) && __r0 < __m);				\
       }									\
     __r0 -= __m;							\
 									\
-- 
1.5.2

Re: [PATCH] Fix remainder calculating bug in single floating point division

[Benjamin Herrenschmidt]

On Sun, 2008-01-06 at 22:26 +0800, Liu Yu wrote:
> This bug exists in the emulation of floating point division for powerpc.
> 
> The original code cannot count the remainder correctly. 
> I can provide a test case to trigger this bug. 
> When use fdiv to count 1.1754941e-38f / 0.9999999f, 
> the result is expected to be 1.175494e-38f, 
> but we will get 1.174921e-38f in the original case.

It's nice to see somebody digging in that scary math emu stuff. If you
could also get rid of the warnings, it would be perfect :-)

Now, appart from that, I seem to remember Dan telling me at some point
in the past (I think it was Dan... ) that this code was lifted from
somewhere else (glibc ? gcc soft-float ?), in which case it might be
worth to fix that somewhere else as well, or if it's already been fixed,
maybe resync. 

Cheers,
Ben.

Re: [PATCH] Fix remainder calculating bug in single floating point division

[Dan Malek]

On Jan 6, 2008, at 12:07 PM, Benjamin Herrenschmidt wrote:

> It's nice to see somebody digging in that scary math emu stuff. If you
> could also get rid of the warnings, it would be perfect :-)

Yes, it is :-)  I didn't think it would have a life beyond MPC8xx.

> .... that this code was lifted from
> somewhere else (glibc ? gcc soft-float ?),

It seems like a lifetime ago....  I copied the framework
from Sparc, and the internals from gcc soft-float.  I didn't
change any of the internal emulation functions (hence,
some of the warnings), just the calling interface.

While it's convenient, I still don't think kernel float
emulation should be a solution.  The tools should
generate soft-float for the applications and libraries.

Thanks.

	-- Dan

Re: [PATCH] Fix remainder calculating bug in single floating point division

[Benjamin Herrenschmidt]

On Sun, 2008-01-06 at 12:44 -0800, Dan Malek wrote:
> While it's convenient, I still don't think kernel float
> emulation should be a solution.  The tools should
> generate soft-float for the applications and libraries.

I agree, for deployement, it's definitely not a solution, though it's
very convenient for us hackers :-)

Ben.

Re: [PATCH] Fix remainder calculating bug in single floating point division

[Josh Boyer]

On Mon, 07 Jan 2008 07:49:33 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> 
> On Sun, 2008-01-06 at 12:44 -0800, Dan Malek wrote:
> > While it's convenient, I still don't think kernel float
> > emulation should be a solution.  The tools should
> > generate soft-float for the applications and libraries.
> 
> I agree, for deployement, it's definitely not a solution, though it's
> very convenient for us hackers :-)

Hacks have a way of becoming solutions... ;)

josh

Re: [PATCH] Fix remainder calculating bug in single floating point division

[Kumar Gala]

On Jan 6, 2008, at 2:44 PM, Dan Malek wrote:

>
> On Jan 6, 2008, at 12:07 PM, Benjamin Herrenschmidt wrote:
>
>> It's nice to see somebody digging in that scary math emu stuff. If  
>> you
>> could also get rid of the warnings, it would be perfect :-)
>
> Yes, it is :-)  I didn't think it would have a life beyond MPC8xx.
>
>> .... that this code was lifted from
>> somewhere else (glibc ? gcc soft-float ?),
>
> It seems like a lifetime ago....  I copied the framework
> from Sparc, and the internals from gcc soft-float.  I didn't
> change any of the internal emulation functions (hence,
> some of the warnings), just the calling interface.
>
> While it's convenient, I still don't think kernel float
> emulation should be a solution.  The tools should
> generate soft-float for the applications and libraries.

If we think this is really true, we could move to using include/math- 
emu/* instead of the files in powerpc/math-emu.

The problem I had was when I tried to recreate the history of the code  
in powerpc/math-emu and how it doesn't really match the glibc code  
base.  There are some differences and I wasn't sure if they were do to  
trying to match PPC HW at a bit level or not.

I was hoping that the work Liu Yu would get as a bit of a testsuite to  
see if there was any harm in moving over to include/math-emu.

- k

RE: [PATCH] Fix remainder calculating bug in single floating pointdivision

[Liu Yu]

> -----Original Message-----
> From: linuxppc-dev-bounces+b13201=3Dfreescale.com@ozlabs.org=20
> [mailto:linuxppc-dev-bounces+b13201=3Dfreescale.com@ozlabs.org]=20
> On Behalf Of Kumar Gala
> Sent: Tuesday, January 08, 2008 2:20 PM
> To: Dan Malek
> Cc: Liu Yu; linuxppc-dev@ozlabs.org
> Subject: Re: [PATCH] Fix remainder calculating bug in single=20
> floating pointdivision
>=20
>=20
> On Jan 6, 2008, at 2:44 PM, Dan Malek wrote:
>=20
> >
> > On Jan 6, 2008, at 12:07 PM, Benjamin Herrenschmidt wrote:
> >
> >> It's nice to see somebody digging in that scary math emu stuff. If=20
> >> you could also get rid of the warnings, it would be perfect :-)
> >
> > Yes, it is :-)  I didn't think it would have a life beyond MPC8xx.
> >
> >> .... that this code was lifted from
> >> somewhere else (glibc ? gcc soft-float ?),
> >
> > It seems like a lifetime ago....  I copied the framework=20
> from Sparc,=20
> > and the internals from gcc soft-float.  I didn't change any of the=20
> > internal emulation functions (hence, some of the warnings),=20
> just the=20
> > calling interface.
> >
> > While it's convenient, I still don't think kernel float emulation=20
> > should be a solution.  The tools should generate soft-float for the=20
> > applications and libraries.
>=20
> If we think this is really true, we could move to using include/math-
> emu/* instead of the files in powerpc/math-emu.

Why it's better to move to using include/math-emu.
I found they have similar framework, is powerpc/math-emu evolved from
include/math-emu?

>=20
> The problem I had was when I tried to recreate the history of=20
> the code in powerpc/math-emu and how it doesn't really match=20
> the glibc code base.  There are some differences and I wasn't=20
> sure if they were do to trying to match PPC HW at a bit level or not.
>=20
> I was hoping that the work Liu Yu would get as a bit of a=20
> testsuite to see if there was any harm in moving over to=20
> include/math-emu.
>=20
> - k
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@ozlabs.org
> https://ozlabs.org/mailman/listinfo/linuxppc-dev
>=20

Re: [PATCH] Fix remainder calculating bug in single floating pointdivision

[Kumar Gala]

On Jan 9, 2008, at 9:38 AM, Liu Yu wrote:

>
>> -----Original Message-----
>> From: linuxppc-dev-bounces+b13201=freescale.com@ozlabs.org
>> [mailto:linuxppc-dev-bounces+b13201=freescale.com@ozlabs.org]
>> On Behalf Of Kumar Gala
>> Sent: Tuesday, January 08, 2008 2:20 PM
>> To: Dan Malek
>> Cc: Liu Yu; linuxppc-dev@ozlabs.org
>> Subject: Re: [PATCH] Fix remainder calculating bug in single
>> floating pointdivision
>>
>>
>> On Jan 6, 2008, at 2:44 PM, Dan Malek wrote:
>>
>>>
>>> On Jan 6, 2008, at 12:07 PM, Benjamin Herrenschmidt wrote:
>>>
>>>> It's nice to see somebody digging in that scary math emu stuff. If
>>>> you could also get rid of the warnings, it would be perfect :-)
>>>
>>> Yes, it is :-)  I didn't think it would have a life beyond MPC8xx.
>>>
>>>> .... that this code was lifted from
>>>> somewhere else (glibc ? gcc soft-float ?),
>>>
>>> It seems like a lifetime ago....  I copied the framework
>> from Sparc,
>>> and the internals from gcc soft-float.  I didn't change any of the
>>> internal emulation functions (hence, some of the warnings),
>> just the
>>> calling interface.
>>>
>>> While it's convenient, I still don't think kernel float emulation
>>> should be a solution.  The tools should generate soft-float for the
>>> applications and libraries.
>>
>> If we think this is really true, we could move to using include/math-
>> emu/* instead of the files in powerpc/math-emu.
>
> Why it's better to move to using include/math-emu.
> I found they have similar framework, is powerpc/math-emu evolved from
> include/math-emu?

* We dont really need more than one way in the kernel source tree to  
do math-emu
* include/math-emu is used by more archs so gets more review
* include/math-emu is closer to glibc soft-fp code so fixes to one  
apply cleanly to the other

- k

Re: [PATCH] Fix remainder calculating bug in single floating point division

[Kumar Gala]

On Jan 6, 2008, at 8:26 AM, Liu Yu wrote:

> This bug exists in the emulation of floating point division for  
> powerpc.
>
> The original code cannot count the remainder correctly.
> I can provide a test case to trigger this bug.
> When use fdiv to count 1.1754941e-38f / 0.9999999f,
> the result is expected to be 1.175494e-38f,
> but we will get 1.174921e-38f in the original case.
>
> Comments are always welcomed!

can you provide the test case that shows the error.

- k

RE: [PATCH] Fix remainder calculating bug in single floating point division

[Liu Yu]

>=20
> can you provide the test case that shows the error.
>=20

It seems that math-emu only be used for MPC8xx on current kernel.

---
#include <stdio.h>
#include <math.h>
#include <bits/nan.h>
#include <bits/inf.h>

void
fdiv(float f, float f1, float expected)
{
    float f2;

    f2 =3D f / f1;

    printf("fdiv %e / %e =3D %e expected %e %s \n", f, f1, f2, expected,
               (f2 =3D=3D expected) ? "(PASS)" : "(FAIL)");
}
=20
int
main(void)
{
	fdiv(1.1754941e-38f, 0.9999999f, 1.1754942e-38f);
}

Re: [PATCH] Fix remainder calculating bug in single floating point division

[David Gibson]

On Wed, Jan 09, 2008 at 09:24:07AM +0800, Liu Yu wrote:
> 
> > 
> > can you provide the test case that shows the error.
> > 
> 
> It seems that math-emu only be used for MPC8xx on current kernel.

It can be used for IBM/AMCC 4xx too.

-- 
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