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. @PaulSandoz As some hardware does differentiate masks based on element type, at some point we have to differentiate between them. From a design point of view, they are both implementation details so there might be no consideration regarding the API. On the other hand, having more in the Java side seems to be more desirable, as it does illustrate the operations more intuitively compared to the graph management in C2. Another important point I can think of is that having a constant shape for a Java class would help us in implementing the vector calling convention, as we can rely on the class information instead of some side channels. As a result, I think I do prefer the current class hierarchy. ------------- PR Comment: https://git.openjdk.org/jdk/pull/13093#issuecomment-1478374992
