On Mon, 18 Oct 2021 22:56:37 GMT, Sandhya Viswanathan 
<sviswanat...@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

Reply via email to