[vector] Intrinsics for fromByteArray, fromByteBuffer, intoByteArray, intoByteBuffer
Al K
someusername3 at gmail.com
Tue May 8 23:53:12 UTC 2018
As far as I can tell, I /don't/ think the arr_type calculation would
work w.r.t ByteBuffer. I think, however, since we still have access
to the container argument, we could just check if it's a byte buffer,
that would fix the using_byte_array logic. Though I'm not sure if that's
the correct way of going about it.
On Tue, May 8, 2018 at 6:48 PM, Paul Sandoz <paul.sandoz at oracle.com> wrote:
>
>
> > On May 8, 2018, at 3:56 PM, Al K <someusername3 at gmail.com> wrote:
> >
> > Right, concern is mainly around the type checking/casting and correct
> > use, I haven't dealt with any of this before so it's new to me. To
> clarify
> > I'm accessing the base + address correctly, as you suggested, using
> > var handles as an example, f.e. store,
> >
> > @Override
> > @ForceInline
> > public void intoByteBuffer(ByteBuffer bb, int ix) {
> > if (!bb.isReadOnly() && bb.order() == ByteOrder.nativeOrder()) {
> > int num_bytes = bitSize() / Byte.SIZE;
> > int ax = VectorIntrinsics.checkIndex(ix, bb.limit(),
> num_bytes);
> > VectorIntrinsics.store($vectortype$.class, $type$.class,
> LENGTH,
> > bb, VectorIntrinsics.UNSAFE.getObject(bb,
> BYTE_BUFFER_HB),
> > VectorIntrinsics.UNSAFE.getLong(bb,
> BUFFER_ADDRESS) + ax,
> > this, (base, off) ->
> super.intoByteBuffer(bb, ax));
> > } else {
> > super.intoByteBuffer(bb, ix);
> > }
> > }
> >
>
> Great!
>
>
> > But past that, I'm unsure what inline_vector_mem_operation should look
> like.
> > The diff I pasted above seems to work, and generates okay assembly,
> though
> > I haven't tested it much.
>
> I am not an except here but what you have done seem like in the right
> direction. The element type is tracked in the unsafe address node.
>
> I am unsure about the calculation of arr_type, does it works for
> ByteBuffer? we may need to be consistent in those two cases, if the
> using_byte_array logic is still required (which may more accurately means
> we are scribbling on a memory region).
>
> Paul.
>
> >
> > On Tue, May 8, 2018 at 5:43 PM, Paul Sandoz <paul.sandoz at oracle.com>
> wrote:
> >
> >
> >> On May 8, 2018, at 3:31 PM, Al K <someusername3 at gmail.com> wrote:
> >>
> >> Got it, thanks. Mainly wondering what the changes for Paul's suggestion
> would
> >> look like, I don't have a good idea as to how the on-/off-heap would
> work for
> >> a byte buffer.
> >
> > It’s easy to uniformly extract out the base + address from a byte
> buffer. That’s what VarHandles do (using unsafe accesses), and also
> vectorized buffer comparison support (using methods/fields because its in
> the same package, plus there are some other constraints for uniform access
> across all buffers, not just byte buffers) e.g. see code in
> java.nio.BufferMismatch:
> > static int mismatch(ByteBuffer a, int aOff, ByteBuffer b, int bOff, int
> length) {
> > int i = 0;
> > if (length > 7) {
> > i = ArraysSupport.vectorizedMismatch(
> > a.base(), a.address + aOff,
> > b.base(), b.address + bOff,
> > length,
> > ArraysSupport.LOG2_ARRAY_BYTE_INDEX_SCALE);
> > if (i >= 0) return i;
> > i = length - ~i;
> > }
> > for (; i < length; i++) {
> > if (a.get(aOff + i) != b.get(bOff + i))
> > return i;
> > }
> > return -1;
> > }
> > Paul.
> >
> >> Something like below is what I have, though I don't think heap byte
> >> buffers would result in using_byte_array being true or if the usage is
> correct at all,
> >>
> >> - Node* arr = argument(3);
> >> - Node* idx = argument(4);
> >> -
> >> - const TypeAryPtr* arr_type = gvn().type(arr)->isa_aryptr();
> >> - if (arr_type == NULL) {
> >> - return false; // should be an array
> >> - }
> >> + Node* container = argument(3);
> >> + Node* base = argument(4);
> >> + Node* offset = ConvL2X(argument(5));
> >> + Node* addr = make_unsafe_address(base, offset, elem_bt, true);
> >> + const TypePtr *addr_type = gvn().type(addr)->isa_ptr();
> >> +
> >> + const TypeAryPtr* arr_type = addr_type->isa_aryptr();
> >>
> >> // Now handle special case where load/store happens from/to byte
> array but element type is not byte.
> >> - bool using_byte_array = arr_type->elem()->array_element_basic_type()
> == T_BYTE && elem_bt != T_BYTE;
> >> + bool using_byte_array = arr_type != NULL && arr_type->elem()->array_element_basic_type()
> == T_BYTE && elem_bt != T_BYTE;
> >>
> >> // It must be the case that if it is not special byte array case,
> there is consistency between
> >> // array and vector element types.
> >> - if (!using_byte_array && elem_bt != arr_type->elem()->array_element_basic_type())
> {
> >> + if (!using_byte_array && arr_type != NULL && elem_bt !=
> arr_type->elem()->array_element_basic_type()) {
> >> return false;
> >> }
> >>
> >> - Node* adr = array_element_address(arr, idx, using_byte_array ?
> T_BYTE : elem_bt);
> >> - const TypePtr* adr_type = adr->bottom_type()->is_ptr();
> >> + /*Node* adr = array_element_address(base, idx, using_byte_array ?
> T_BYTE : elem_bt);
> >> + const TypePtr* adr_type = adr->bottom_type()->is_ptr();*/
> >>
> >> ciKlass* vbox_klass = vector_klass->const_oop()->as_
> instance()->java_lang_Class_klass();
> >> const TypeInstPtr* vbox_type = TypeInstPtr::make_exact(TypePtr::NotNull,
> vbox_klass);
> >>
> >> if (is_store) {
> >> - Node* val = unbox_vector(argument(5), vbox_type, elem_bt,
> num_elem);
> >> + Node* val = unbox_vector(argument(6), vbox_type, elem_bt,
> num_elem);
> >> if (val == NULL) {
> >> return false; // operand unboxing failed
> >> }
> >> @@ -7440,19 +7440,19 @@
> >> val = gvn().transform(new VectorReinterpretNode(val,
> val->bottom_type()->is_vect(), to_vect_type));
> >> }
> >>
> >> - Node* vstore = gvn().transform(StoreVectorNode::make(0,
> control(), memory(adr), adr, adr_type, val, store_num_elem));
> >> - set_memory(vstore, adr_type);
> >> + Node* vstore = gvn().transform(StoreVectorNode::make(0,
> control(), memory(addr), addr, addr_type, val, store_num_elem));
> >> + set_memory(vstore, addr_type);
> >> set_vector_result(vstore, false);
> >> } else {
> >> // When using byte array, we need to load as byte then reinterpret
> the value. Otherwise, do a simple vector load.
> >> Node* vload = NULL;
> >> if (using_byte_array) {
> >> int load_num_elem = num_elem * type2aelembytes(elem_bt);
> >> - vload = gvn().transform(LoadVectorNode::make(0, control(),
> memory(adr), adr, adr_type, load_num_elem, T_BYTE));
> >> + vload = gvn().transform(LoadVectorNode::make(0, control(),
> memory(addr), addr, addr_type, load_num_elem, T_BYTE));
> >> const TypeVect* to_vect_type = TypeVect::make(elem_bt, num_elem);
> >> vload = gvn().transform(new VectorReinterpretNode(vload,
> vload->bottom_type()->is_vect(), to_vect_type));
> >> } else {
> >> - vload = gvn().transform(LoadVectorNode::make(0, control(),
> memory(adr), adr, adr_type, num_elem, elem_bt));
> >> + vload = gvn().transform(LoadVectorNode::make(0, control(),
> memory(addr), addr, addr_type, num_elem, elem_bt));
> >> }
> >>
> >> On Mon, May 7, 2018 at 3:36 PM, Lupusoru, Razvan A <
> razvan.a.lupusoru at intel.com> wrote:
> >> Hi Al,
> >>
> >>
> >>
> >> I am not quite certain of the answer to your question but I’d like to
> explain why that node is used in implementation.
> >>
> >>
> >>
> >> Basically, VectorReinterpretNode is intended to allow “type conversion”
> of vectors while keeping contents unmodified. It also allows for compiler
> to keep track of appropriate types. It does not have anything to do with
> whether an access is on heap or off heap.
> >>
> >>
> >>
> >> So let us say we have a node of TypeVect “#vectory[8]:{float}”. Since
> when using ByteBuffer and byte arrays we explicitly check that we have a
> byte array, we mark that the “address” of the array is of type
> “@byte[int:>=0]:exact+any *”. Thus the VectorReinterpret node is used as a
> transition node to convert from “#vectory[8]:{float}” to type
> “#vectory[32]:{byte}”. Thus now we have checkable strong typing in place
> where we do not mix and allow store to memory that does not match the type
> of what we are storing.
> >>
> >>
> >>
> >> I am not very familiar with the type of unsafe memory and whether
> compiler will complain if your vector type is not consistent with your
> memory type. My guess is that there is special marking used for unsafe
> accesses - and thus it would seem to me that VectorReinterpret node is not
> necessarily needed. That said, the node itself does not add any overheads
> (as long as input and output type have same size), and thus it’s not a
> problem to keep it.
> >>
> >>
> >>
> >> Hopefully my answer is somewhat helpful in answering your question.
> >>
> >>
> >>
> >> Thanks,
> >>
> >> Razvan
> >>
> >>
> >>
> >> From: Al K [mailto:someusername3 at gmail.com]
> >> Sent: Monday, May 07, 2018 12:51 PM
> >> To: Lupusoru, Razvan A <razvan.a.lupusoru at intel.com>
> >> Cc: Paul Sandoz <paul.sandoz at oracle.com>; John Rose <
> john.r.rose at oracle.com>; panama-dev at openjdk.java.net
> >>
> >>
> >> Subject: Re: [vector] Intrinsics for fromByteArray, fromByteBuffer,
> intoByteArray, intoByteBuffer
> >>
> >>
> >>
> >> Is VectorReinterpretNode required when loading from/storing to off-heap
> buffer? Wondering
> >>
> >> if it's sufficient enough to just invoke make_unsafe_address() with the
> base and offset, and
> >>
> >> continue on, or if there's more complications associated with it.
> >>
> >>
> >>
> >> On Fri, May 4, 2018 at 1:54 PM, Lupusoru, Razvan A <
> razvan.a.lupusoru at intel.com> wrote:
> >>
> >> Paul and John,
> >>
> >> I just merged the patch with the following changes which stemmed from
> your discussion:
> >>
> >> - No longer allow writing to a read only buffer
> >>
> >> - No longer intrinsify if bytebuffer endianness does not match
> native order
> >>
> >> o There is no silent conversion taking place.
> >>
> >> Thanks for your reviews in finding the various issues. I agree that a
> worthy improvement would be to allow writing to buffers that do not have
> backing array by using the Unsafe mechanism you recommended.
> >>
> >> Thanks!
> >>
> >> --Razvan
> >>
> >> From: Paul Sandoz [mailto:paul.sandoz at oracle.com]
> >> Sent: Thursday, May 03, 2018 8:52 AM
> >> To: John Rose <john.r.rose at oracle.com>
> >> Cc: Lupusoru, Razvan A <razvan.a.lupusoru at intel.com>;
> panama-dev at openjdk.java.net
> >> Subject: Re: [vector] Intrinsics for fromByteArray, fromByteBuffer,
> intoByteArray, intoByteBuffer
> >>
> >>
> >>
> >>
> >> On May 3, 2018, at 8:48 AM, John Rose <john.r.rose at oracle.com<mailto:
> john.r.rose at oracle.com>> wrote:
> >>
> >> On May 3, 2018, at 8:40 AM, Paul Sandoz <paul.sandoz at oracle.com<mailto:
> paul.sandoz at oracle.com>> wrote:
> >>
> >> stick to native endianness for the moment to keep things simpler
> initially
> >>
> >> I suggest throwing an exception if the buffer is in the wrong
> endianness.
> >> We have regretted silently converting to native on other occasions.
> >>
> >> A good point, it will remind us to fix it and revisit the
> specification. I will add placeholder in the load/store tests.
> >>
> >> Paul.
> >>
> >>
> >>
> >>
> >
> >
>
>
More information about the panama-dev
mailing list