RFR: 8342676: Unsigned Vector Min / Max transforms [v4]

[Jatin Bhateja]

On Fri, 28 Mar 2025 11:00:09 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Jatin Bhateja has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains eight additional commits since the last revision:
>> 
>>  - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8342676
>>  - Review suggestions incorporated.
>>  - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8342676
>>  - Updating copyright year of modified files
>>  - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8342676
>>  - Update IR transforms and tests
>>  - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8342676
>>  - 8342676: Unsigned Vector Min / Max transforms
>
> src/hotspot/share/opto/vectornode.cpp line 1045:
> 
>> 1043: }
>> 1044: 
>> 1045: bool VectorNode::is_commutative() {
> 
> Why did you make this change? It seems unrelated, maybe it slipped in from another change set? Plus, it is not an accurate name, if `if (in(1)->_idx > in(2)->_idx) {` would fail, you would say that it is not commutative ... which is wrong ;)

It was modified as per suggestion from Vladimir Ivanov https://github.com/openjdk/jdk/pull/22863#discussion_r1967092266

I am ok with skipping it for now.

> src/hotspot/share/opto/vectornode.cpp line 2213:
> 
>> 2211:     umin = n->in(2);
>> 2212:     umax = n->in(1);
>> 2213:   }
> 
> Suggestion:
> 
>   } else {
>     // Either both Min or Max.
>     return nullptr;
>   }
> 
> That way, you don't need the check below:
> `umin != nullptr && umax != nullptr`.

We still want to execute VectorNode::Ideal if new transforms are not applicable.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21604#discussion_r2042168184
PR Review Comment: https://git.openjdk.org/jdk/pull/21604#discussion_r2042167983