On Mon, 16 Sep 2024 18:35:42 GMT, Paul Sandoz <[email protected]> wrote:
>> Jatin Bhateja has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Disabling VectorLoadShuffle bypassing optimization to comply with
>> rearrange semantics at IR level.
>
> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/X-Vector.java.template
> line 561:
>
>> 559: for (int i = 0; i < vlen; i++) {
>> 560: int index = ((int)vecPayload1[i]);
>> 561: res[i] = index >= vlen ? vecPayload3[index & (vlen - 1)] :
>> vecPayload2[index];
>
> This is incorrect as the index could be negative. You need to wrap in the
> range `[0, 2 * vlen - 1]` before the comparison and selection.
>
> int index = ((int)vecPayload1[i]) & ((vlen << 1) - 1));
> res[i] = index < vlen ? vecPayload2[index] : vecPayload3[index - vlen];
Hi @PaulSandoz , we already pass wrapped indexes to this helper routine called
from fallback implementation.
> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/X-Vector.java.template
> line 2974:
>
>> 2972: final $abstractvectortype$ selectFromTemplate(Class<? extends
>> Vector<$Boxbitstype$>> indexVecClass,
>> 2973: $abstractvectortype$
>> v1, $abstractvectortype$ v2) {
>> 2974: int twoVectorLen = length() * 2;
>
> We should assert that the length is a power of two.
API only accepts vector parameters and there is no means though public facing
API to create a vector of NPOT sizes.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/20508#discussion_r1762504366
PR Review Comment: https://git.openjdk.org/jdk/pull/20508#discussion_r1762504318
