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.