[Qemu-devel] [PATCH] powerpc: fix denorm float->double conversion

[Sergei Trofimovich]

The bug is initially discovered in GHC test suite. Here is minimal reproducer:

```c

int main() {
    volatile float f;
    volatile double d;

    *(volatile uint32_t*)&f = 0xc0de;
    d = f;
    printf("f  = %#x\n", *(volatile uint32_t*)&f);
    printf("d  = %#llx (expect 0x37981bc000000000)\n",
        *(volatile uint64_t*)&d);
    printf("d  = %e\n", d);
    f = d;
    printf("f  = %#x\n", *(volatile uint32_t*)&f);
}
```

```
$ powerpc-unknown-linux-gnu-gcc -O2 a.c -Wall -o a \
    -fno-strict-aliasing -static && qemu-ppc ./a
f  = 0xc0de
d  = 0x37a00000000c0de0 (expect 0x37981bc000000000)
d  = 9.183550e-41
f  = 0x10000
```

Here denormalization conversion has a few bugs:
- significand (abs_arg) has 32-bit unsigned wraparound in
    ret |= abs_arg << (shift + 29);
- significand does not drop explicit leading '1' in denorm
  'float' when converting to normalized 'double'
- significand had an off-by-one shift

CC: Richard Henderson <richard.hender...@linaro.org>
CC: David Gibson <da...@gibson.dropbear.id.au>
CC: qemu-...@nongnu.org
CC: qemu-devel@nongnu.org
Bug: https://bugs.launchpad.net/qemu/+bug/1821444
Signed-off-by: Sergei Trofimovich <sly...@gentoo.org>
---
 target/ppc/fpu_helper.c | 32 +++++++++++++++++++++++++++-----
 1 file changed, 27 insertions(+), 5 deletions(-)

diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c
index 2ed4f42275..1e8b014890 100644
--- a/target/ppc/fpu_helper.c
+++ b/target/ppc/fpu_helper.c
@@ -64,13 +64,35 @@ uint64_t helper_todouble(uint32_t arg)
         ret |= (uint64_t)extract32(arg, 0, 30) << 29;
     } else {
         /* Zero or Denormalized operand.  */
-        ret = (uint64_t)extract32(arg, 31, 1) << 63;
+
+        /*
+         * Conversion mechanics:
+         * float denorm (2^(-126) - biased):
+         *    [ sign (1 bit) | exp32 (8 bits)  | sign32 (23 bits) ]
+         *                 s                0    0001abc...def
+         * double norm (2^(-1023) - biased):
+         *    [ sign (1 bit) | exp64 (11 bits) | sign64 (52 bits) ]
+         *                 s              exp    abc...def 00..0
+         * Thus we are performing the following conversion steps:
+         * 1. preserve the sign
+         * 2. normalize denorm sign32:
+         *   2a. drop explicit leading '1' as normalized numbers
+         *       don't contain it
+         *   2b. calculate the bit-shift needed to match implicit '1'
+         * 3. calculate 'exp64' as bias delta plus denorm offset
+         * 4. put calculated 'sign64' into new location
+         */
+        ret = (uint64_t)extract32(arg, 31, 1) << 63; /* [1.] */
         if (unlikely(abs_arg != 0)) {
             /* Denormalized operand.  */
-            int shift = clz32(abs_arg) - 9;
-            int exp = -126 - shift + 1023;
-            ret |= (uint64_t)exp << 52;
-            ret |= abs_arg << (shift + 29);
+            int lz = clz32(abs_arg);
+            abs_arg &= ~(1 << (31 - lz)); /* [2a.] */
+
+            /* shift within sign32 includeing leading '1' */
+            int shift = lz + 1 - (32 - 23);
+            int exp = -126 + 1023 - shift; /* [2b]. */
+            ret |= (uint64_t)exp << 52; /* [3.] */
+            ret |= (uint64_t)abs_arg << (52 - 23 + shift); /* [4.] */
         }
     }
     return ret;
-- 
2.21.0