[vectorIntrinsics] RFR: Load store memory segment
Maurizio Cimadamore
mcimadamore at openjdk.java.net
Wed Mar 30 08:30:01 UTC 2022
On Tue, 29 Mar 2022 21:47:02 GMT, Paul Sandoz <psandoz at openjdk.org> wrote:
> This is the initial implementation for loading/storing vectors from/to memory segments.
>
> Some of the changes are related to `int` -> `long` for the offset. I had to change the load/store intrinsic signatures to accept an offset as a `long`. This does not affect the C2 implementation, only the fallback lambdas. So in some cases there is a downcast to `int`, which is safe because the original input was an `int` e.g. the offset for primitive array access.
>
> Some changes related to module dependency and memory segment proxies will go away when the Memory Segment API is integrated into `java.base` as a preview API.
>
> There is still more work to do but i wanted to get this in now and iterate:
> - I will remove the `byte[]` and `ByteBuffer` accepting load/store methods in a following PR.
> - some additional negative tests are required, specifically if a scope of segment is not alive, or the segment covers a heap array other than `byte[]`.
Looks good
src/java.base/share/classes/jdk/internal/misc/X-ScopedMemoryAccess.java.template line 674:
> 672: private static
> 673: <V extends VectorSupport.Vector<E>, E>
> 674: void storeIntoMemorySegmentScoped(ScopedMemoryAccess.Scope scope,
I believe the rest of the class uses "internal" as a suffix for the scoped method
src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Vector.java line 3527:
> 3525: * {@link #intoMemorySegment(MemorySegment,long,ByteOrder,VectorMask)
> 3526: * intoMemorySegment()} as follows:
> 3527: * <pre>{@code
you might want to revisit later to use code snippets
src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Vector.java line 3546:
> 3544: * or if access occurs from a thread other than the thread owning the session.
> 3545: */
> 3546: public abstract void intoMemorySegment(MemorySegment ms, long offset, ByteOrder bo);
Just a note - in the memory segment API we used to have API points taking an explicit byte order (e.g. dereference methods) and it turned out these were just specific instances of a more general API method accepting a full layout. I sense the same might apply here - e.g. if you take a segment, an offset and a layout, then you might be able to generalize the API to more than byte[]-backed memory segment. But for now it's fine. Or - maybe, in this case the vector instance should have a lane layout somewhere, which can be used here.
test/jdk/jdk/incubator/vector/Byte128VectorLoadStoreTests.java line 970:
> 968:
> 969: try {
> 970: SPECIES.zero().intoMemorySegment(a, 0, bo);
I think these can be written more succinctly with `expectThrows(UnsupportedOperationException.class, () -> { ... })`
-------------
Marked as reviewed by mcimadamore (Committer).
PR: https://git.openjdk.java.net/panama-vector/pull/187
More information about the panama-dev
mailing list