On Fri, 19 Dec 2025 09:55:25 GMT, Eric Fang <[email protected]> wrote:
>> The original implementation of UMIN/UMAX reductions in JDK-8346174 used
>> incorrect identity values in the Java implementation and test code.
>>
>> Problem:
>> --------
>> UMIN was using MAX_OR_INF (signed maximum value) as the identity:
>> - Byte.MAX_VALUE (127) instead of max unsigned byte (255)
>> - Short.MAX_VALUE (32767) instead of max unsigned short (65535)
>> - Integer.MAX_VALUE instead of max unsigned int (-1)
>> - Long.MAX_VALUE instead of max unsigned long (-1)
>>
>> UMAX was using MIN_OR_INF (signed minimum value) as the identity:
>> - Byte.MIN_VALUE (-128) instead of 0
>> - Short.MIN_VALUE (-32768) instead of 0
>> - Integer.MIN_VALUE instead of 0
>> - Long.MIN_VALUE instead of 0
>>
>> This caused incorrect result. For example:
>> UMAX([42,42,...,42]) returned 128 instead of 42
>>
>> Solution:
>> ---------
>> Use correct unsigned identity values:
>> - UMIN: ($type$)-1 (maximum unsigned value)
>> - UMAX: ($type$)0 (minimum unsigned value)
>>
>> Changes:
>> --------
>> - X-Vector.java.template: Fixed identity values in reductionOperations
>> - gen-template.sh: Fixed identity values for test code generation
>> - templates/Unit-header.template: Updated copyright year to 2025
>> - Regenerated all Vector classes and test files
>>
>> Testing:
>> --------
>> All types (byte/short/int/long) now return correct results in both
>> interpreter mode (-Xint) and compiled mode.
>
> Eric Fang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Refine the tests for identity values
Very good, a good stepping stone to for further improvements to declare and
test these constants. Just one last comment which i hope will be easy to
address then I think its good for integration.
test/jdk/jdk/incubator/vector/templates/Unit-Reduction-op-func.template line 25:
> 23: "[[TEST]](" + x + ", [[TEST_INIT]]) != "
> + x);
> 24: }
> 25: }
Just one last thing wrap the loop in a `try/catch(AssertionError e)` where the
`assertEquals` in the loop do not pass in a string argument. The production of
such strings within the loop slow the tests down. In the catch duplicate the
`assertEquals with the string argument.
-------------
PR Review: https://git.openjdk.org/jdk/pull/28692#pullrequestreview-3600300548
PR Review Comment: https://git.openjdk.org/jdk/pull/28692#discussion_r2636437477
