RFR: 8271515: Integration of JEP 417: Vector API (Third Incubator) [v3]

[Paul Sandoz]

On Mon, 18 Oct 2021 22:56:37 GMT, Sandhya Viswanathan <sviswanathan at openjdk.org> wrote:

>> Paul Sandoz has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains seven commits:
>> 
>>  - Merge branch 'master' into JDK-8271515-vector-api
>>  - Apply patch from https://github.com/openjdk/panama-vector/pull/152
>>  - Apply patch from https://github.com/openjdk/panama-vector/pull/142
>>  - Apply patch from https://github.com/openjdk/panama-vector/pull/139
>>  - Apply patch from https://github.com/openjdk/panama-vector/pull/151
>>  - Add new files.
>>  - 8271515: Integration of JEP 417: Vector API (Third Incubator)
>
> src/hotspot/share/utilities/globalDefinitions_vecApi.hpp line 29:
> 
>> 27: // the intent of this file to provide a header that can be included in .s files.
>> 28: 
>> 29: #ifndef SHARE_VM_UTILITIES_GLOBALDEFINITIONS_VECAPI_HPP
> 
> The file src/hotspot/share/utilities/globalDefinitions_vecApi.hpp is not needed.

I notice src/jdk.incubator.vector/windows/native/libsvml/globals_vectorApiSupport_windows.S.inc contains a refence in comments to that file, I presume i can remove that comment too?

> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Byte128Vector.java line 278:
> 
>> 276:     @Override
>> 277:     @ForceInline
>> 278:     public Byte128Vector lanewise(Unary op, VectorMask<Byte> m) {
> 
> Should this method be final as well?

It's actually redundant because the class is final. Better to drop final from all declarations, at the risk of creating a larger diff.

> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/Byte128Vector.java line 321:
> 
>> 319:     public final
>> 320:     Byte128Vector
>> 321:     lanewise(Ternary op, Vector<Byte> v1, Vector<Byte> v2, VectorMask<Byte> m) {
> 
> Should we use VectorOperators.Ternary here?

We static import `import static jdk.incubator.vector.VectorOperators.*;`, and the qualification of enclosing type `VectorOperators` seems inconsistently used. I think we can drop it at all declarations.

> src/jdk.incubator.vector/share/classes/jdk/incubator/vector/ByteVector.java line 603:
> 
>> 601:         if (opKind(op, VO_SPECIAL)) {
>> 602:             if (op == ZOMO) {
>> 603:                 return blend(broadcast(-1), compare(NE, 0, m));
> 
> This doesn't look correct. The lanes where mask is false should get the original lane value in this vector.

That should work, since `compare(NE, 0, m) === compare(NE, 0).and(m)`, so when an `m` lane is unset the lane element of `this` vector will be selected.

Running jshell against a build of PR:

$ ~/Projects/jdk/jdk/build/macosx-x86_64-server-release/images/jdk/bin/jshell --add-modules jdk.incubator.vector
|  Welcome to JShell -- Version 18-internal
|  For an introduction type: /help intro

jshell> import jdk.incubator.vector.*

jshell> var s = IntVector.SPECIES_256;
s ==> Species[int, 8, S_256_BIT]

jshell> var v = IntVector.fromArray(s, new int[]{0, 1, 0, -2, 0, 3, 0, -4}, 0);
v ==> [0, 1, 0, -2, 0, 3, 0, -4]

jshell> var z = v.lanewise(VectorOperators.ZOMO);
z ==> [0, -1, 0, -1, 0, -1, 0, -1]

jshell> z = v.lanewise(VectorOperators.ZOMO, s.loadMask(new boolean[]{false, false, false, false, true, true, true, true}, 0));
z ==> [0, 1, 0, -2, 0, -1, 0, -1]

jshell>

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

PR: https://git.openjdk.java.net/jdk/pull/5873