On Mon, 8 Dec 2025 05:43:38 GMT, Quan Anh Mai <[email protected]> wrote:

>> Hi @merykitty thanks for the review, `a[idx]` is already the starting value 
>> for the reduction operation of this function, see line 4193. What do you 
>> mean by `a better manner` ?
>
> No, the starting value is `-1`, this test depends on the fact that we choose 
> the correct identity value for this particular operation as the starting 
> value, which this issue is about. As a result, it would be better to write 
> the test so that we do not depend on the identity value.
> 
>         byte res = a[idx];
>         for (int i = idx + 1; i < (idx + SPECIES.length()); i++) {
>             res = (byte) VectorMath.minUnsigned(res, a[i]);
>         }

Hi @merykitty , I feel it's better to start the reduction operation from `the 
identity value`, because `firstNonZero`, `min/max`, and `umin/umax` all reuse 
the same template function `gen_reduction_op_func`. If we start from `a[idx]` 
for `umin/umax` (`min/max` is also possible), then we have to create a new 
template function. Considering we've already correctly fixed this issue, I 
think the current approach might be more maintainable. What do you think?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28692#discussion_r2601369650

Reply via email to