On Thu, 17 Apr 2025 18:03:47 GMT, Vladimir Ivanov <vliva...@openjdk.org> wrote:
>> Migrate Vector API math library (SVML and SLEEF) linkage from native code >> (in JVM) to Java FFM API. >> >> Since FFM API doesn't support vector calling conventions yet, migration >> affects only symbol lookup for now. But it still enables significant >> simplifications on JVM side. >> >> The patch consists of the following parts: >> * on-demand symbol lookup in Java code replaces eager lookup from native >> code during JVM startup; >> * 2 new VM intrinsics for vector calls (support unary and binary shapes) >> (code separated from unary/binary vector operations); >> * new internal interface to query supported CPU ISA extensions >> (`jdk.incubator.vector.CPUFeatures`) used for CPU dispatching. >> >> `java.lang.foreign` API is used to perform symbol lookup in vector math >> library, then the address is cached and fed into corresponding JVM >> intrinsic, so C2 can turn it into a direct vector call in generated code. >> >> Once `java.lang.foreign` supports vectors & vector calling conventions, VM >> intrinsics can go away. >> >> Performance is on par with original implementation (tested with >> microbenchmarks on linux-x64 and macosx-aarch64). >> >> Testing: hs-tier1 - hs-tier6, microbenchmarks (on linux-x64 and >> macosx-aarch64) >> >> Thanks! > > 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/88f7b422...88eacc48 I just ran some basic tests on riscv, seems there are some issues, also have some comments. src/hotspot/share/runtime/abstract_vm_version.cpp line 349: > 347: assert(features_offset <= cpu_info_string_len, ""); > 348: if (features_offset < cpu_info_string_len) { > 349: assert(cpu_info_string[features_offset + 0] == ',', ""); This assert fails on riscv. src/hotspot/share/runtime/abstract_vm_version.hpp line 61: > 59: static const char* _features_string; > 60: > 61: static const char* _cpu_info_string; Not quite sure the reason to introduce `_cpu_info_string`. Seems to me you could just use _features_string, and remove _cpu_info_string and its related code, e.g. `extract_features_string`. Please check the code in `test/lib/jdk/test/whitebox/cpuinfo/CPUInfo.java` src/jdk.incubator.vector/share/classes/jdk/incubator/vector/CPUFeatures.java line 45: > 43: String featuresString = VectorSupport.getCPUFeatures(); > 44: debug(featuresString); > 45: String[] features = > featuresString.toLowerCase(Locale.ROOT).split(", "); // ", " is used as a > delimiter On riscv, it's splitted by " ", for the fix please refer to CPUInfo.java in test. src/jdk.incubator.vector/share/classes/jdk/incubator/vector/CPUFeatures.java line 82: > 80: return features; > 81: } > 82: } Maybe an extra line needed at the end of this file? ------------- PR Review: https://git.openjdk.org/jdk/pull/24462#pullrequestreview-2784177749 PR Review Comment: https://git.openjdk.org/jdk/pull/24462#discussion_r2054468091 PR Review Comment: https://git.openjdk.org/jdk/pull/24462#discussion_r2054692791 PR Review Comment: https://git.openjdk.org/jdk/pull/24462#discussion_r2054472788 PR Review Comment: https://git.openjdk.org/jdk/pull/24462#discussion_r2054157269
