[vector] ByteBuffer byte ordering

[Lupusoru, Razvan A]

I uploaded the patch with updated fix that does not override the ByteBuffer byte order but internally for other methods which do not specify, it uses native order. I also left the rebracket complex byte ordering explanation in place but we can remove if needed. Thanks again for your advice and for noting that byte buffer implicit ordering is design smell. I would be happy to assist with a better solution. For now - using native order works well for intrinsification since it makes it so that vector operation semantics are consistent with java implementation.

--Razvan

From: Paul Sandoz [mailto:paul.sandoz at oracle.com]
Sent: Friday, April 13, 2018 2:39 PM
To: Lupusoru, Razvan A <razvan.a.lupusoru at intel.com>
Cc: John Rose <john.r.rose at oracle.com>; panama-dev at openjdk.java.net
Subject: Re: [vector] ByteBuffer byte ordering




On Apr 13, 2018, at 2:09 PM, Lupusoru, Razvan A <razvan.a.lupusoru at intel.com<mailto:razvan.a.lupusoru at intel.com>> wrote:

I agree - the correct code actually should have preserved byte order for all API methods that actually take in ByteBuffer argument.
Here is updated patch: http://cr.openjdk.java.net/~rlupusoru/panama/byteorder_01/byteorder_01.patch

Namely, the fixed version does the following things:
-          For byte arrays, since internally it uses byte buffer, it uses native ordering

For now that’s ok, but i think we will need to revisit what the default order should be, and we should expand to accept an order parameter.



-          For methods accepting ByteBuffer, they need to maintain ordering.
o   For case where ByteBuffer is duplicated (namely the version taking ByteBuffer + index), the order must be overridden since duplication of ByteBuffer does not preserve original ordering but updates it to be BIG_ENDIAN.


Ok. We can sweep up the other ByteBuffer accepting methods in another round.

I am unsure about the need for such explanatory in the reshape/rebracket, ok for now, can revisit later.

Thanks,
Paul.

Let me know what you think now! Thanks for catching this error.

--Razvan

From: John Rose [mailto:john.r.rose at oracle.com]
Sent: Friday, April 13, 2018 1:12 PM
To: Paul Sandoz <paul.sandoz at oracle.com<mailto:paul.sandoz at oracle.com>>
Cc: Lupusoru, Razvan A <razvan.a.lupusoru at intel.com<mailto:razvan.a.lupusoru at intel.com>>; panama-dev at openjdk.java.net<mailto:panama-dev at openjdk.java.net>
Subject: Re: [vector] ByteBuffer byte ordering

On Apr 13, 2018, at 12:40 PM, Paul Sandoz <paul.sandoz at oracle.com<mailto:paul.sandoz at oracle.com>> wrote:

I still consider it an open question as to whether native order is the correct approach. For now i think what we have is ok, and ensures consistency with intrinsics.

Intuitively it appears ok at least for byte[], as there is a clear mapping to the vector load/stores. But those bytes may have come from somewhere else (e.g. off the network) so we may need to provide an optional argument specifying the byte-order (when order == native-order all is good, and when not some permutation of those bytes is required).

I question this also.  BB's have an ordering flag for a reason, and it is risky to just reset it
to native for some operation.

A more correct approach would be to sense the BB's declared ordering flag, and then
perform swaps (only if needed) when loading data from the BB.

Of course, this more correct approach is most correct if the vector lanes are the
same size at the BB elements (so we're really talking about LongBuffer, etc.).
But it would be reasonable (less surprising on balance) to respect the flag,
or if that is impossible, throw an exception, rather than silently changing the
byte order.

— John