RFR: 8304450: [vectorapi] Refactor VectorShuffle implementation [v2]

[Paul Sandoz]

On Tue, 21 Mar 2023 16:29:44 GMT, Paul Sandoz <psandoz at openjdk.org> wrote:

>> I have moved most of the methods to `AbstractVector` and `AbstractShuffle`, I have to resort to raw types, though, since there seems to be no way to do the same with wild cards, and the generics mechanism is not powerful enough for things like `Vector<E.integral>`. The remaining failure seems to be related to [JDK-8304676](https://bugs.openjdk.org/projects/JDK/issues/JDK-8304676), so I think this patch is ready for review now.
>> 
>>> The mask implementation is specialized by the species of vectors it operates on, but does it have to be
>> 
>> Apart from the mask implementation, shuffle implementation definitely has to take into consideration the element type. However, this information does not have to be visible to the API, similar to how we currently handle the vector length, we can have `class AbstractMask<E> implements VectorMask`. As a result, the cast method would be useless and can be removed in the API, but our implementation details would still use it, for example
>> 
>>     Vector<E> blend(Vector<E> v, VectorMask w) {
>>         AbstractMask<?> aw = (AbstractMask<?>) w;
>>         AbstractMask<E> tw = aw.cast(vspecies());
>>         return VectorSupport.blend(...);
>>     }
>> 
>>     Vector<E> rearrange(VectorShuffle s) {
>>         AbstractShuffle<?> as = (AbstractShuffle<?>) s;
>>         AbstractShuffle<E> ts = s.cast(vspecies());
>>         return VectorSupport.rearrangeOp(...);
>>     }
>> 
>> What do you think?
>
>> Apart from the mask implementation, shuffle implementation definitely has to take into consideration the element type.
> 
> Yes, the way you have implemented shuffle is tightly connected, that looks ok.
> 
> I am wondering if we can make the mask implementation more loosely coupled and modified such that it does not have to take into consideration the element type (or species) of the vector it operates on, and instead compatibility is based solely on the lane count.
> 
> Ideally it would be good to change the `VectorMask::check` method to just compare the lanes counts and not require a cast in the implementation, which i presume requires some deeper changes in C2?
> 
> What you propose seems a possible a interim step towards a more preferable API, if the performance is good.

> Thanks @PaulSandoz and @XiaohongGong for the reviews and testings.

Running tier2/3 tests.

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

PR Comment: https://git.openjdk.org/jdk/pull/13093#issuecomment-1492274813