[PATCH] Fix bug on soft emulation of float point in gcc/longlong.c
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
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
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 >