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

[Emanuel Peter]

On Fri, 28 Mar 2025 11:10:30 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 2230:
> 
>> 2228:   }
>> 2229: 
>> 2230:   return static_cast<VectorNode*>(n)->VectorNode::Ideal(phase, can_reshape);
> 
> Hmm. You now have a function `UMinMaxV_Ideal` that promises to do something specific (i.e. work with a MinMax case). But now you actually call into a more general method `VectorNode::Ideal`. I don't think that is a good approach ;)
> 
> Our usual approach is something like this:
> 
>   3871 Node *StoreBNode::Ideal(PhaseGVN *phase, bool can_reshape){
>   3872   Node *progress = StoreNode::Ideal_masked_input(phase, 0xFF);                                                                                                                                                                                                                                                                                                                                                                       
>   3873   if( progress != nullptr ) return progress;
>   3874 
>   3875   progress = StoreNode::Ideal_sign_extended_input(phase, 24);
>   3876   if( progress != nullptr ) return progress;
>   3877                                                                 
>   3878   // Finally check the default case
>   3879   return StoreNode::Ideal(phase, can_reshape);
>   3880 }
> 
> 
> That way, you don't have to do the whole trick with `static_cast<VectorNode*>(n)->` either.

Ah, and then `UMinMaxV_Ideal` also does not need the argument `can_reshape` any more!

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

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