[PATCH] 4511638: Double.toString(double) sometimes produces incorrect results

[Andrew Haley]

On 09/26/2018 06:39 PM, raffaello.giulietti at gmail.com wrote:

> The submitted code contains both the changes to the current
> implementation and extensive jtreg tests.
>
> While I've struggled to keep the code within the 80 chars/line limit,
> mercurial still generates longer lines. Thus, to avoid possible problems
> with the email systems, the code is submitted both inline and as an
> attachment. Hope at least one copy makes its way without errors.

Overall, the commenting is much too light. There are many places
where I think I know what you're doing but you don't explain it.

Here, for example:

> +
> +        // pow5 = pow51*2^63 + pow50
> +        long pow51 = ceilPow5dHigh(-k);
> +        long pow50 = ceilPow5dLow(-k);
> +
> +        // p = p2*2^126 + p1*2^63 + p0 and p = pow5 * cb
> +        long x0 = pow50 * cb;
> +        long x1 = multiplyHigh(pow50, cb);
> +        long y0 = pow51 * cb;
> +        long y1 = multiplyHigh(pow51, cb);
> +        long z = (x1 << 1 | x0 >>> 63) + (y0 & MASK_63);
> +        long p0 = x0 & MASK_63;
> +        long p1 = z & MASK_63;
> +        long p2 = (y1 << 1 | y0 >>> 63) + (z >>> 63);
> +        long vn = p2 << 1 + ord2alpha | p1 >>> 62 - ord2alpha;
> +        if ((p1 & mask) != 0 || p0 >= threshold) {
> +            vn |= 1;
> +        }

... etc. I think I can figure out what you're doing, but you could
explain it.

If you write the comments now while the code is still fresh in your
mind it'll be easy.

> +    private static final long[] ceilPow5d = {
> +        /* -292 */ 0x7FBB_D8FE_5F5E_6E27L, 0x497A_3A27_04EE_C3DFL,
> +        /* -291 */ 0x4FD5_679E_FB9B_04D8L, 0x5DEC_6458_6315_3A6CL,
> +        /* -290 */ 0x63CA_C186_BA81_C60EL, 0x7567_7D6E_7BDA_8906L,
> +        /* -289 */ 0x7CBD_71E8_6922_3792L, 0x52C1_5CCA_1AD1_2B48L,

What exactly is this table anyway?  How was it generated?  Please say.

There are many more places in the code. What you've done is nice, but
it could be exemplary.

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671