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