RFR: 8291809: Convert compiler/c2/cr7200264/TestSSE2IntVect.java to IR verification test [v2]

[Christian Hagedorn]

On Wed, 17 Jan 2024 15:06:55 GMT, Daniel Lundén <dlunden at openjdk.org> wrote:

>> test/hotspot/jtreg/compiler/c2/cr7200264/TestIntVect.java line 155:
>> 
>>> 153:             test_addc(a0, a1);
>>> 154:             for (int i=0; i<ARRLEN; i++) {
>>> 155:                 errn += verify("test_addc: ", i, a0[i], (int)((int)(ADD_INIT+i)+VALUE));
>> 
>> I suggest to either directly use:
>> 
>> Asserts.assertEQ(a0[i],  (int)((int)(ADD_INIT+i)+VALUE), "test_addc failed at a0[" + i + "]");
>> 
>> Or change `verify()` such that it uses `Asserts.assertEQ()` (just an example and could also be adjusted):
>> 
>>     static int verify(String text, int i, int elem, int val) {
>>         Asserts.assertEQ(elem, val, text + " failed at a0[" + i + "]").
>>     }
>
> When translating the test, I focused only on translating the old ad-hoc IR tests to the IR verification framework. Do we want to extend the scope of this issue to also update the testing code? Mainly, the reason I'm asking is that copies of `TestIntVect.java` appears in many places (e.g., `test/compiler/6340864/TestIntVect.java`), and the testing pattern in particular appears in many places (grep for, e.g., `errn += verify(`). If we update the pattern here, we should probably also update it everywhere else?

Good point, I guess it's okay then to leave this code as as it is. It just looked odd to do correctness testing the way it does. But as you say, if you change that you probably also need to revisit other tests. That's probably not worth it. The main goal should be to introduce IR matching with the IR framework. So, I'm fine with not touching that code or the print statements.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17428#discussion_r1457015405