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

[Andrew Haley]

On Wed, 10 Apr 2024 17:13:22 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> Thanks for the sample code.
>> 
>> I modify the current test a bit by using your code, i.e. change from 2 level nested loop to single while loop as below, let's call it `new test`
>> 
>>   @Test
>>   static boolean test(int testInt) {
>>         float testFloat = Float.intBitsToFloat(testInt);
>>         return Math.round(testFloat) != golden_round(testFloat);
>>   }
>> 
>>   @Run(test = "test")
>>   static void test_rounds(RunInfo runInfo) {
>>     for (int i = 0; i < 1000; i++) {        
>>         test(i);
>>     }
>>     if (runInfo.isWarmUp()) {
>>         return;
>>     }
>>     boolean runTest = true; // modify here to have try.
>>     if (!runTest) return;
>>     int testInt = 0;
>>     boolean fail = false;
>>     do {
>>         fail |= test(testInt);
>>     } while (++testInt != 0);
>>     if (fail) {
>>         throw new RuntimeException();
>>     }
>>   }
>> 
>> 
>> It still took more than 5 minutes to finish the test; if I assign `runTest = false`, it will take seconds. So most of time is spent on the while loop in `test_rounds` with `@Run` annotation in new test, I'm not sure how the annotation @Run works, but seems that's the reason why it's slower than a pure while loop (in your sample code). But we need the annotations in the test (check below).
>> There are still some gaps between this new test and current test:
>> * we still not yet verify IR Node (`IRNode.ROUND_VF`); to verify it, we need to put the (part of) test into a nested loop, and put this loop in a function (`test_round` in current test), and annotate this function with `@IR` to verify the IR node.
>> 
>> Or maybe there are other ways to implement this test and qualify below requirements? Currently I'm not sure.
>> 1. run in a minute, as we want it to be an automatic test,
>> 2. verify Math.round (intrinsic) result,
>> 3. verify IR node (`IRNode.ROUND_VF) generation,
>> 4. make sure all the verification is done after the warmup.
>
> Yes, I see. The `@Run` annotation runs it all with the IR framework, which you don't really need for an exhaustive test over the 32-bit range.
> 
> I think I may have been rather misled by the "Add exhaustive tests for Math.round intrinsics" title, which this PR doesn't do. I strongly suggest that you do add an exhaustive test for the 32-bit range, without using the `@Run` annotation, just the bare code.

And my test only works on the scalar version, of course.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17753#discussion_r1559823545