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

Emanuel Peter epeter at openjdk.org
Wed Mar 13 06:59:21 UTC 2024


On Tue, 12 Mar 2024 20:26:25 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.
>
> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
> 
>   refine code; fix bug

Thanks for adjusting the IR rules!

I still have trouble reviewing your input value generation, and a few other comments.

Thanks for the work you are putting in, I really appreciate it 😊

test/hotspot/jtreg/compiler/vectorization/TestRoundVectorDoubleRandom.java line 30:

> 28:  * @summary Test vector intrinsic for Math.round(double) in full 64 bits range.
> 29:  *
> 30:  * @requires vm.compiler2.enabled

Do we really require C2? We should also run this for C1, and any other potential compiler.

test/hotspot/jtreg/compiler/vectorization/TestRoundVectorDoubleRandom.java line 110:

> 108:       fis[fidx] = 1 << fidx;
> 109:     }
> 110:     fis[fidx++] = 0;

The zero is now always in the same spot. What if vectorization messes up only in a specific slot, and then never encounters that zero? We would maybe never see a zero in that bad spot.

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

Changes requested by epeter (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/17753#pullrequestreview-1933271647
PR Review Comment: https://git.openjdk.org/jdk/pull/17753#discussion_r1522615886
PR Review Comment: https://git.openjdk.org/jdk/pull/17753#discussion_r1522626600


More information about the hotspot-compiler-dev mailing list