RFR: 8307513: C2: intrinsify Math.max(long,long) and Math.min(long,long)

[Galder Zamarreño]

On Thu, 12 Dec 2024 12:03:11 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>>> @galderz Thanks for taking this task on! Had a quick look at it. So auto-vectorization in SuperWord should now be working, right? If yes:
>>> 
>>> It would be nice if you tested both for `IRNode.MIN_VL` and `IRNode.MIN_REDUCTION_V`, the same for max.
>>> 
>>> You may want to look at these existing tests, to see what other tests there are for the `int` version: `test/hotspot/jtreg/compiler/loopopts/superword/MinMaxRed_Int.java` `test/hotspot/jtreg/compiler/c2/irTests/TestIfMinMax.java` `test/hotspot/jtreg/compiler/vectorization/TestAutoVecIntMinMax.java` `test/hotspot/jtreg/compiler/c2/TestMinMaxSubword.java` There may be some duplicates already here... not sure. 
>> 
>> +1 to adding such tests. I'm looking into it right now. It's a bit confusing how the tests are spread around (and duplication?) but I'm currently leaning towards adding a `compiler/loopopts/superword/MinMaxRed_Long.java`.
>> 
>>> And maybe you need to check what to do about probabilities as well.
>> 
>> I will add probabilities logic (50, 80, 100) to control data, but you can already see that from https://github.com/openjdk/jdk/pull/20098#issuecomment-2379386872 that with the patch in an AVX512 system min/max reduction nodes will appear in all probabilities.
>
> @galderz Yes, there is significant duplication, sadly. Often there were old tests there, but then one comes along and sees that one wants to have more comprehensive tests. So one adds it, but does not feel 100% comfortable removing old tests. A little bit of duplication is probably ok. Often, there are still subtle differences, and sometimes those end up mattering.
> 
> `compiler/loopopts/superword/MinMaxRed_Long.java` sounds like a good idea.

@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.

@jaskarth FYI I've adjusted the expectations in `TestMinMaxIdentities` after this change (thx for adding the test!). Check if there's any comments/changes you'd like.

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

PR Comment: https://git.openjdk.org/jdk/pull/20098#issuecomment-2549259008