RFR: 8315024: Vector API FP reduction tests should not test for exact equality
Emanuel Peter
epeter at openjdk.org
Wed Oct 4 09:50:40 UTC 2023
On Tue, 3 Oct 2023 07:53:05 GMT, Gergö Barany <gbarany at openjdk.org> wrote:
> Certain floating point reduction operations in the Vector API are allowed to introduce rounding errors. Adjust the corresponding tests to allow a small relative error when comparing the operation's result to the expected value. Also, add a new generator `double[i / 10.0 + 0.1]` to test floating point operations with somewhat more interesting input data.
Generally looks good, thanks for looking into this.
I left a few comments below.
Another concern I have, which I ran into by writing tests for the auto-vectorizer:
Are we making sure the float/double reductions do not degenerate to either zero or infinity? Because if they do degenerate, then we have only a very weak test.
I'm especially worried about all the values that depend on `i`, and then get multiplied. Don't the multiplications hit the maximal float value very quickly?
test/jdk/jdk/incubator/vector/Double128VectorTests.java line 64:
> 62:
> 63: // for floating point reduction ops that may introduce rounding errors
> 64: private static final double RELATIVE_ROUNDING_ERROR = (double)0.01;
It seems a bit large. Have you tried to make it smaller? Or what is your justification for this value?
test/jdk/jdk/incubator/vector/Double128VectorTests.java line 150:
> 148: }
> 149: }
> 150:
Optional: reduce code duplication. Call `assertReductionArraysEquals` with `relativeError = 0` in pre-existing method.
test/jdk/jdk/incubator/vector/Double128VectorTests.java line 1117:
> 1115: return fill(s * BUFFER_REPS,
> 1116: i -> (double)(i / (double) 10.0 + 0.1));
> 1117: }),
Did this generate rounding issues for addition?
Another option would be random values that make sure to fill the whole mantissa with random information.
Of course the tricky part is to keep them within reasonable bounds so that on multiplication they do not degenerate to zero or infinity.
Also: it would be nice to have some cases with extreme values (infinity, NaN, etc).
-------------
Changes requested by epeter (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/16024#pullrequestreview-1657079233
PR Review Comment: https://git.openjdk.org/jdk/pull/16024#discussion_r1345490344
PR Review Comment: https://git.openjdk.org/jdk/pull/16024#discussion_r1345499960
PR Review Comment: https://git.openjdk.org/jdk/pull/16024#discussion_r1345503637
More information about the hotspot-compiler-dev
mailing list