On Fri, 20 Dec 2024 15:42:14 GMT, Fei Gao <f...@openjdk.org> wrote:
>> Galder Zamarreño has updated the pull request incrementally with five
>> additional commits since the last revision:
>>
>> - Added comment around the assertions
>> - Adjust min/max identity IR test expectations after changes
>> - Fix style
>> - Add max reduction test
>> - Add empty line
>
> test/hotspot/jtreg/compiler/loopopts/superword/MinMaxRed_Long.java line 135:
>
>> 133: @IR(applyIf = {"SuperWordReductions", "true"},
>> 134: applyIfCPUFeatureOr = { "avx512", "true" },
>> 135: counts = {IRNode.MIN_REDUCTION_V, " > 0"})
>
>> @eme64 I've addressed all your comments except aarch64 testing. `asimd` is
>> not enough, you need `sve` for this, but I'm yet to make it work even with
>> `sve`, something's up and need to debug it further.
>
> Hi @galderz , may I ask if these long-reduction cases can't work even with
> `sve`? It might be related with the limitation
> [here](https://github.com/openjdk/jdk/blob/75420e9314c54adc5b45f9b274a87af54dd6b5a8/src/hotspot/share/opto/superword.cpp#L1564-L1566).
> Some `sve` machines have only 128 bits.
That's right. Neoverse V2 is 4 pipes of 128 bits, V1 is 2 pipes of 256 bits.
That comment is "interesting". Maybe it should be tunable by the back end.
Given that Neoverse V2 can issue 4 SVE operations per clock cycle, it might
still be a win.
Galder, how about you disable that line and give it another try?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20098#discussion_r1894118531
