[PATCH] Fix bug on soft emulation of float point in gcc/longlong.c

2008-03-12 Thread Liu Yu
There are 2 bugs existing in __udiv_qrnnd_c:

1. remainder could not be counted correctly
__r1 and __r0 are not adding enough, so I use do..while to replace former if

2. the case of overflow of __m are not considered
so I add a couple of lines to handle it

I found the bugs from Linux kernel,
for PowerPC math-emu use __udiv_qrnnd_c to emulate float div.

I sent the patch to the maintainers of kernel, but they replied to me that
this code came from gcc and should better keep coincident.
So I think I should send patch to here.

I don't know how to trigger __udiv_qrnnd_c in gcc.
but if the argument of __udiv_qrnnd_c are like
0x07f8 0x07f8 0x00210fff 0xc000 0x07f8,
the bug will be reproduced.

Also, you can try the program below, I have test it on Linux kernel math-emu 
but haven't test on gcc.

---
#include 

union fu {
unsigned int u;
float f;
};

union fu fdiv(union fu a, union fu b, union fu expected)
{
union fu z;
z.f = a.f / b.f;

printf("fdiv %08x / %08x = %08x expected %08x %s\n",
a.u, b.u, z.u, expected.u,
(z.u == expected.u) ? "(PASS)" : "(FAIL)");
printf(" %e / %e = %e\n", a.f, b.f, z.f);
}

int main()
{
union fu fa,fb,fe;

fa.u = 0xc0843fff;
fb.u = 0x80ff;
fe.u = 0x7f044000;
    fdiv(fa,fb,fe);
}

---


Signed-off-by: Liu Yu <[EMAIL PROTECTED]>
---
 gcc/longlong.h |   18 --
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/gcc/longlong.h b/gcc/longlong.h
index 6c24564..256399b 100644
--- a/gcc/longlong.h
+++ b/gcc/longlong.h
@@ -1368,12 +1368,15 @@ UDItype __umulsidi3 (USItype, USItype);
 __q1 = (n1) / __d1;
\
 __m = (UWtype) __q1 * __d0;
\
 __r1 = __r1 * __ll_B | __ll_highpart (n0); \
+if (__d0 && __q1 > __m / __d0) \
+   do {\
+   __q1--, __r1 += (d);\
+   } while (__r1 >= (d));  \
 if (__r1 < __m)\
   {
\
-   __q1--, __r1 += (d);\
-   if (__r1 >= (d)) /* i.e. we didn't get carry when adding to __r1 */\
- if (__r1 < __m)   \
+   do {\
__q1--, __r1 += (d);\
+   } while (__r1 >= (d) && __r1 < __m);\
   }
\
 __r1 -= __m;   \
\
@@ -1381,12 +1384,15 @@ UDItype __umulsidi3 (USItype, USItype);
 __q0 = __r1 / __d1;
\
 __m = (UWtype) __q0 * __d0;
\
 __r0 = __r0 * __ll_B | __ll_lowpart (n0);  \
+if (__d0 && __q0 > __m / __d0) \
+   do {\
+   __q0--, __r0 += (d);\
+   } while (__r0 >= (d));  \
 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



[PATCH] Fix bug on soft emulation of float point in gcc/longlong.c

2008-03-12 Thread Liu Yu

There are 2 bugs existing in __udiv_qrnnd_c:

1. remainder could not be counted correctly
__r1 and __r0 are not adding enough, so I use do..while to replace
former if

2. the case of overflow of __m are not considered
so I add a couple of lines to handle it

I found the bugs from Linux kernel,
for PowerPC math-emu use __udiv_qrnnd_c to emulate float div.

I sent the patch to the maintainers of kernel, but they replied to me
that
this code came from gcc and should better keep coincident.
So I think I should send patch to here.

I don't know how to trigger __udiv_qrnnd_c in gcc.
but if the argument of __udiv_qrnnd_c are like
0x07f8 0x07f8 0x00210fff 0xc000 0x07f8,
the bug will be reproduced.

Also, you can try the program below, I have test it on Linux kernel
math-emu but haven't test on gcc.

---
#include 

union fu {
unsigned int u;
float f;
};

union fu fdiv(union fu a, union fu b, union fu expected)
{
union fu z;
z.f = a.f / b.f;

printf("fdiv %08x / %08x = %08x expected %08x %s\n",
a.u, b.u, z.u, expected.u,
(z.u == expected.u) ? "(PASS)" : "(FAIL)");
printf(" %e / %e = %e\n", a.f, b.f, z.f);
}

int main()
{
union fu fa,fb,fe;

fa.u = 0xc0843fff;
fb.u = 0x80ff;
fe.u = 0x7f044000;
fdiv(fa,fb,fe);
}
--


---
 gcc/longlong.h |   18 --
 1 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/gcc/longlong.h b/gcc/longlong.h
index 6c24564..256399b 100644
--- a/gcc/longlong.h
+++ b/gcc/longlong.h
@@ -1368,12 +1368,15 @@ UDItype __umulsidi3 (USItype, USItype);
 __q1 = (n1) / __d1;
\
 __m = (UWtype) __q1 * __d0;
\
 __r1 = __r1 * __ll_B | __ll_highpart (n0);
\
+if (__d0 && __q1 > __m / __d0)
\
+   do {
\
+   __q1--, __r1 += (d);
\
+   } while (__r1 >= (d));
\
 if (__r1 < __m)
\
   {
\
-   __q1--, __r1 += (d);
\
-   if (__r1 >= (d)) /* i.e. we didn't get carry when adding to __r1
*/\
- if (__r1 < __m)
\
+   do {
\
__q1--, __r1 += (d);
\
+   } while (__r1 >= (d) && __r1 < __m);
\
   }
\
 __r1 -= __m;
\
 
\
@@ -1381,12 +1384,15 @@ UDItype __umulsidi3 (USItype, USItype);
 __q0 = __r1 / __d1;
\
 __m = (UWtype) __q0 * __d0;
\
 __r0 = __r0 * __ll_B | __ll_lowpart (n0);
\
+if (__d0 && __q0 > __m / __d0)
\
+   do {
\
+   __q0--, __r0 += (d);
\
+   } while (__r0 >= (d));
\
 if (__r0 < __m)
\
   {
\
-   __q0--, __r0 += (d);
\
-   if (__r0 >= (d))
\
- if (__r0 < __m)
\
+   do {
\
__q0--, __r0 += (d);
\
+   } while (__r0 >= (d) && __r0 < __m);
\
   }
\
 __r0 -= __m;
\
 
\
-- 




RE: [PATCH] Fix bug on soft emulation of float point in gcc/longlong.c

2008-03-12 Thread Liu Yu

Hi Ian,

Thanks a lot for your detailed explanation.

> -Original Message-
> From: Ian Lance Taylor [mailto:[EMAIL PROTECTED] 
> Sent: Thursday, March 13, 2008 1:45 PM
> To: Liu Yu
> Cc: gcc@gcc.gnu.org
> Subject: Re: [PATCH] Fix bug on soft emulation of float point 
> in gcc/longlong.c
> 
> Liu Yu <[EMAIL PROTECTED]> writes:
> 
> > There are 2 bugs existing in __udiv_qrnnd_c:
> >
> > 1. remainder could not be counted correctly
> > __r1 and __r0 are not adding enough, so I use do..while to replace 
> > former if
> >
> > 2. the case of overflow of __m are not considered so I add 
> a couple of 
> > lines to handle it
> >
> > I found the bugs from Linux kernel,
> > for PowerPC math-emu use __udiv_qrnnd_c to emulate float div.
> 
> Is the kernel code testing UDIV_NEEDS_NORMALIZATION and 
> implementing the normalization required before calling __udiv_qrnnd_c?
> 
> 
> > I don't know how to trigger __udiv_qrnnd_c in gcc.
> 
> gcc will use __udiv_qrnnd_c in -lgcc for division and modulos 
> of 64-bit values on processors which can't do that directly 
> and for which there is no alternate implementation in longlong.h.
> 
> 
> > but if the argument of __udiv_qrnnd_c are like
> > 0x07f8 0x07f8 0x00210fff 0xc000 0x07f8, the 
> bug will 
> > be reproduced.
> 
> Those arguments to __udiv_qrnnd_c are not valid.  For correct 
> operation, the most significant bit of the last argument is 
> required to be 1.  UDIV_NEEDS_NORMALIZATION signals this 
> fact.  In gcc the required normalization is implemented 
> before calling __udiv_qrnnd_c, in __udivmoddi4 in libgcc2.c.
> 
> This normalization approach is taken, rather than introducing 
> while loops as you suggest, because the while loops can run 
> for quite a long time on small numbers.
> 
> Ian
>