RFR: 8282221: x86 intrinsics for divideUnsigned and remainderUnsigned methods in java.lang.Integer and java.lang.Long [v8]

[Srinivas Vamsi Parasa]

On Wed, 6 Apr 2022 00:46:01 GMT, Vladimir Kozlov <kvn at openjdk.org> wrote:

>> Srinivas Vamsi Parasa has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   add error msg for jtreg test
>
> I have few comments.

Hi Vladimir (@vnkozlov),

Incorporated all the suggestions you made in the previous review and pushed a new commit.
Please let me know if anything else is needed.

Thanks,
Vamsi

> src/hotspot/cpu/x86/assembler_x86.cpp line 12375:
> 
>> 12373: }
>> 12374: #endif
>> 12375: 
> 
> Please, place it near `idivq()` so you would not need `#ifdef`.

Made the change as per your suggestion.

> src/hotspot/cpu/x86/c2_MacroAssembler_x86.cpp line 4568:
> 
>> 4566:   subl(rdx, divisor);
>> 4567:   if (VM_Version::supports_bmi1()) andnl(rax, rdx, rax);
>> 4568:   else {
> 
> Please, follow our coding stile here and in following methods:
> 
> if (VM_Version::supports_bmi1()) {
>   andnl(rax, rdx, rax);
> } else {

Pls see the new commit which fixed the coding style.

> src/hotspot/cpu/x86/x86_64.ad line 8701:
> 
>> 8699: %}
>> 8700: 
>> 8701: instruct udivI_rReg(rax_RegI rax, no_rax_rdx_RegI div, rFlagsReg cr, rdx_RegI rdx)
> 
> I suggest to follow the pattern in other `div/mod` instructions: `(rax_RegI rax, rdx_RegI rdx, no_rax_rdx_RegI div, rFlagsReg cr)`
> 
> Similar in following new instructions.

Pls see the new commit which fixed the pattern.

> test/hotspot/jtreg/compiler/intrinsics/TestIntegerDivMod.java line 55:
> 
>> 53:             dividends[i] = rng.nextInt();
>> 54:             divisors[i] = rng.nextInt();
>> 55:         }
> 
> I don't trust RND to generate corner cases.
> Please, add cases here and in TestLongDivMod.java for MAX, MIN, 0.

You are right. Using an updated corner cases test revealed divide by zero crash which was fixed. Please see the updated jtreg tests inspired by unsigned divide/remainder tests in test/jdk/java/lang/Long/Unsigned.java and test/jdk/java/lang/Integer/Unsigned.java.

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

PR: https://git.openjdk.java.net/jdk/pull/7572