I think that the final webrev http://cr.openjdk.java.net/~darcy/4851642.2/
is good.
Thanks.
On Fri, Apr 15, 2016 at 8:00 AM, joe darcy <joe.da...@oracle.com> wrote:
> Hi Dmitry,
>
> On 4/14/2016 7:43 PM, Dmitry Nadezhin wrote:
>
> Hi Joe,
>
> It looks good except a guard expression in the line Math:1550
> (tmp == 0.0 && a == 0.0 || b == 0.0)
> Variables a and b occur asymmetrically in this expression.
> A symmetrical expression would be either
> (a == 0.0 || b == 0.0)
> or
> (product.signum() == 0)
>
>
> Thank you for the careful review. I wrote some additional tests to cover
> those cases. I believe the current code is functionally correct since (a ==
> 0.0 || b == 0.0) implies tmp == 0.0, but I've changed the guard to
>
> (a == 0.0 || b == 0.0)
>
> and added some more comments. The code in question is now:
>
> } else { // All inputs finite
> BigDecimal product = (new BigDecimal(a)).multiply(new
> BigDecimal(b));
> if (c == 0.0) { // Positive or negative zero
> // If the product is an exact zero, use a
> // floating-point expression to compute the sign
> // of the zero final result. The product is an
> // exact zero if and only if at least one of a and
> // b is zero.
> if (a == 0.0 || b == 0.0) {
> return a * b + c;
> } else {
> // The sign of a zero addend doesn't matter if
> // the product is nonzero. The sign of a zero
> // addend is not factored in the result if the
> // exact product is nonzero but underflows to
> // zero; see IEEE-754 2008 section 6.3 "The
> // sign bit".
> return product.doubleValue();
> }
> } else {
> return product.add(new BigDecimal(c)).doubleValue();
> }
> }
>
> Final webrev with minor adjustments at
>
> http://cr.openjdk.java.net/~darcy/4851642.2/
>
> Thanks,
>
> -Joe
>
>
> On Fri, Apr 15, 2016 at 3:14 AM, joe darcy <joe.da...@oracle.com> wrote:
>
>> Hello,
>>
>> In response to the review comments from Dmitry and Brian, I've prepared
>> another iteration of the webrev:
>>
>> http://cr.openjdk.java.net/~darcy/4851642.1/
>>
>> Summary of the changes:
>>
>> * Renamed the method to "fma" to follow the precedent of the C library.
>> * Added API note citing IEEE 754 operation.
>> * More test cases
>> * More comments
>> * Restructured handling of finite double value to be more straightforward.
>>
>> Thanks,
>>
>> -Joe
>>
>>
>> On 4/12/2016 5:21 PM, joe darcy wrote:
>>
>>> Hello,
>>>
>>> Please review the changes for
>>>
>>> JDK-4851642: Add fused mac to Java math library
>>> http://cr.openjdk.java.net/~darcy/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). The fused mac
>>> operation was added in the 2008 update to the IEEE 754 floating-point
>>> standard and hardware support for the operation is becoming more and more
>>> common in different processor families.
>>>
>>> When present as a hardware instruction, a fused mac can speed up loops
>>> such as those for polynomial evaluation. A fused mac can also be used to
>>> support a correctly rounding floating-point divide and support various
>>> higher-precision operations such as "doubled-double" arithmetic.
>>>
>>> With the increasing availability of fused mac as a hardware primitive,
>>> the time has come to add fused mac to the Java math library. Fused mac is
>>> an ideal candidate to be intrinsified where hardware support is available.
>>> However, this initial implementation does not attempt to add any such
>>> intrinsics support in HotSpot; a follow-up RFE has been filed for that work
>>> (JDK-8154122). The current library implementation favors code simplicity
>>> over speed; a more performant implementation could be written by directly
>>> decomposing the floating-point inputs rather than turning to BigDecimal and
>>> may be written in the future. More extensive tests could be added in the
>>> future as well.
>>>
>>> Thanks,
>>>
>>> -Joe
>>>
>>
>>
>
>
