RFR: 8352869: Verify.checkEQ: extension for NaN, VectorAPI and arbitrary Objects [v7]

[Emanuel Peter]

On Wed, 2 Apr 2025 14:16:58 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> Emanuel Peter has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   refactor with checkEQWithRawBits
>
> test/hotspot/jtreg/compiler/lib/verify/Verify.java line 187:
> 
>> 185:     private void checkEQimpl(char a, char b, String field, Object aParent, Object bParent) {
>> 186:         if (a != b) {
>> 187:             System.err.println("ERROR: Verify.checkEQ failed: value mismatch: " + (int)a + " vs " + (int)b);
> 
> Why do you need an upcast here? Same for `short`.

Look at this ;)

jshell> char a = 66;
a ==> 'B'

jshell> System.out.println("a: " + a);
a: B

jshell> System.out.println("a: " + (int)a);
a: 66


But I can remove the casts for `short`.

> test/hotspot/jtreg/compiler/lib/verify/Verify.java line 254:
> 
>> 252:     private void checkEQimpl(float a, float b, String field, Object aParent, Object bParent) {
>> 253:         if (isFloatEQ(a, b)) {
>> 254:             System.err.println("ERROR: Verify.checkEQ failed: value mismatch. check raw: " + isFloatCheckWithRawBits);
> 
> Just noticed this now (there are other places as well): Since we now have `Verify.checkEQ()` and `Verify.checkEQWithRawBits()`, it would improve the readability if we reported which method was used. It could be done with something like that (pseudo code):
> 
> System.err.println("ERROR: Verify.checkEQ" + withRawBitsString() + " failed: value mismatch.
> 
> String withRawBitsString() {
>     return isFloatCheckWithRawBits ? "WithRawBits" : "";
> }

Boah. That is really going to bloat the code, don't you think?
The exception that is thrown will already give you the complete stack trace, including which methods were called. Is that not good enough?

> test/hotspot/jtreg/compiler/lib/verify/Verify.java line 256:
> 
>> 254:             System.err.println("ERROR: Verify.checkEQ failed: value mismatch. check raw: " + isFloatCheckWithRawBits);
>> 255:             System.err.println("  Values: " + a + " vs " + b);
>> 256:             System.err.println("  Raw:    " + Float.floatToRawIntBits(a) + " vs " + Float.floatToRawIntBits(b));
> 
> Do we always want to dump the raw bits even when `isFloatCheckWithRawBits` is false? I guess it does not hurt.

Yes, I want that. It can help if there are different `NaN` encodings. Or if we somehow reinterpreted integer values as floats. It's been useful for me in the past :)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/24224#discussion_r2026269456
PR Review Comment: https://git.openjdk.org/jdk/pull/24224#discussion_r2026272974
PR Review Comment: https://git.openjdk.org/jdk/pull/24224#discussion_r2026274418