[vector api] Vector elements shift confusion

[Tomasz Kowalczewski]

Thanks Vladimir for taking time to look into it. If it makes sense I
can try to fix these (should be simple - on Java level) and post a
webrev (I have signed the OCA).

--
Tomek

On Wed, Jan 23, 2019 at 4:12 AM Vladimir Ivanov
<vladimir.x.ivanov at oracle.com> wrote:
>
> Good catch, Tomasz! I confirm that rotateER is completely broken :-)
>
> There's definitely disagreement between javadoc & implementation on
> shiftEL/ER variants, but I'm in favor of the javadoc here and would like
> to see the implementation aligned with it.
>
> While fromArray() and [shift|rotate]EL/ER seem to disagree
> (little-endian vs big-endian style), it aligns with the behavior of
> shifts on primitives where:
>    "left" == "towards higher bits" == "towards higher indices"
>    "right" == "towards lower bits" == "towards higher indices"
>
> And as you noted, implementation behaves in the opposite direction while
> rotateEL agrees with the spec [1].
>
> Best regards,
> Vladimir Ivanov
>
> [1] $ jshell --add-modules=jdk.incubator.vector
>
>  > import jdk.incubator.vector.*
>
>  > var I256 =
> IntVector.species(jdk.incubator.vector.Vector.Shape.S_256_BIT);
>
>  > var v1 = I256.fromArray(new int[] { 1, 2, 3, 4, 5, 6, 7, 8 }, 0)
> v1 ==> [1, 2, 3, 4, 5, 6, 7, 8]
>
>  > v1.get(0)
>   ==> 1
>
>  > v1.get(7)
>   ==> 8
>
>  > v1.shiftEL(1)
>   ==> [2, 3, 4, 5, 6, 7, 8, 0]
>
>  > v1.shiftEL(1).get(0)
>   ==> 2
>
>  > v1.shiftEL(1).get(7)
>   ==> 0
>
>  > v1.shiftER(1)
>   ==> [0, 1, 2, 3, 4, 5, 6, 7]
>
>  > v1.shiftER(1).get(0)
>   ==> 0
>
>  > v1.shiftER(1).get(7)
>   ==> 7
>
>  > v1.rotateEL(1)
>   ==> [8, 1, 2, 3, 4, 5, 6, 7]
>
> jshell> v1.rotateEL(1).get(0)
>   ==> 8
>
> jshell> v1.rotateEL(1).get(7)
>   ==> 7
>
>  > v1.rotateER(1)
> |  Exception java.lang.ArrayIndexOutOfBoundsException: Index -1 out of
> bounds for length 8
> |        at Int256Vector.rotateER (Int256Vector.java:924)
> |        at Int256Vector.rotateER (Int256Vector.java:39)
> |        at (#44:1)
>
>
> > I started this email having problems using rotateER on vectors. I have
> > not found any way to call this method without getting an exception :).
> > More investigation revealed that shift-ing vector elements does not
> > work in accordance with javadoc. I realised this email will be quite
> > long so I split it up and will describe just shift issues. If my
> > suspicions are correct I will follow up with rotations.
> >
> > First shiftEL operation describes itself "as if rotating left the lane
> > elements by i [...] zero value is placed into the result vector at
> > lane indexes less than i % this.length()."
> >
> > *I suspect that the rotation is described in the wrong direction*.
> > Lets try to confirm it but just looking at zeroed lane indices (using
> > get() which "Gets the lane element at lane index i"):
> >
> > @Test
> > public void shouldShiftElementsLeft() {
> >      // Given
> >      IntVector intVector =
> > IntVector.species(Vector.Shape.S_128_BIT).fromArray(new int[]{
> >              1, 2, 3, 4
> >      }, 0);
> >
> >      // When
> >      intVector = intVector.shiftEL(2);
> >
> >      // Then
> >      assertThat(intVector.get(0)).isEqualTo(0);
> >      assertThat(intVector.get(1)).isEqualTo(0);
> > }
> >
> > So after the shift of "2" lane indexes less than "2" should be zero.
> > This test fails as the rotation is done in the opposite (to me -
> > correct) direction:
> >
> > @Test
> > public void shouldShiftElementsLeft() {
> >      // Given
> >      IntVector intVector =
> > IntVector.species(Vector.Shape.S_128_BIT).fromArray(new int[]{
> >              1, 2, 3, 4
> >      }, 0);
> >
> >      // When
> >      intVector = intVector.shiftEL(2);
> >
> >      // Then
> >      assertThat(intVector.toArray()).containsExactly(3, 4, 0, 0);
> > }
> >
> > Same case is with shiftER. Java doc says that "zero value is placed
> > into the result vector at lane indexes greater or equal to
> > this.length() - (i % this.length()).". But zeroed are indexes < i %
> > this.length().
> >
> > Looks like implementations are correct but javadoc-s are swapped (I
> > hope I am not making fool of myself).
> >
> > Please advise,
> > Tomasz
> >