On Tue, 15 Nov 2022 19:43:11 GMT, Volodymyr Paprotski <d...@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 25 commits:
> 
>  - Vladimir's review comments
>  - Merge remote-tracking branch 'origin/master' into avx512-poly
>  - 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
>  - ... and 15 more: https://git.openjdk.org/jdk/compare/7357a1a3...8f5942d9

src/hotspot/cpu/x86/stubGenerator_x86_64_poly.cpp line 896:

> 894: 
> 895:   // Cleanup
> 896:   __ vpxorq(xmm0, xmm0, xmm0, Assembler::AVX_512bit);

What's the purpose of the cleanup?

src/hotspot/cpu/x86/stubGenerator_x86_64_poly.cpp line 1004:

> 1002:   __ jcc(Assembler::less, L_process16Loop);
> 1003: 
> 1004:   poly1305_process_blocks_avx512(input, length,

I'd like to see a comment here explaining what register effects are implicit.

`poly1305_process_blocks_avx512` has the following comment, but it doesn't 
mention xmm registers:

  // Register Map:
  // reserved: rsp, rbp, rcx
  // PARAMs: rdi, rbx, rsi, r8-r12
  // poly1305_multiply_scalar clobbers: r13-r15, rax, rdx

-------------

PR: https://git.openjdk.org/jdk/pull/10582

Reply via email to