On Fri, 2 May 2025 17:30:55 GMT, fabioromano1 <d...@openjdk.org> wrote:
>> Again, I don't think that `n == 1` is a frequent case which would make any
>> practical difference.
>>
>> As for the `bitLength` check, the product might overflow.
>> Further, `bitLength` might not be that cheap.
>> Finally, the test would just help to _fail_ faster at the expense of making
>> the successful runs slightly slower.
>
>> As for the `bitLength` check, the product might overflow. Further,
>> `bitLength` might not be that cheap.
>
> The current implementations of `bitLength()` call `numberOfLeadingZeros()`,
> which are:
>
> public static int numberOfLeadingZeros(long i) {
> int x = (int)(i >>> 32);
> return x == 0 ? 32 + Integer.numberOfLeadingZeros((int)i)
> : Integer.numberOfLeadingZeros(x);
> }
>
> public static int numberOfLeadingZeros(int i) {
> // HD, Count leading 0's
> if (i <= 0)
> return i == 0 ? 32 : 0;
> int n = 31;
> if (i >= 1 << 16) { n -= 16; i >>>= 16; }
> if (i >= 1 << 8) { n -= 8; i >>>= 8; }
> if (i >= 1 << 4) { n -= 4; i >>>= 4; }
> if (i >= 1 << 2) { n -= 2; i >>>= 2; }
> return n - (i >>> 1);
> }
Yes, I'm familiar with both this Java code and the intrinsic code.
Compare this with the much simpler proposed code.
The checked multiplication `unsignedMultiplyExact` apparently performs two
64x64->64 multiplications, but on some architectures it might end up in a
single 64x64->128 multiplication and one check.
So the proposed code performs 6 such multiplications if the method returns + 5
ordinary multiplications in the worst case.
As a general rule, the simpler the code, the better the outcome of the
optimizing compiler.
Again, to me there's no point in failing fast at the expense of the successful
case.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25003#discussion_r2071955574
