RFR: 8372978: [VectorAPI] Fix incorrect identity values in UMIN/UMAX reductions [v2]
Eric Fang
erfang at openjdk.org
Fri Dec 12 08:52:00 UTC 2025
On Tue, 9 Dec 2025 17:34:19 GMT, Paul Sandoz <psandoz at openjdk.org> wrote:
>> Okay, I understand, thank you for your insight! I'll wait for @PaulSandoz 's comment and see if we should add two **public** constants to this PR. Then I'll modify it accordingly.
>
> I would prefer to declare identity constants, but **only** in tests [1] , such as `MUL_IDENTITY`, `UMAX_IDENTITY` etc, that we consistently refer to and then add basic tests to verify that identity with respect to the scalar operation. The identity values are also embedded in the JDK and HotSpot compiler and i want to ensure they are clearly compared against the expected identity when an all-false mask is used.
>
> [1] later we could place these in some internal JDK class so we can use the same values in the JDK code, then adjust the tests to grant access to the internal JDK class to use those values. A more general place to surface up scalar identity values is in `VectorOperators` for the associative operators, something to consider later on perhaps, and that would require a CSR.
@PaulSandoz Thanks for your suggestion! I declared some identity constants in both the tests and the implementations. And added some tests to verify the correctness of these constants.
@merykitty Now we're using a correct constant to represent the identity value, eliminating the dependency on incorrect literals. So I've chosen to keep the current coding style. I tried the style you suggested, but I feel the original style is more readable and maintainable. Do you think this is okay?
Please help take another look, thank you!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28692#discussion_r2613421273
More information about the core-libs-dev
mailing list