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

[Emanuel Peter]

On Wed, 8 Jan 2025 09:03:36 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 five additional commits since the last revision:
>> 
>>  - 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 2162:
> 
>> 2160:   // UMax (UMin(a, b), UMax(a, b))  => UMax(a, b)
>> 2161:   // UMax (UMax(a, b), UMin(b, a))  => UMax(a, b)
>> 2162:   if (umin && umax) {
> 
> That looks like an implicit null check. Not allowed according to style guide:
> `Do not use ints or pointers as (implicit) booleans with &&, ||, if, while. Instead, compare explicitly, i.e. if (x != 0) or if (ptr != nullptr), etc.`
> https://github.com/openjdk/jdk/blob/master/doc/hotspot-style.md

Suggestion:

  if (umin != null && umax) != null {

> test/hotspot/jtreg/compiler/vectorapi/VectorUnsignedMinMaxOperationsTest.java line 29:
> 
>> 27: * @summary Support new unsigned and saturating vector operators in VectorAPI
>> 28: * @modules jdk.incubator.vector
>> 29: * @requires vm.compiler2.enabled
> 
> I think you can drop that requirement.

IR testing is only enabled with C2, and so we can verify that the tests at least execute / have correct results on other platforms / compilers.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21604#discussion_r1906841553
PR Review Comment: https://git.openjdk.org/jdk/pull/21604#discussion_r1906813301