On Fri, 18 Apr 2025 16:16:28 GMT, Jorn Vernee <jver...@openjdk.org> wrote:
>> Vladimir Ivanov has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 24 additional >> commits since the last revision: >> >> - Merge remote-tracking branch 'origin/master' into vector.math.01.java >> - RVV and SVE adjustments >> - fix broken merge >> - Merge branch 'master' into vector.math.01.java >> - Fix debugName handling >> - Merge branch 'master' into vector.math.01.java >> - RVV and SVE adjustments >> - Merge branch 'master' into vector.math.01.java >> - Fix windows-aarch64 build failure >> - features_string -> cpu_info_string >> - ... and 14 more: https://git.openjdk.org/jdk/compare/685357bf...88eacc48 > > src/hotspot/share/prims/vectorSupport.cpp line 622: > >> 620: >> 621: ThreadToNativeFromVM ttn(thread); >> 622: return env->NewStringUTF(features_string); > > Isn't there a way to do this without the extra transition? > > How about: > > > oop result = java_lang_String::create_oop_from_str((char*) bytes, > CHECK_NULL); > return (jstring) JNIHandles::make_local(THREAD, result); Fair enough. > src/jdk.incubator.vector/share/classes/jdk/incubator/vector/VectorMathLibrary.java > line 272: > >> 270: MemorySegment addr = LOOKUP.findOrThrow(symbol); >> 271: debug("%s %s => 0x%016x\n", op, symbol, addr.address()); >> 272: T impl = implSupplier.apply(opc); // TODO: should call >> the very same native implementation eventually (once FFM API supports >> vectors) > > FWIW, one current barrier I see to implementing the vector calling convention > in the linker, is that the FFM linker (currently) transmits register values > to the downcall stub use Java primitive types. So, in order to support vector > calling conventions, we would need to add some kind of 'primitive' that can > hold the entire vector value, and preferably gets passed in the right > register. > > However, I think in the case of these math libraries in particular, speed of > the fallback implementation is not that much of an issue, since there is also > an intrinsic. So alternatively, we could split a vector value up into smaller > integral types (`int`, `long`) -> pass them to the downcall stub in that form > -> and then reconstruct the full vector value in its target register. (I used > the same trick when I was experimenting with FP80 support, which also > requires splitting up the 80 bit value up into 2 `long`s). IMO an in-memory representation for vectors is preferred when it comes to FFM linker calling conventions. 512-bit vector requires 8 longs, so some of them will end up passed on stack for any non-trivial case. And with in-memory representation, VM can elide vector store/load once FFM linker stub is intrinsified. > src/jdk.incubator.vector/share/classes/jdk/incubator/vector/VectorMathLibrary.java > line 305: > >> 303: @ForceInline >> 304: /*package-private*/ static >> 305: <E, V extends Vector<E>> > > Here you're using `Vector` instead of `VectorPayload` for the binary op, so > there seems to be a discrepancy with `VectorSupport`. I don't have a strong preference, but I kept it aligned with `unaryOp`/`binaryOp` intrinsics. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24462#discussion_r2055086616 PR Review Comment: https://git.openjdk.org/jdk/pull/24462#discussion_r2055086145 PR Review Comment: https://git.openjdk.org/jdk/pull/24462#discussion_r2055087281
