RFR: 8288047: Accelerate Poly1305 on x86_64 using AVX512 instructions [v16]
Vladimir Ivanov
vlivanov at openjdk.org
Tue Nov 15 00:23:53 UTC 2022
On Mon, 14 Nov 2022 17:58:36 GMT, Volodymyr Paprotski <duke at openjdk.org> wrote:
>> Handcrafted x86_64 asm for Poly1305. Main optimization is to process 16 message blocks at a time. For more details, left a lot of comments in `macroAssembler_x86_poly.cpp`.
>>
>> - Added new KAT test for Poly1305 and a fuzz test to compare intrinsic and java.
>> - Would like to add an `InvalidKeyException` in `Poly1305.java` (see commented out block in that file), but that conflicts with the KAT. I do think we should detect (R==0 || S ==0) so would like advice please.
>> - Added a JMH perf test.
>> - JMH test had to use reflection (instead of existing `MacBench.java`), since Poly1305 is not 'properly' registered with the provider.
>>
>> Perf before:
>>
>> Benchmark (dataSize) (provider) Mode Cnt Score Error Units
>> Poly1305DigestBench.digest 64 thrpt 8 2961300.661 ± 110554.162 ops/s
>> Poly1305DigestBench.digest 256 thrpt 8 1791912.962 ± 86696.037 ops/s
>> Poly1305DigestBench.digest 1024 thrpt 8 637413.054 ± 14074.655 ops/s
>> Poly1305DigestBench.digest 16384 thrpt 8 48762.991 ± 390.921 ops/s
>> Poly1305DigestBench.digest 1048576 thrpt 8 769.872 ± 1.402 ops/s
>>
>> and after:
>>
>> Benchmark (dataSize) (provider) Mode Cnt Score Error Units
>> Poly1305DigestBench.digest 64 thrpt 8 2841243.668 ± 154528.057 ops/s
>> Poly1305DigestBench.digest 256 thrpt 8 1662003.873 ± 95253.445 ops/s
>> Poly1305DigestBench.digest 1024 thrpt 8 1770028.718 ± 100847.766 ops/s
>> Poly1305DigestBench.digest 16384 thrpt 8 765547.287 ± 25883.825 ops/s
>> Poly1305DigestBench.digest 1048576 thrpt 8 14508.458 ± 56.147 ops/s
>
> Volodymyr Paprotski has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 23 commits:
>
> - Merge remote-tracking branch 'origin/master' into avx512-poly
> - Vladimir's review
> - live review with Sandhya
> - jcheck
> - Sandhya's review
> - fix windows and 32b linux builds
> - add getLimbs to interface and reviews
> - fix 32-bit build
> - make UsePolyIntrinsics option diagnostic
> - Merge remote-tracking branch 'origin/master' into avx512-poly
> - ... and 13 more: https://git.openjdk.org/jdk/compare/e269dc03...a26ac7db
src/hotspot/cpu/x86/stubGenerator_x86_64_poly.cpp line 103:
> 101:
> 102: ATTRIBUTE_ALIGNED(64) uint64_t POLY1305_MASK44[] = {
> 103: // OFFSET 64: mask_44
Redundant comment.
src/hotspot/cpu/x86/stubGenerator_x86_64_poly.cpp line 384:
> 382: void StubGenerator::poly1305_limbs(const Register limbs, const Register a0, const Register a1, const Register a2, bool only128)
> 383: {
> 384: const Register t1 = r13;
Please, make the temps explicit and lift them into arguments. Otherwise, it's hard to see what registers are clobbered when helper methods are called.
src/hotspot/cpu/x86/stubGenerator_x86_64_poly.cpp line 387:
> 385: const Register t2 = r14;
> 386:
> 387: __ movq(a0, Address(limbs, 0));
I don't understand how it works. `limbs` comes directly from `c_rarg2` and contains raw oop. So, `Address(limbs, 0)` reads object mark word rather than the first element from the array.
(Same situation in `poly1305_limbs_out`. And now I'm curious why doesn't object header corruption trigger a crash.)
src/hotspot/cpu/x86/stubGenerator_x86_64_poly.cpp line 987:
> 985:
> 986: // Load R into r1:r0
> 987: poly1305_limbs(R, r0, r1, r1, true);
What's the intention here when you pass `r1` twice? Just load `R[0]` and `R[2]`. You could use `noreg` to mark an optional operation and check for it in `poly1305_limbs` before loading the corresponding element.
-------------
PR: https://git.openjdk.org/jdk/pull/10582
More information about the hotspot-dev
mailing list