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

[Emanuel Peter]

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

>> 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!

Suggestion:

  return nullptr;

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

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