Hi Brian.
I don't claim to understand the fine details of these methods but i can see how
the new method avoid loosing bits.
4947 private static long[] divRemNegativeLong(long n, long d) {
4948 if (n >= 0) {
4949 throw new IllegalArgumentException("Non-negative numerator");
4950 } else if (d == 1) {
4951 return new long[]{0, n};
4952 }
Why not use an assert instead of an IAE since this is an internal method. Also
the case of d ==1 could be pulled out just like for the case of tmp being +ve:
if (v1 == 1) {
q1 = tmp;
r_tmp = 0;
} else if (tmp >= 0) {
...
} else {
...
}
then the asserts would be:
assert n < 0 : n;
assert d != 1 : d;
Paul.
On Jan 16, 2015, at 10:18 PM, Brian Burkhalter <[email protected]>
wrote:
> Hello,
>
> Please review at your convenience.
>
> Issue: https://bugs.openjdk.java.net/browse/JDK-8066842
> Patch: http://cr.openjdk.java.net/~bpb/8066842/webrev.00/
>
> The problem appears to have been that at line 4941 of the old source, in the
> divWord() method, one or both of the long variables ‘r’ and ‘q’ overflowed
> the range of int so that information was lost when these variables were
> truncated to 32 bits. The code of divWord() seems to have been ported from a
> method of the same name in MutableBigInteger wherein its constraints were
> made explicit. In the patch, divWord() is replaced by divRemNegativeLong(),
> and the use of the former in divideAndRound128() replaced with inline code
> for the non-negative dividend cases, and by divRemNegativeLong() for the
> negative dividend cases.
>
> Thanks,
>
> Brian
