RFR: 8338023: Support two vector selectFrom API [v7]

[Emanuel Peter]

On Fri, 30 Aug 2024 13:17:26 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

>> Jatin Bhateja has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Adding descriptive comments
>
> src/hotspot/cpu/x86/matcher_x86.hpp line 215:
> 
>> 213:   }
>> 214: 
>> 215:   static bool vector_indexes_needs_massaging(BasicType ety, int vlen) {
> 
> The name "massaging" sounds quite vague. Can we have something more expressive / descriptive? Is it the vector that "needs" massaging or the indices that "need" massaging?
> 
> Why `ety` and not `bt`? Is that not the name we use most often?

Hmm, I see that `ety` is used in other places here. What does it stand for?

> src/hotspot/share/opto/vectornode.cpp line 2183:
> 
>> 2181:     };
>> 2182:     // Targets emulating unsupported permutation for certain vector types
>> 2183:     // may need to message the indexes to match the users intent.
> 
> Suggestion:
> 
>     // may need to massage the indexes to match the users intent.

This optimization for now seems quite specific to your `SelectFromTwoVectorNode::Ideal` lowering code. Can this conversion not be done there already?

What is the semantics of `VectorRearrangeNode`? Should its shuffle vector always be bytes, and we now violated that "for a quick second"? Or is it going to be generally the idea to create all sorts of shuffle types and then fix that up? But then why do we need the `vector_indexes_needs_massaging`?

Can you help me understand the concept/strategy behind this?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/20508#discussion_r1738714401
PR Review Comment: https://git.openjdk.org/jdk/pull/20508#discussion_r1738862138