On Fri, 12 Dec 2025 08:49:04 GMT, Eric Fang <[email protected]> wrote:
>> 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! Then, please add tests to verify the correctness of these identity value (i.e. `x + i == x`). ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/28692#discussion_r2624199711
