RFR: 8341781: Improve Min/Max node identities [v2]

[Jasmine Karthikeyan]

On Wed, 30 Oct 2024 08:50:33 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Jasmine Karthikeyan has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Suggestions from review
>
> src/hotspot/share/opto/addnode.cpp line 1268:
> 
>> 1266: Node* MaxINode::Identity(PhaseGVN* phase) {
>> 1267:   const TypeInt* t1 = phase->type(in(1))->is_int();
>> 1268:   const TypeInt* t2 = phase->type(in(2))->is_int();
> 
> Could any input be `TOP`?

I think we can't encounter `TOP` here because we run `Value()` before `Identity()`, so if TOP is returned by Value() the idealization process exits to return the top node before running Identity().

> test/hotspot/jtreg/compiler/c2/irTests/TestMinMaxIdentities.java line 35:
> 
>> 33:  * @summary Test identities of MinNodes and MaxNodes.
>> 34:  * @key randomness
>> 35:  * @requires (os.simpleArch == "x64" & vm.cpu.features ~= ".*avx.*") | os.arch == "aarch64" | os.arch == "riscv64"
> 
> Is there a chance we can add these `requires` to the `@IR` rules instead? That way we can still do the result verification on all other platforms, which could be valuable on its own.

>From my understanding this isn't possible as-is since CPU features seem to be checked regardless of whether the architecture supports it or not, so we can't simply check for AVX because that would fail on aarch64 and riscv64. I think we could work around this with `applyIfCPUFeatureOr = {"avx", "true", "asimd", "true", "rvv", "true"}` to force a check for all 3 platforms but it'd be filtering more platforms than strictly necessary.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21439#discussion_r1823736920
PR Review Comment: https://git.openjdk.org/jdk/pull/21439#discussion_r1823736946