RFA: Fix for rounding bug in _fpmul_parts()

Jim Wilson wilson@tuliptree.org
Wed Jun 11 06:39:00 GMT 2003


Nick Clifton wrote:
> 	* config/fp-bit.c (_fpmul_parts): Detect a situation where a
> 	double rounding could occur, and prevent it.

It seems like a flaw in the design of fp-bit that we have to do this, 
but the patch does make some sense.  There is even an #if 0 section of 
code immediately above that tries to do the same thing, though not as 
carefully as your patch.  However, the fact that this has been tried 
before and then disabled makes me wonder if your patch is really the 
right solution.  Maybe your patch will work because it was implemented 
more carefully.

If we need this for fpmul_parts, do we also need it for fpadd_parts 
and/or fpdiv_parts?  Maybe you could look into that.

There is also a copy of fp-bit in gdb in src/sim/common/sim-fpu.c.  This 
one has been rewritten quite a bit.  It looks like this one never rounds 
in the arithmetic routines.  It only rounds in the pack routines, which 
implies that we should do the same.  However, that requires changing the 
fp_number_type so it can hold unrounded numbers, and that makes it a 
much bigger change.

A build and testsuite run isn't convincing proof for a fp-bit.c change. 
like this.  You really should generate a few million (billion?) random 
nubmers and perform add/sub/mul/div operations on them, and then check 
to see if they are right, for example by using fpu hardware we trust 
(i.e. not x86).

If you really care about accuracy of FP, it is probably better to 
replace fp-bit than try to fix it.  See for instance Torbjorn Granlund's 
replacement.  Or Aldy's threats to use the kernel/glibc fp emulation code.

I have reservations about the patch, since it just seems to be papering 
over the real problem (rounding in the arith routines), and it isn't 
obvious that it safe.

I think the comments in the testcase belong in fp-bit.c.  The patch 
makes more sense after reading the comments in the testcase, so they 
belong in the source code not in the testsuite.

In all of your tests, the result is a denormalized number.  Does this 
patch work when the result is not a denormalized number?  Maybe checking 
for denorm exponents here makes it safe?

Jim



More information about the Gcc-patches mailing list