[PATCH] math-emu: correct test for downshifting fraction in _FP_FROM_INT()

[Mikael Pettersson]

Mikael Pettersson writes:
 > The kernel's math-emu code contains a macro _FP_FROM_INT() which is
 > used to convert an integer to a raw normalized floating-point value.
 > It does this basically in three steps:
 > 
 > 1. Compute the exponent from the number of leading zero bits.
 > 2. Downshift large fractions to put the MSB in the right position
 >    for normalized fractions.
 > 3. Upshift small fractions to put the MSB in the right position.
 > 
 > There is an boundary error in step 2, causing a fraction with its
 > MSB exactly one bit above the normalized MSB position to not be
 > downshifted.  This results in a non-normalized raw float, which when
 > packed becomes a massively inaccurate representation for that input.
 > 
 > The impact of this depends on a number of arch-specific factors,
 > but it is known to have broken emulation of FXTOD instructions
 > on UltraSPARC III, which was originally reported as GCC bug 44631
 > <http://gcc.gnu.org/bugzilla/show_bug.cgi?id=44631>.
 > 
 > Any arch which uses math-emu to emulate conversions from integers to
 > same-size floats may be affected.
 > 
 > The fix is simple: the exponent comparison used to determine if the
 > fraction should be downshifted must be "<=" not "<".
 > 
 > I'm sending a kernel module to test this as a reply to this message.
 > There are also SPARC user-space test cases in the GCC bug entry.

And here's the test module.

To illustrate the bug it uses math-emu to convert a series of
single-bit power-of-two ints to corresponding floats and back,
and prints the intermediate representations and the differences
between the original values and the final ones.  These ints all
have exact float representations so there should be no differences.

But for 0x08000000 the conversion goes wrong.  The test case
then converts a few more numbers near this one and up to the next
power-of-two which did convert Ok.

With an unpatched kernel you get the following kernel messages
after insmod:

mathemu_test: init
test_itof: 0x40000000 -> 0x4e800000 -> 0x40000000, diff 0
test_itof: 0x20000000 -> 0x4e000000 -> 0x20000000, diff 0
test_itof: 0x10000000 -> 0x4d800000 -> 0x10000000, diff 0
test_itof: 0x08000000 -> 0x4d800000 -> 0x10000000, diff 134217728
test_itof: 0x04000000 -> 0x4c800000 -> 0x04000000, diff 0
test_itof: 0x02000000 -> 0x4c000000 -> 0x02000000, diff 0
test_itof: 0x01000000 -> 0x4b800000 -> 0x01000000, diff 0
test_itof: 0x00800000 -> 0x4b000000 -> 0x00800000, diff 0
test_itof: 0x0f000000 -> 0x4de00000 -> 0x1c000000, diff 218103808
test_itof: 0x0e000000 -> 0x4dc00000 -> 0x18000000, diff 167772160
test_itof: 0x0d000000 -> 0x4da00000 -> 0x14000000, diff 117440512
test_itof: 0x0c000000 -> 0x4d800000 -> 0x10000000, diff 67108864
test_itof: 0x0b000000 -> 0x4de00000 -> 0x1c000000, diff 285212672
test_itof: 0x0a000000 -> 0x4dc00000 -> 0x18000000, diff 234881024
test_itof: 0x09000000 -> 0x4da00000 -> 0x14000000, diff 184549376
test_itof: 0x08000000 -> 0x4d800000 -> 0x10000000, diff 134217728
test_itof: 0x07000000 -> 0x4ce00000 -> 0x07000000, diff 0

With the patch applied, you instead get this:

mathemu_test: init
test_itof: 0x40000000 -> 0x4e800000 -> 0x40000000, diff 0
test_itof: 0x20000000 -> 0x4e000000 -> 0x20000000, diff 0
test_itof: 0x10000000 -> 0x4d800000 -> 0x10000000, diff 0
test_itof: 0x08000000 -> 0x4d000000 -> 0x08000000, diff 0
test_itof: 0x04000000 -> 0x4c800000 -> 0x04000000, diff 0
test_itof: 0x02000000 -> 0x4c000000 -> 0x02000000, diff 0
test_itof: 0x01000000 -> 0x4b800000 -> 0x01000000, diff 0
test_itof: 0x00800000 -> 0x4b000000 -> 0x00800000, diff 0
test_itof: 0x0f000000 -> 0x4d700000 -> 0x0f000000, diff 0
test_itof: 0x0e000000 -> 0x4d600000 -> 0x0e000000, diff 0
test_itof: 0x0d000000 -> 0x4d500000 -> 0x0d000000, diff 0
test_itof: 0x0c000000 -> 0x4d400000 -> 0x0c000000, diff 0
test_itof: 0x0b000000 -> 0x4d300000 -> 0x0b000000, diff 0
test_itof: 0x0a000000 -> 0x4d200000 -> 0x0a000000, diff 0
test_itof: 0x09000000 -> 0x4d100000 -> 0x09000000, diff 0
test_itof: 0x08000000 -> 0x4d000000 -> 0x08000000, diff 0
test_itof: 0x07000000 -> 0x4ce00000 -> 0x07000000, diff 0

Unfortunately it seems difficult to write a generic module
which uses math-emu:
- <math-emu/soft-fp.h> includes <asm/sfp-machine.h>,
  but only a handful of archs have it
- <asm/sfp-machine.h> isn't always self-contained and may depend
  on various $arch-specific declarations being present

The given test module works on sparc64 and ppc64, where it uses
the kernel's sfp-machine.h, and on x86 where it uses a stub
sfp-machine.h supplied by itself.  I tried to cross-compile it
for alpha, but that failed due to its sfp-machine.h not being
self-contained.  I didn't try sh or s390.

/Mikael

-------------- next part --------------
A non-text attachment was scrubbed...
Name: mathemu_test.tar.gz
Type: application/octet-stream
Size: 1656 bytes
Desc: math-emu test module
URL: <http://lists.ozlabs.org/pipermail/linuxppc-dev/attachments/20100720/5bc5d461/attachment.obj>