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

Hamlin Li mli at openjdk.org
Tue Mar 12 20:19:19 UTC 2024


On Tue, 12 Mar 2024 16:52:40 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> test/hotspot/jtreg/compiler/vectorization/TestRoundVectorDoubleRandom.java line 124:
>> 
>>> 122:         bits = bits | (1 << 63);
>>> 123:         input[ei_idx*2+1] = Double.longBitsToDouble(bits);
>>> 124:       }
>> 
>> Why do all this complicated stuff, and not just pick a random `long`, and convert it to double with `Double.longToDoubleBits`?
>
> Does this ever generate things like `+0, -0, infty, NaN` etc?

It's testing following cases:
1. all the `e` range, e.g. for double it's 11 bits, for float it's 8 bits
2. for `f` I add a special value `0` explicitly `fis[fidx++] = 0;`
3. for sign, both `+` and `-` are tested.

So, yes, it will test cases like `+/- 0, infty, NaN`.

>> test/hotspot/jtreg/compiler/vectorization/TestRoundVectorDoubleRandom.java line 134:
>> 
>>> 132:         for (int sign = 0; sign < 2; sign++) {
>>> 133:           int idx = ei_idx * 2 + sign;
>>> 134:           if (res[idx] != Math.round(input[idx])) {
>> 
>> Is it ok to use `Math.round` here? What if we compile it, and its computation is wrong in the compilation?
>
> This direct comparison tells me that you are not testing `NaN`s...

> Is it ok to use Math.round here? What if we compile it, and its computation is wrong in the compilation?

It's a bug, will fix.

> This direct comparison tells me that you are not testing NaNs...

this comparison is between long value, for NaN Math.round(NaN) == 0. Or maybe I misunderstood your question?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17753#discussion_r1522059691
PR Review Comment: https://git.openjdk.org/jdk/pull/17753#discussion_r1522059293


More information about the hotspot-compiler-dev mailing list