RFR: 8288047: Accelerate Poly1305 on x86_64 using AVX512 instructions [v7]
Volodymyr Paprotski
duke at openjdk.org
Fri Nov 4 03:54:13 UTC 2022
On Fri, 4 Nov 2022 03:20:11 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 12 commits:
>
> - Merge remote-tracking branch 'origin/master' into avx512-poly
> - address Jamil's review
> - invalidkeyexception and some review comments
> - extra whitespace character
> - assembler checks and test case fixes
> - Merge remote-tracking branch 'origin/master' into avx512-poly
> - Merge remote-tracking branch 'origin' into avx512-poly
> - further restrict UsePolyIntrinsics with supports_avx512vlbw
> - missed white-space fix
> - - Fix whitespace and copyright statements
> - Add benchmark
> - ... and 2 more: https://git.openjdk.org/jdk/compare/9d3b4ef2...38d9e83c
@jnimeh Hopefully last change addresses your pending comments.
More data, new data...
datasize | master | optimized | disabled | opt/mst | dis/mst
-- | -- | -- | -- | -- | --
32 | 3218169 | 3476352 | 3126538 | 1.08 | 0.97
64 | 2858030 | 3391015 | 2846735 | 1.19 | 1.00
128 | 2396796 | 3239888 | 2406931 | 1.35 | 1.00
256 | 1780679 | 3063749 | 1765664 | 1.72 | 0.99
512 | 1168824 | 2918524 | 1153009 | 2.50 | 0.99
1024 | 648772.1 | 2716787 | 688467.7 | 4.19 | 1.06
2048 | 357009 | 2382723 | 376023.7 | 6.67 | 1.05
16384 | 48854.33 | 896850 | 53104.68 | 18.36 | 1.09
1048576 | 771.461 | 15088.63 | 846.247 | 19.56 | 1.10
src/hotspot/share/opto/library_call.cpp line 7016:
> 7014: Node* rObj = new CheckCastPPNode(control(), rFace, rtype);
> 7015: rObj = _gvn.transform(rObj);
> 7016: Node* rlimbs = load_field_from_object(rObj, "limbs", "[J");
@jnimeh if you could be particularly 'critical' here please? I generally know what I wanted to accomplish. And stepped through things with a debugger... but all the various IR types and conversions, I just don't know. I copied things from AES, which seem to work, as they do here, but I don't _understand_ the code.
i.e. recursive `getfield`s `((IntegerPolynomial$MutableElement)(this.a)).limbs` plus checks if we know field offsets: if (recursive) classes are loaded. But if not loaded, crashing with assert? Seems 'rude'. I think Poly1305 class constructor running would had forced the classes here to load so nothing to worry about, so I suppose assert is enough.)
src/hotspot/share/opto/library_call.cpp line 7027:
> 7025: // Node* cmp = _gvn.transform(new CmpINode(load_array_length(alimbs), intcon(5)));
> 7026: // Node* bol = _gvn.transform(new BoolNode(cmp, BoolTest::eq));
> 7027: // Node* if_eq = generate_slow_guard(bol, slow_region);
@jnimeh I had "valiantly" tried to do a length check here, but couldn't find where to steal code from! If you have some suggestions...
Meanwhile, I decided that perhaps a Java check would not be _that_ bad for non-intrinsic code. See `checkLimbsForIntrinsic`; I had to change the interface `IntegerModuloP` which initially felt like a hack.
But perhaps the java check is 'alright', reminds java developer that there is a related intrinsic.
test/jdk/com/sun/crypto/provider/Cipher/ChaCha20/unittest/java.base/com/sun/crypto/provider/Poly1305IntrinsicFuzzTest.java line 39:
> 37: public static void main(String[] args) throws Exception {
> 38: //Note: it might be useful to increase this number during development of new Poly1305 intrinsics
> 39: final int repeat = 100;
@jnimeh FYI... In case you end up doing supporting other architectures, left a trail (and lots of 'math' comments in the assembler)
-------------
PR: https://git.openjdk.org/jdk/pull/10582
More information about the security-dev
mailing list