[vector] unify load/store tests

[Paul Sandoz]

> On May 22, 2018, at 11:53 AM, Lupusoru, Razvan A <razvan.a.lupusoru at intel.com> wrote:
> 
> Wow! The patch looks great! Good job and thanks so much for doing this.
> 

Thanks.


> Everything looks good except one change - it seems that you removed check for ByteBuffer which only intrinsifies if ByteBuffer is native order. I wonder if you should keep that check in place since intrinsification leads to instructions that use native ordering while the input ByteBuffer may be marked as the other ordering. I believe the previous implementation was correct because it did not override the ByteBuffer ordering... Or did you have another plan for fixing the byte ordering issue?
> 

No yet. 

Perhaps it's better for now to reject buffers whose order is not native. How about this:

diff -r 72bd69f0f38e src/jdk.incubator.vector/share/classes/jdk/incubator/vector/X-VectorBits.java.template
--- a/src/jdk.incubator.vector/share/classes/jdk/incubator/vector/X-VectorBits.java.template	Tue May 22 11:54:03 2018 -0700
+++ b/src/jdk.incubator.vector/share/classes/jdk/incubator/vector/X-VectorBits.java.template	Tue May 22 12:26:49 2018 -0700
@@ -860,6 +860,9 @@
     @ForceInline
     public void intoByteBuffer(ByteBuffer bb, int ix) {
         // @@@ Endianess
+        if (bb.order() != ByteOrder.nativeOrder()) {
+            throw new IllegalArgumentException();
+        }
         if (bb.isReadOnly()) {
             throw new ReadOnlyBufferException();
         }
@@ -1548,6 +1551,9 @@
         @ForceInline
         public $vectortype$ fromByteBuffer(ByteBuffer bb, int ix) {
             // @@@ Endianess
+            if (bb.order() != ByteOrder.nativeOrder()) {
+                throw new IllegalArgumentException();
+            }
             ix = VectorIntrinsics.checkIndex(ix, bb.limit(), bitSize() / Byte.SIZE);
             return VectorIntrinsics.load($vectortype$.class, $type$.class, LENGTH,
                                          U.getObject(bb, BYTE_BUFFER_HB), U.getLong(bb, BUFFER_ADDRESS) + ix,

Otherwise, it’s possible to separate out the default implementation and call it separately, but i would prefer to fail for now and fix this properly later on. To do that we will need to reverse the bytes of the vector elements. Are there any vector instructions to reverse bytes? and if so perhaps we need to expose that operation in the API?

Plus for the to/from byte[] we may need to pass in the byte order as a argument.

Paul.


> --Razvan
> 
> -----Original Message-----
> From: panama-dev [mailto:panama-dev-bounces at openjdk.java.net] On Behalf Of Paul Sandoz
> Sent: Tuesday, May 22, 2018 11:31 AM
> To: panama-dev at openjdk.java.net
> Subject: [vector] unify load/store tests
> 
> Hi,
> 
> Here is an updated patch for unifying load/store. This is merged with the load/store tests.
> 
>  http://cr.openjdk.java.net/~psandoz/panama/unify-load-store/webrev/
> 
> - The relative buffer accessors are removed.
> 
> - The load/store intrinsics take a base + address for intrinsic access and a container + index for default access.
> 
> - Related methods were made abstract or final on the public primitive vectors.
> 
> - Redundant casts were removed from the implementation.
> 
> - Type variable names of intrinsic methods were changed for those that accept a Vector or Mask (alas we cannot represent this in the type system unless Vector and Mask share are common super type other than Object).
> 
> All tests pass locally for me in fast debug mode with -XX:+UseVectorApiIntrinsics -XX:-TieredCompilation.
> 
> Thanks,
> Paul.