Hi Paul,

On Feb 5, 2015, at 6:06 AM, Paul Sandoz <[email protected]> wrote:

> 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.

I had actually wondered whether this could just be a “trusted” method since the 
check of the first parameter is already done in the calling code, then the 
if-branch could be removed altogether, but I suppose the assert is safer.

> 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;

Thanks for the suggestions. Please see updated patch:

http://cr.openjdk.java.net/~bpb/8066842/webrev.01/

Brian

Reply via email to