On Fri, 7 Aug 2026 13:03:48 GMT, Emanuel Peter <[email protected]> wrote:

>> Jatin Bhateja has updated the pull request with a new target base due to a 
>> merge or a rebase. The incremental webrev excludes the unrelated changes 
>> brought in by the merge/rebase. The pull request contains nine additional 
>> commits since the last revision:
>> 
>>  - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8382523
>>  - Review comments resolution
>>  - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8382523
>>  - Review comments resolution
>>  - Review comments resolution
>>  - Review comments resolution
>>  - Review comments resolution
>>  - Review comments resolution
>>  - 8382523: Optimize Float16 to integral conversion operations for 
>> AVX512-FP16 targets
>
> test/micro/org/openjdk/bench/jdk/incubator/vector/Float16ToIntegralConvBenchmark.java
>  line 60:
> 
>> 58:             i -> {
>> 59:                 if ((i % 100) == 0) {
>> 60:                     fp16inp[i] = float16ToRawShortBits(specialValues[i % 
>> specialValues.length]);
> 
> This comment comes a bit too late, but you will only ever have `i = k * 100`. 
> And `specialValues.length = 5`. So you are really always picking 
> `specialValues[0] = Float.NaN` here.
> 
> It should be `specialValues[(i / 100) % specialValues.length]` instead.
> 
> @jatin-bhateja @sviswa7 @missa-prime Could this have an impact on the 
> performance measurement? I'm especially thinking about NaN/infinity fixup.

@eme64 Very good catch. We will plan to update the jmh micro in the coming 
weeks along with some additional optimizations.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/30928#discussion_r3753434962

Reply via email to