On Wed, 16 Oct 2024 14:00:37 GMT, Hamlin Li <m...@openjdk.org> wrote:
>> Hi, >> Can you help to review the patch? Previously it's >> https://github.com/openjdk/jdk/pull/18605. >> This pr is based on https://github.com/openjdk/jdk/pull/20781. >> >> Thanks! >> >> ## Test >> ### tests: >> * test/jdk/jdk/incubator/vector/ >> * test/hotspot/jtreg/compiler/vectorapi/ >> >> ### options: >> * -XX:UseSVE=1 -XX:+EnableVectorSupport -XX:+UseVectorStubs >> * -XX:UseSVE=0 -XX:+EnableVectorSupport -XX:+UseVectorStubs >> * -XX:+EnableVectorSupport -XX:-UseVectorStubs >> >> ## Performance >> >> ### Tests >> jmh tests are test/micro/org/openjdk/bench/jdk/incubator/vector/operation/ >> from vectorIntrinsics branch in panama-vector. It's good to have these tests >> in jdk main stream, I will do it in a separate pr later. (These tests are >> auto-generated tests from some script&template, it's good to also have those >> scrip&template in jdk main stream, but those scrip&template generates more >> other tests than what we need here, so better to add these tests and >> script&template in another pr). >> >> ### Options >> * +intrinsic: >> 'FORK=1;ITER=10;WARMUP_ITER=10;JAVA_OPTIONS=-XX:+UnlockExperimentalVMOptions >> -XX:+EnableVectorSupport -XX:+UseVectorStubs' >> * -intrinsic: >> 'FORK=1;ITER=10;WARMUP_ITER=10;JAVA_OPTIONS=-XX:+UnlockExperimentalVMOptions >> -XX:+EnableVectorSupport -XX:-UseVectorStubs' >> >> ### Performance data >> I have re-tested, there is no much difference from >> https://github.com/openjdk/jdk/pull/18605, so please check performance data >> in that pr. > > Hamlin Li has updated the pull request incrementally with one additional > commit since the last revision: > > add missing files src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp line 8207: > 8205: for (int op = 0; op < VectorSupport::NUM_VECTOR_OP_MATH; op++) { > 8206: int vop = VectorSupport::VECTOR_OP_MATH_START + op; > 8207: if (vop == VectorSupport::VECTOR_OP_TANH) { Could you please add a comment that mentions the reason, for example `// Skip "tanh" because there is performance regression` src/hotspot/cpu/aarch64/stubGenerator_aarch64.cpp line 8225: > 8223: for (int op = 0; op < VectorSupport::NUM_VECTOR_OP_MATH; op++) { > 8224: int vop = VectorSupport::VECTOR_OP_MATH_START + op; > 8225: if (vop == VectorSupport::VECTOR_OP_TANH) { Ditto ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/21502#discussion_r1808455401 PR Review Comment: https://git.openjdk.org/jdk/pull/21502#discussion_r1808470048
