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