On Tue, 21 Mar 2023 16:29:44 GMT, Paul Sandoz <psan...@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
