Hi Brian and Dmitry,
On 4/13/2016 12:43 PM, Brian Burkhalter wrote:
Joe / Dmitry,
On Apr 12, 2016, at 5:21 PM, joe darcy <joe.da...@oracle.com
<mailto:joe.da...@oracle.com>> wrote:
Please review the changes for
JDK-4851642: Add fused mac to Java math library
http://cr.openjdk.java.net/~darcy/4851642.0/
<http://cr.openjdk.java.net/%7Edarcy/4851642.0/>
Fused mac (multiply-accumulate) is a ternary floating-point operation
which accepts three inputs, a, b, c, and computes
a * b + c
with a single rounding error rather than the usual two rounding
errors (a first for the multiply, a second one for the add).
A couple of points of curiosity. Firstly, is this not “fused
multiply-add” rather than “fused multiply-accumulate?” Secondly, why
the choice of name “fusedMac()” instead of the common “fma()” or the
longer but perhaps clearer “fusedMultiplyAdd()?”
On naming, there are a few candidates. As background, I'll note that the
naming in java.lang.Math at times follows the C-style naming ("cos"
rather than "cosine"), but that methods we've added more recently
outside of the traditional C math.h have followed more Java-style
conventions. FWIW, C99 calls this "fma".
So, "fma()" is a possible choice, certainly concise, but I don't think
many people would find it very suggestive as to what it does, at least
not with the current familiarity with fused multiply-add.
In the IEEE 754 2008 standard, the operation is spelled out as
"fusedMultiplyAdd", but that is a bit long.
The "multiply accumulate" term is how I first heard of the operation and
there is some other usage of it
(https://en.wikipedia.org/wiki/Multiply%E2%80%93accumulate_operation),
but "fused multiply add" is also an accurate description.
A picayune javadoc point: in the unordered lists <ul></ul> should
“</li>” be used to close the list items?
I don't think that is necessary. The javadoc does pass fine through
doclint without warnings.
On Apr 13, 2016, at 5:41 AM, Dmitry Nadezhin
<dmitry.nadez...@gmail.com <mailto:dmitry.nadez...@gmail.com>> wrote:
I concur with Dmitry’s points. With respect to the second one,
2) Lines Math:1508-1525 could be simpler:
====
double result = a * b + c;
return Double.isNaN(result) && Double.isFinite(a) &&
Double.isFinite(b)
? c : result;
====
not trusting this to my own visual inspection, I wrote nested loops to
run a, b, and c over
double[] values = new double[] {
-0.0, 0.0, +0.0, -42.0, 42.0,
-Double.MAX_VALUE, Double.MAX_VALUE,
Double.NaN, Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY
};
and there is no difference between the webrev and Dmitry’s suggestion
above for the cases where
!Double.isFinite(a) || !Double.isFinite(b) || !Double.isFinite(c) == true
As part of favoring "simplicity over speed," I intentionally wrote the
code in a way that tried to follow the structure of the specification in
a straightforward manner. For example, the spec says
"<li> If any argument is NaN, the result is NaN."
and there is code
1508 if (Double.isNaN(a) || Double.isNaN(b) ||
Double.isNaN(c)) {
1509 return Double.NaN;
and the spec says
1472 * <li> If one of the first two arguments is infinite and the
1473 * other is zero, the result is NaN.
and there is code
1511 if ((Double.isInfinite(a) && b == 0.0) ||
1512 (Double.isInfinite(b) && a == 0.0)) {
1513 return Double.NaN;
etc.
For this initial implementation, I think this kind of simplicity is
desirable. Longer term, I wouldn't be surprised if this implementation
was retired out to be a reference implementation for additional
regression tests.
Thanks,
-Joe
