RFR: 8325438: Add exhaustive tests for Math.round intrinsics [v6]

Emanuel Peter epeter at openjdk.org
Mon Mar 18 13:30:35 UTC 2024


On Mon, 18 Mar 2024 13:07:21 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Hamlin Li has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains ten additional commits since the last revision:
>> 
>>  - Fix test failure in TestRoundVectorDoubleRandom.java
>>  - Merge branch 'master' into round-v-exhaustive-tests
>>  - add comments; refine code
>>  - refine code; fix bug
>>  - Merge branch 'master' into round-v-exhaustive-tests
>>  - fix issue
>>  - mv tests
>>  - use IR framework to construct the random tests
>>  - Initial commit
>
> test/hotspot/jtreg/compiler/vectorization/TestRoundVectorDoubleRandom.java line 100:
> 
>> 98:     // f (significant) part of a float value
>> 99:     final int fWidth = eShift;
>> 100:     final long fBound = 1 << fWidth;
> 
> Suggestion:
> 
>     final long fBound = 1L << fWidth;

There may be others like it!

> test/hotspot/jtreg/compiler/vectorization/TestRoundVectorDoubleRandom.java line 122:
> 
>> 120:       final int eStart = rand.nextInt(9);
>> 121:       final int eStep = (1 << 3) + rand.nextInt(3);
>> 122:       for (int ei = eStart; ei < eBound; ei += eStep) {
> 
> Why modify the `eStart`, but not the `eBound`? Why not have both constant?

Ah, you just want to step through with some random start-offset, and some random stride. I see.
`eStart = 0...8`
`eStep = 8...10`
Fair enough. Maybe add a comment for this.

> test/hotspot/jtreg/compiler/vectorization/TestRoundVectorDoubleRandom.java line 130:
> 
>> 128:         input[eiIdx*2] = Double.longBitsToDouble(bits);
>> 129:         // negative values
>> 130:         bits = bits | (1 << 63);
> 
> Suggestion:
> 
>         bits = bits | (1L << 63);
> 
> And yet another of these

jshell> 1 << 63
$4 ==> -2147483648

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17753#discussion_r1528500677
PR Review Comment: https://git.openjdk.org/jdk/pull/17753#discussion_r1528510828
PR Review Comment: https://git.openjdk.org/jdk/pull/17753#discussion_r1528513243


More information about the hotspot-compiler-dev mailing list