[lworld+vector] RFR: 8296767: Support multi-field based vector classes.

[Paul Sandoz]

On Thu, 10 Nov 2022 08:15:23 GMT, Jatin Bhateja <jbhateja at openjdk.org> wrote:

> Initial Java side changes for treating vector as a value class based on suggestions from John Rose[1].
> 
> Summary of changes:
> 1) Re-define existing vector class hierarchy to use generic multi-field[2] based payloads of bit sizes 64, 128, 256 and 512.
> 2) Concrete vectors are now value classes encapsulating primitive class instances as payload.
> 3) Update fallback implementation to read/write from multi-field payload using Unsafe APIs.
> 4) New VM symbol definitions for generic payloads.
> 
> All existing JTREG tests are passing with -Xint option.
> 
> C2 side change, runtime support for vector object reconstruction, shuffle and masks handling is still based on existing flow
> and will be handled in subsequent patch.
> 
> MAX species queries target supported max vector size, since multi-field annotation does not accept variable argument hence MAX species based vector class generation has been disabled currently.
> 
> [1] http://cr.openjdk.java.net/~jrose/values/multi-field.html
> [2] https://github.com/fparain/valhalla/tree/multifield

I would recommend if you can to avoid unnecessary renaming such as uOp -> uOpMF, thereby making it easier to read/review as we go.

Something to do in a further iteration: we should be able to place the Unsafe access methods within static methods of the vector payload classe and avoid the use in the vector implementations.

make/common/JavaCompilation.gmk line 272:

> 270:   PARANOIA_FLAGS := -implicit:none -Xprefer:source -XDignore.symbol.file=true -encoding ascii
> 271: 
> 272:   $1_FLAGS += -g -XDenablePrimitiveClasses -Xlint:all $$($1_TARGET_RELEASE) $$(PARANOIA_FLAGS) $$(JAVA_WARNINGS_ARE_ERRORS)

Can you modify `make/modules/java.base/Java.gmk` and possibly `make/modules/jdk.incubator.vector/Java.gmk` instead to limit the scope?

src/java.base/share/classes/jdk/internal/vm/vector/VectorSupport.java line 205:

> 203:         }
> 204: 
> 205:         public static VectorPayloadMF createVectPayloadInstanceB(int elemSize, int length, byte [] init) {

Suggestion:

        public static VectorPayloadMF createVectPayloadInstanceB(int elemSize, int length, byte[] init) {

Same for others.

src/java.base/share/classes/jdk/internal/vm/vector/VectorSupport.java line 211:

> 209:             for (int i = 0; i < length; i++) {
> 210:                 Unsafe.getUnsafe().putByte(obj, start_offset + i * Byte.BYTES, init[i]);
> 211:             }

It may be possibly to unify all these create methods using `Unsafe.copyMemory`

src/jdk.incubator.vector/share/classes/jdk/incubator/vector/gen-src.sh line 60:

> 58:   Type="$(tr '[:lower:]' '[:upper:]' <<< ${type:0:1})${type:1}"
> 59:   TYPE="$(tr '[:lower:]' '[:upper:]' <<< ${type})"
> 60:   Boxinitials="$(tr '[:lower:]' '[:upper:]' <<< ${type:0:1})"

No need for this. Just use the full box type name.

test/jdk/jdk/incubator/vector/ShortMaxVectorTests.java line 27:

> 25:  * @test
> 26:  * @modules jdk.incubator.vector
> 27:  * @run testng/othervm -ea -esa -Xbatch ShortMaxVectorTests

No need for this change and in the other max vector tests?

test/jdk/jdk/incubator/vector/gen-tests.sh line 136:

> 134:   Log true "${Type}:"
> 135: 
> 136:   for bits in 64 128 256 512

No need for this change?

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

PR: https://git.openjdk.org/valhalla/pull/806