RFR: 8346954: [JMH] jdk.incubator.vector.MaskedLogicOpts fails due to IndexOutOfBoundsException

[Nicole Xu]

On Thu, 13 Feb 2025 11:51:53 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

>> Suite MaskedLogicOpts.maskedLogicOperationsLong512() failed on both x86 and AArch64 with the following error:
>> 
>> 
>> java.lang.IndexOutOfBoundsException: Index 252 out of bounds for length 249
>> 
>> 
>> The variable `long256_arr_idx` is misused when indexing  'LongVector l2, l3, l4, l5' in function `maskedLogicOperationsLongKernel()`. 'long256_arr_idx' increases by 4 every time the benchmark runs and ensures the incremented value remains within the bounds of the array. However, for `LongVector.SPECIES_512`, it loads 8 numbers from the array each time the benchmark runs, resulting in an out-of-range indexing issue.
>> 
>> Hence, we revised the index variables from `long256_arr_idx` to `long512_arr_idx`, which has a stride of 8, to ensure that the loaded vector is inside of the array boundary for all vector species. This is also consistent with other kernel functions.
>> 
>> Additionally, some defined but unused variables have been removed.
>
> test/micro/org/openjdk/bench/jdk/incubator/vector/MaskedLogicOpts.java line 126:
> 
>> 124:     }
>> 125: 
>> 126:     @CompilerControl(CompilerControl.Mode.INLINE)
> 
> By making the index hop over 16 ints or 8 longs we may leave gaps in between for 128-bit and 256-bit species, this will unnecessarily include the noise due to cache misses or (on some targets) prefetching additional cache lines which are not usable, thereby impacting the crispness of microbenchmark.

Thanks, @jatin-bhateja, for your thorough review.

Yes, you're right, the current design does introduce gaps when accessing the data. And other cases in this suite are also designed in a similar way. I am wondering if you're interested in refactoring the code to address this more comprehensively.

The primary goal of this pull request is to address an out-of-bounds issue that was blocking our tests. We aimed to ensure all test cases pass and become unblocked. Additionally, our available hardware resources limit our ability to rigorously test a wide range of scenarios at this time.

Therefore, we opted to maintain consistency with the existing logic in other cases within the suite and simply fix the crash issue. This approach allows us to unblock the current tests and keep things moving for now.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22963#discussion_r1957668625