Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-13 Thread Vitaly Davidovich
x == Integer.MIN_VALUE should be faster than x == -x as it's a cmp against a constant whereas the latter requires negating x (that's a dependency too), tying up a register to store the negation, and then doing the cmp. Sent from my phone On Feb 3, 2012 12:53 PM, "Eamonn McManus" wrote: > My init

Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-13 Thread Eamonn McManus
Hacker's Delight gives a formula equivalent to this one on page 27: ((x ^ r) & (y ^ r)) < 0 Having said that, I think Florian's assertion that such a rewrite will be faster needs proof. In the original form... (x ^ r) < 0 && (x ^ y) >= 0 ...the first condition will be false half the time for ra

Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-06 Thread Tom Rodriguez
On Feb 3, 2012, at 8:37 PM, Krystal Mok wrote: > Hi Tom, > > Yes, I'm sorry for not including enough context when cc'ing. > > The original discussion thread starts from [1]. The webrev is at [2]. > > For the string concats in multiplyExact(): > * the StringBuilder doesn't escape > * it's only

Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-06 Thread Stephen Colebourne
On 6 February 2012 21:15, Roger Riggs wrote: >> Separately but related, there are missing methods in BigInteger, >> longValueExact() and intValueExact() > > BigInteger provides a bitLength method than be tested to see if the result > would overflow.  That's a better API than an ArithmeticException

Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-06 Thread Roger Riggs
Éamonn, Thanks for the detailed review. I did some simple performance tests with and without the optimization test. With the optimization, performance was nearly doubled. The individual tests for x or y ==1 did not improve performance. Those cases are likely optimized inside the divide operat

Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-06 Thread Roger Riggs
On 02/03/2012 10:23 AM, Stephen Colebourne wrote: I prefer the method naming safeAdd/safeNegate/safeMultiply (I think they are a lot clearer), but can live with the ones proposed (which are somewhat linked to BigInteger/BigDecimal). The method names should start with the operation. I would like

Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-04 Thread Eamonn McManus
Concerning the long multiplyExactly, I have a number of comments. public static long multiplyExact(long x, long y) { long r = x * y; long ax = Math.abs(x); long ay = Math.abs(y); if (((ax | ay) >>> 31 == 0) || (x == 1) || (y == 1)) { return r;

Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-04 Thread Roger Riggs
The methods for increment, decrement, and also negation test for exactly one value at the extremities of the value range. The verbosity of method calls in the source code and the overhead in the execution make these a poor choice for developers. If the code must really operate correctly at the ex

Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-03 Thread Krystal Mok
Hi Tom, Yes, I'm sorry for not including enough context when cc'ing. The original discussion thread starts from [1]. The webrev is at [2]. For the string concats in multiplyExact(): * the StringBuilder doesn't escape * it's only concatenating Strings and ints which should meet the requirements

Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-03 Thread Vitaly Davidovich
Yeah my comment was more general - I didn't look at the entire context, just wasn't sure why you thought that comparison would be faster (nevermind that it's wrong as you mention :)). Sent from my phone On Feb 3, 2012 1:58 PM, "Eamonn McManus" wrote: > On 3 February 2012 10:22, Vitaly Davidovich

Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-03 Thread Eamonn McManus
On 3 February 2012 10:22, Vitaly Davidovich  wrote: > x == Integer.MIN_VALUE should be faster than x == -x as it's a cmp against a >constant whereas the latter requires negating x (that's a dependency too), >tying up a register to store the negation, and then doing the cmp. The value -x is neede

Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-03 Thread Stephen Colebourne
On 3 February 2012 17:52, Eamonn McManus wrote: > I agree with Stephen Colebourne that brief implementation comments would be > useful. But I disagree with his proposed further methods in Math > (increment, decrement, int+long variants), which I don't think would pull > their weight. FWIW, JSR-31

Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-03 Thread Eamonn McManus
My initial remarks: In negateExact, the condition x == -x should be faster to evaluate than x == Integer.MIN_VALUE and reflects the intent just as well. In addExact and subtractExact, I would be inclined to implement the int versions using long arithmetic, like this: long lr = x + y; int r = (in

Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-03 Thread Tom Rodriguez
On Feb 3, 2012, at 12:38 AM, Krystal Mok wrote: > Hi Florian, > > On Fri, Feb 3, 2012 at 4:11 PM, Florian Weimer wrote: > Will Hotspot be able to optimize away the string construction on the > exception path in multiplyExact() if the exception is caught locally? > > At least -XX:+OptimizeStrin

Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-03 Thread Joe Darcy
On 02/03/2012 06:44 AM, Florian Weimer wrote: * Roger Riggs: The boolean expression can be refactored. (Only bit-31 is significant). But it becomes pretty inscrutable. (x ^ r)< 0&& (x ^ y)>= 0 becomes (x ^ r)< 0&& (~(x ^ y))< 0 becomes ((x ^ r)& ~(x ^ y))< 0 That's what I go

Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-03 Thread Stephen Colebourne
I prefer the method naming safeAdd/safeNegate/safeMultiply (I think they are a lot clearer), but can live with the ones proposed (which are somewhat linked to BigInteger/BigDecimal). I would like to see source code comments explaining why the algorithm works in the final version in OpenJDK. Thats

Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-03 Thread Florian Weimer
* Roger Riggs: > The boolean expression can be refactored. (Only bit-31 is significant). > But it becomes pretty inscrutable. > > (x ^ r)< 0&& (x ^ y)>= 0 becomes > > (x ^ r)< 0&& (~(x ^ y))< 0 becomes > > ((x ^ r)& ~(x ^ y))< 0 That's what I got in my second attempt, too. It doesn't res

Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-03 Thread Roger Riggs
The boolean expression can be refactored. (Only bit-31 is significant). But it becomes pretty inscrutable. (x ^ r)< 0&& (x ^ y)>= 0 becomes (x ^ r)< 0&& (~(x ^ y))< 0 becomes ((x ^ r)& ~(x ^ y))< 0 Roger On 02/03/2012 07:12 AM, Florian Weimer wrote: * Eamonn McManus: On 3 Februa

Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-03 Thread Florian Weimer
* Eamonn McManus: > On 3 February 2012 00:11, Florian Weimer wrote: >> Would it make sense to replace (x ^ r) < 0 && (x ^ y) >= 0 >> with (x ^ y ^ r) < 0? > > That would not be correct. For example, it would signal overflow for -1 + 1. Oops. But there has to be a way to implement this using jus

Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-03 Thread Eamonn McManus
On 3 February 2012 00:11, Florian Weimer wrote: > Would it make sense to replace (x ^ r) < 0 && (x ^ y) >= 0 > with (x ^ y ^ r) < 0? That would not be correct. For example, it would signal overflow for -1 + 1. Éamonn On 3 February 2012 00:11, Florian Weimer wrote: > * Roger Riggs: > > > to s

Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-03 Thread Krystal Mok
Hi Florian, On Fri, Feb 3, 2012 at 4:11 PM, Florian Weimer wrote: > Will Hotspot be able to optimize away the string construction on the > exception path in multiplyExact() if the exception is caught locally? > > At least -XX:+OptimizeStringConcat should remove the need to construct a StringBuil

Re: Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-03 Thread Florian Weimer
* Roger Riggs: > to support addExact(), subtractExact(), negateExact(), multiplyExact(), > and toIntExact() for int and long primitive types. Would it make sense to replace (x ^ r) < 0 && (x ^ y) >= 0 in +public static int addExact(int x, int y) { +int r = x + y; +if ((x ^ r)

Need reviewer: JDK 8 CR for Support Integer overflow

2012-02-02 Thread Roger Riggs
There is a need for arithmetic operations that throw exceptions when the results overflow the representation of int or long. The CR is 6708398: Support integer overflow Please review this webrev