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