RFR: 8325438: Add exhaustive tests for Math.round intrinsics [v6]
Hamlin Li
mli at openjdk.org
Tue Mar 19 14:20:36 UTC 2024
On Mon, 18 Mar 2024 13:27:35 GMT, Emanuel Peter <epeter at openjdk.org> wrote:
> Thanks for the changes, the comments help!
>
> You could still use full names, rather than `e` and `f`.
>
> Left a few comments. There are a few int/long issues like this:
>
> ```
> jshell> int x = 52;
> x ==> 52
>
> jshell> long y = 1 << x;
> y ==> 1048576
>
> jshell> long y = 1L << x
> y ==> 4503599627370496
> ```
>
> I think I understand how you want to do the generation now. But as you see, there was a bug in it. And it was hard to find. That is why I was asking for just pure random value generation. Constructing values ourselves often goes wrong, and then the test-coverage drops quickly.
>
Thanks a lot for catching this!
> Also: what is the probability that you will ever generate a `infty` or a `Nan` for doubles? Can you give me an estimate?
For inf, the probability is 2 in all runs (+inf, -inf); for NaN, the probability is (eStep/eBound) (but depends on rand value), iin new version I modify it to make sure it tests NaN, so it's (eStep/eBound) now.
> test/hotspot/jtreg/compiler/vectorization/TestRoundVectorDoubleRandom.java line 57:
>
>> 55: public static void main(String args[]) {
>> 56: TestFramework.runWithFlags("-XX:-TieredCompilation", "-XX:CompileThresholdScaling=0.3", "-XX:MaxVectorSize=16");
>> 57: TestFramework.runWithFlags("-XX:-TieredCompilation", "-XX:CompileThresholdScaling=0.3", "-XX:MaxVectorSize=32");
>
> You should either drop the `"-XX:MaxVectorSize=16"`, or at least have a run without this flag.
> There are machines with higher max vector length, i.e. AVX512 have `64`. Would be nice to test those too ;)
Thanks for the suggestion.
Unfortunately, I don't have access to a machine with AVX512, but I do run with a aarch64 via qemu where max vector size > 16, and it works with "-XX:MaxVectorSize=16".
The reason why the previous test failed (which I fixed in previous commit) with "-XX:MaxVectorSize=8", is because in test framework, it checks the length of vector and make sure it > length of double(8 bytes), i.e. at least 2*(length of Double).
> test/hotspot/jtreg/compiler/vectorization/TestRoundVectorDoubleRandom.java line 131:
>
>> 129: // negative values
>> 130: bits = bits | (1 << 63);
>> 131: input[eiIdx*2+1] = Double.longBitsToDouble(bits);
>
> Is this some sort of luck, or why is this always in bounds?
This is always in bounds of array, because it's calculated in such a way, eBound = 1 << 11, max(eiIdx) == eBound/8 == 256, max(eiIdx*2+1) == (256*2+1) == 513;
-------------
PR Comment: https://git.openjdk.org/jdk/pull/17753#issuecomment-2007304368
PR Review Comment: https://git.openjdk.org/jdk/pull/17753#discussion_r1530464928
PR Review Comment: https://git.openjdk.org/jdk/pull/17753#discussion_r1530465106
More information about the hotspot-compiler-dev
mailing list