On Wed, 10 Jan 2024 14:48:21 GMT, Roger Riggs <rri...@openjdk.org> wrote:

>> Pavel Rappo has updated the pull request with a new target base due to a 
>> merge or a rebase. The pull request now contains 20 commits:
>> 
>>  - Use Integer.compareUnsigned
>>  - Update copyright years and headers
>>  - Merge branch 'master' into 8310813
>>  - Merge branch 'master' into 8310813
>>  - Merge branch 'master' into 8310813
>>  - Merge branch 'master' into 8310813
>>  - Merge branch 'master' into 8310813
>>  - Fix bugs in Shared.createSingle
>>  - Merge branch 'master' into 8310813
>>  - Group params coarser (suggested by @cl4es)
>>    
>>    - Splits 20 params into 3 groups: (S)mall, (M)edium and (L)arge.
>>      Every testXYZ method invokes M operations, where M is the maximum
>>      number of elements in a group. Shorter groups are cyclically padded.
>>    - Uses the org.openjdk.jmh.infra.Blackhole API and increases
>>      benchmark time.
>>    - Fixes a bug in Shared that precluded 0 from being in a pair.
>>  - ... and 10 more: https://git.openjdk.org/jdk/compare/bc05893f...08e6adca
>
> src/java.base/share/classes/java/math/BigInteger.java line 3998:
> 
>> 3996:         int i = ArraysSupport.mismatch(m1, m2, len1);
>> 3997:         if (i != -1)
>> 3998:             return Integer.compareUnsigned(m1[i], m2[i]) < 0 ? -1 : 1;
> 
> Just an observation.  The (Java and intrinsic) implementation of 
> Integer.compareUnsigned already returns -1, 0, 1.
> Returning `Integer.compareUnsigned(m1[i], m2[i])` would yield the same result 
> without the tertiary expression.

Yes, that's what was proposed 
[here](https://github.com/openjdk/jdk/pull/14630#discussion_r1242245875) some 
time ago.

But the spec of `compareUnsigned()` does _not_ guarantee a -1, 0, 1 result, so 
there's a (minimal) risk when returning its value directly. (For some reason, 
`BigInteger` specifies a -1, 0, 1 outcome.)

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14630#discussion_r1447502797

Reply via email to