RFR: 8284932: [Vector API] Incorrect implementation of LSHR operator for negative byte/short elements

[Eric Liu]

On Mon, 18 Apr 2022 04:14:39 GMT, Jie Fu <jiefu at openjdk.org> wrote:

>> Hi,
>> 
>> Because unsigned cast should operate on unsigned types, the more appropriate usage is `(src[i] & 0xFF) >>> 3`, with the `&` operation is the cast from unsigned byte to int. Actually, I fail to understand the intention of your example, why not use signed shift instead, what does unsigned shift provide here apart from extra cognitive load in reasoning the operation.
>> 
>> May you provide a more concrete example to the utilisation of unsigned shift on signed subword types, please. The example provided by @fg1417 in #7979 seems to indicate the real intention is to right shifting unsigned bytes, with the unsigned cast sometimes omitted (changed to a signed cast) because the shift results are masked by a stricter mask immediately.
>> 
>> Thank you very much.
>
>> Because unsigned cast should operate on unsigned types, the more appropriate usage is `(src[i] & 0xFF) >>> 3`, with the `&` operation is the cast from unsigned byte to int. Actually, I fail to understand the intention of your example, why not use signed shift instead, what does unsigned shift provide here apart from extra cognitive load in reasoning the operation.
> 
> 
> The fact is that you can't prevent developers from using `>>>` upon negative elements since neither the JVMS nor the JLS prevents it.
> 
> 
>> May you provide a more concrete example to the utilisation of unsigned shift on signed subword types, please. The example provided by @fg1417 in #7979 seems to indicate the real intention is to right shifting unsigned bytes, with the unsigned cast sometimes omitted (changed to a signed cast) because the shift results are masked by a stricter mask immediately.
> 
> Sorry, I can't show the detail of our customer's code.
> However, just image that someone would like to optimize some code segments of bytes/shorts `>>>`, how can you say there should be always non-negative operands?

@DamonFool 

I think the issue is that these two cases of yours are not equal semantically.


 13     public static void urshift(byte[] src, byte[] dst) {
 14         for (int i = 0; i < src.length; i++) {
 15             dst[i] = (byte)(src[i] >>> 3);
 16         }
 17     }
 18 
 19     public static void urshiftVector(byte[] src, byte[] dst) {
 20         int i = 0;
 21         for (; i < spec.loopBound(src.length); i +=spec.length()) {
 22             var va = ByteVector.fromArray(spec, src, i);
 23             var vb = va.lanewise(VectorOperators.LSHR, 3);
 24             vb.intoArray(dst, i);
 25         }
 26 
 27         for (; i < src.length; i++) {
 28             dst[i] = (byte)(src[i] >>> 3);
 29         }
 30     }


Besides the unsigned shift, line15 also has a type conversion which is missing in the vector api case. To get the equivalent result, one need to cast the result explicitly at line24, e.g, `((IntVector)vb.castShape(SPECISE_XXX, 0)).intoArray(idst, i);`

-------------

PR: https://git.openjdk.java.net/jdk/pull/8276