RFR: 8325438: Add exhaustive tests for Math.round intrinsics

[Emanuel Peter]

On Wed, 7 Feb 2024 16:07:02 GMT, Hamlin Li <mli at openjdk.org> wrote:

> HI,
> Can you have a look at this patch adding some tests for Math.round instrinsics?
> Thanks!
> 
> ### FYI:
> During the development of RoundVF/RoundF, we faced the issues which were only spotted by running test exhaustively against 32/64 bits range of int/long.
> It's helpful to add these exhaustive tests in jdk for future possible usage, rather than build it everytime when needed.
> Of course, we need to put it in `manual` mode, so it's not run when `-automatic` jtreg option is specified which I guess is the mode CI used, please correct me if I'm assume incorrectly.

Ah, I see now. This is a `manual` test.

A few comments:
- We don't have any other manual tests. These tests are only run manually, and not automatically. Probably nobody will ever run this test again, hardly anybody knows that this option even exists.
- You iterate over a range of `2^64`. How long does this take on your machine? Did this even ever complete? This will run in the order of days, if not weeks or months on a single machine.
- You construct the float/double values manually, by adding mantissa and exponent. But what about all the values that are outside this range? Or are you sure you covered all `2^64` bit values?
- You still compare both in potentially compiled code versions. That will not do, we may mis-compile both in the same way.
- You don't have any IR verification about what was done by the compiler.

Suggestions:
- Your test must be executable in automatic mode, and not just in manual. This ensures we run the test in CI, and here on GitHub actions.
- Instead of iterating over **all possible values**, you should do it **randomly**. This way the test can run for a limited number or inputs, and terminate in reasonable time.
- You should do it using the IR framework, so you can verify the IR nodes. The IR framework will soon support automatic random input generation and result verification. But for now you will have to do the input generation and verification against "golden" value from the interpreter yourself.
- The random values should mix in `nan, infty, +-0.0, ...` with a higher frequency than just random values taken via `random.nextLong()` converted to double via `Double.longBitsToDouble`.

Other thoughts:
- If your really really want the manual mode with exhaustive iteration over int and long range, then you can put that in, but only in addition to a IR framework test with random values.
- Rather than focusing on doing just `Math.round`, we should probably do this with other (and maybe all) operations. I quickly checked for things like `Math.sqrt` and a few others. For many there is no good random input test with result verification.

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

PR Comment: https://git.openjdk.org/jdk/pull/17753#issuecomment-1959492086