RFR: 8282365: Optimize divideUnsigned and remainderUnsigned for constants [v16]

[Quan Anh Mai]

On Mon, 3 Jul 2023 10:35:06 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Quan Anh Mai has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 50 commits:
>> 
>>  - missing java_negate
>>  - Merge branch 'master' into unsignedDiv
>>  - whitespace
>>  - move asserts to use sites
>>  - windows complaints
>>  - compiler complaints
>>  - undefined internal linkage
>>  - add tests, special casing large shift
>>  - draft
>>  - Merge branch 'master' into unsignedDiv
>>  - ... and 40 more: https://git.openjdk.org/jdk/compare/5b147eb5...eb1f5dd9
>
> src/hotspot/share/opto/divnode.cpp line 39:
> 
>> 37: #include "utilities/powerOfTwo.hpp"
>> 38: 
>> 39: // Portions of code courtesy of Clifford Click
> 
> Not sure if this line should be removed?

I will revert that line

> src/hotspot/share/opto/divnode.cpp line 188:
> 
>> 186:       max_dividend = max_juint;
>> 187:     }
>> 188:     if (julong(magic_const) <= max_julong / max_dividend) {
> 
> Could `max_dividend` ever be `zero`? I guess only if the dividend was exactly `zero`, in which case we should probably not end up here, or is that somehow possible?

I will change this to `max_dividend == 0 || julong(magic_const) <= max_julong / max_dividend` to be safe.

> src/hotspot/share/opto/divnode.cpp line 191:
> 
>> 189:       // No overflow here, just do the transformation
>> 190:       if (shift_const == 32) {
>> 191:         q = phase->intcon(0);
> 
> Would it not be nicer to handle this special case directly in the `URShiftLNode`? Just replace it during `Value` with zero, if the shift constant is too large.

No, because `x >> 64 == x` in int64 arithmetic, while the semantics we need here is integer arithmetic, so we need special handling for this case.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/9947#discussion_r1261396413
PR Review Comment: https://git.openjdk.org/jdk/pull/9947#discussion_r1261400137
PR Review Comment: https://git.openjdk.org/jdk/pull/9947#discussion_r1261405044