On Fri, 12 Jul 2024 13:07:02 GMT, Raffaello Giulietti <rgiulie...@openjdk.org>
wrote:
>> fabioromano1 has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Added comment on normalization in MutableBigInteger.sqrtRemZimmermann()
>
> src/java.base/share/classes/java/math/MutableBigInteger.java line 1997:
>
>> 1995: if (len == 2) { // Base case
>> 1996: long x = ((value[offset] & LONG_MASK) << 32) |
>> (value[offset + 1] & LONG_MASK);
>> 1997: long s = (long) Math.sqrt(x > 0 ? x : x + 0x1p64);
>
> Again, this needs a proof that doing so is always correct.
>
> Suggestion:
>
> long s = (long) Math.sqrt(x >= 0 ? x : x + 0x1p64);
Here there's no need to modify `x > 0` with `x >= 0`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19710#discussion_r1675972896
