On Thu, 5 Mar 2026 10:35:45 GMT, Galder Zamarreño <[email protected]> wrote:
>> Jatin Bhateja has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review comments resolutions
>
> test/hotspot/jtreg/compiler/vectorapi/TestVectorBroadcastReassociations.java
> line 1087:
>
>> 1085:
>> 1086: /* =======================
>> 1087: * Reassociation
>
> The IR tests for reassociation seem to focus on ints. Should other types be
> covered?
Done, added missing tests and move reassociation to a separate file
> test/hotspot/jtreg/compiler/vectorapi/TestVectorBroadcastReassociations.java
> line 1104:
>
>> 1102: IRNode.REPLICATE_I, IRNode.VECTOR_SIZE_ANY, ">= 1"
>> })
>> 1103: @Warmup(value = 10000)
>> 1104: static void test_reassociation1() {
>
> Also, more descriptive names than `test_reassociation` + number would be
> appreciated.
Added descriptive test pattern over each test for clarity.
> test/micro/org/openjdk/bench/jdk/incubator/vector/ReassociateVectorBenchmark.java
> line 89:
>
>> 87:
>> 88: @Benchmark
>> 89: public void reassociateVectorsKernel1() {
>
> I think the benchmark names should be a bit more descriptive, because given
> `reassociateVectorsKernel1` and `reassociateVectorsKernel2` it's not clear
> what the difference both of those is. At a glance it looks like
> `reassociateVectorsKernel1` is for ints and `reassociateVectorsKernel2` for
> longs?
>
> Should other types beyond int and long be covered here too?
Addressed, added descriptive patten over micros
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25617#discussion_r2909630847
PR Review Comment: https://git.openjdk.org/jdk/pull/25617#discussion_r2909630510
PR Review Comment: https://git.openjdk.org/jdk/pull/25617#discussion_r2909631196
