[vector api] Vector elements shift confusion

[Vladimir Ivanov]

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
>