On Tue, 9 Dec 2025 17:34:19 GMT, Paul Sandoz <[email protected]> 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
