Hi Brian,

Thanks for the review; I plan to push a slightly amended version later today. The new version

* rewords the paragraph you suggested
* declares the method strictfp
* fixes a comparison that mixes integer and floating-point values
* add more test cases near the subnormal threshold

Diff of patches below.

Cheers,

-Joe
# FdLibm.java

77c97
< +        public static double compute(double x, double y) {
---
> +        public static strictfp double compute(double x, double y) {
99,107c119,134
< +            // Note: the uses of ha and hb in hypot could be
< +            // eliminated by changing the relative magnitude
< +            // comparison below to either a floating-point divide or a
< +            // comparison of getExponent results coupled initializing
< +            // t1 and t2 using a split generated by floating-point
< +            // operations. The range filtering and exponent
< +            // adjustments already done by hypot implies should a
< +            // split would not need to worry about overflow or
< +            // underflow cases.
---
> +            // Note: the ha and hb variables are the high-order
> +            // 32-bits of a and b stored as integer values. The ha and
> +            // hb values are used first for a rough magnitude
> +            // comparison of a and b and second for simulating higher
> +            // precision by allowing a and b, respectively, to be
> +            // decomposed into non-overlapping portions. Both of these
> +            // uses could be eliminated. The magnitude comparison
> +            // could be eliminated by extracting and comparing the
> +            // exponents of a and b or just be performing a
> +            // floating-point divide.  Splitting a floating-point
> +            // number into non-overlapping portions can be
> +            // accomplished by judicious use of multiplies and
> +            // additions. For details see T. J. Dekker, A Floating
> +            // Point Technique for Extending the Available Precision ,
> +            // Numerische Mathematik, vol. 18, 1971, pp.224-242 and
> +            // subsequent work.
127c154
< + if (hb <= Double.MIN_NORMAL) { // subnormal b or 0 */
---
> +                if (b < Double.MIN_NORMAL) {      // subnormal b or 0 */
169a197,205
> @@ -97,7 +239,7 @@
>       *      1. Compute and return log2(x) in two pieces:
>       *              log2(x) = w1 + w2,
>       *         where w1 has 53 - 24 = 29 bit trailing zeros.
> -     *      2. Perform y*log2(x) = n+y' by simulating muti-precision
> +     *      2. Perform y*log2(x) = n+y' by simulating multi-precision
>       *         arithmetic, where |y'| <= 0.5.
>       *      3. Return x**y = 2**n*exp(y'*log2)
>       *

# HypotTests.java

452,460c488,496
< +            {0x1.0p500,             0x1.0p440, 0x1.0p500},
< +            {0x1.0000000000001p500, 0x1.0p440, 0x1.0000000000001p500},
< +            {0x1.0p500,             0x1.0p439, 0x1.0p500},
< +            {0x1.0000000000001p500, 0x1.0p439, 0x1.0000000000001p500},
< +
< +            {0x1.0p-450,             0x1.0p-500, 0x1.0p-450},
< +            {0x1.0000000000001p-450, 0x1.0p-500, 0x1.0000000000001p-450},
< +            {0x1.0p-450,             0x1.fffffffffffffp-499, 0x1.0p-450},
< + {0x1.0000000000001p-450, 0x1.fffffffffffffp-499, 0x1.0000000000001p-450},
---
> +            {0x1.0p500,               0x1.0p440, 0x1.0p500},
> +            {0x1.0000000000001p500,   0x1.0p440, 0x1.0000000000001p500},
> +            {0x1.0p500,               0x1.0p439, 0x1.0p500},
> +            {0x1.0000000000001p500,   0x1.0p439, 0x1.0000000000001p500},
> +
> +            {0x1.0p-450,              0x1.0p-500, 0x1.0p-450},
> + {0x1.0000000000001p-450, 0x1.0p-500, 0x1.0000000000001p-450}, > + {0x1.0p-450, 0x1.fffffffffffffp-499, 0x1.0p-450}, > + {0x1.0000000000001p-450, 0x1.fffffffffffffp-499, 0x1.0000000000001p-450},
473a510,518
> +
> + {0x1.0000000000000p-1022, 0x0.fffffffffffffp-1022, 0x1.6a09e667f3bccp-1022}, > + {0x1.0000000000000p-1021, 0x0.fffffffffffffp-1022, 0x1.1e3779b97f4a8p-1021}, > + {0x1.0000000000000p-1020, 0x0.fffffffffffffp-1022, 0x1.07e0f66afed07p-1020},
> +
> + // 0x0.0000000000001P-1022 is MIN_VALUE (smallest nonzero number) > + {0x0.0000000000001p-1022, 0x0.0000000000001p-1022, 0x0.0000000000001p-1022}, > + {0x0.0000000000002p-1022, 0x0.0000000000001p-1022, 0x0.0000000000002p-1022}, > + {0x0.0000000000003p-1022, 0x0.0000000000002p-1022, 0x0.0000000000004p-1022},


-Joe

On 9/22/2015 4:20 PM, Brian Burkhalter wrote:
Hi Joe,

Overall this looks good. I only have a couple of minor observations related to internal documentation.

1. FdLibm.java

Lines 158-166: The verbiage in the note might benefit from a little reworking.

2. HypotTests.java

Line 46: “Commutative” is misspelled.

Thanks,

Brian

On Sep 21, 2015, at 6:22 PM, Joseph D. Darcy <[email protected] <mailto:[email protected]>> wrote:

Please review the next portion of the port of fdlibm to Java:

   JDK-7130085 Port fdlibm hypot to Java
http://cr.openjdk.java.net/~darcy/7130085.0/ <http://cr.openjdk.java.net/%7Edarcy/7130085.0/>

As before with pow, this isn't necessarily the end state of the code we'd like to stop at, but it should be sufficiently idiomatic Java for an initial port.

To make comparison against the original C algorithm easier, I listed the C version of hypot as the base file to compare FdLibm.java against.


Reply via email to