On Thu, 5 Oct 2023 05:13:19 GMT, Smita Kamath <svkam...@openjdk.org> wrote:
>> Hi All, >> I would like to submit AES-GCM optimization for x86_64 architectures using >> AVX2 instructions. This optimization interleaves AES and GHASH operations. >> >> Below are the performance numbers on my desktop system with -XX:UseAVX=2 >> option: >> >> |Benchmark | Data Size | Base version (ops/s) | Patched version (ops/s) | >> Speedup >> |-------------|------------|---------------|------------------|-----------| >> |full.AESGCMBench.decrypt | 8192 | 526274.678 | 670014.543 | 1.27 >> full.AESGCMBench.encrypt | 8192 | 538293.315 | 680716.207 | 1.26 >> small.AESGCMBench.decrypt | 8192 | 527854.353 |663131.48 | 1.25 >> small.AESGCMBench.encrypt | 8192 | 548193.804 | 683624.232 |1.24 >> full.AESGCMBench.decryptMultiPart | 8192 | 299865.766 | 299815.851 | 0.99 >> full.AESGCMBench.encryptMultiPart | 8192 | 534406.564 |539235.462 | 1.00 >> small.AESGCMBench.decryptMultiPart | 8192 | 299960.202 |298913.629 | 0.99 >> small.AESGCMBench.encryptMultiPart | 8192 | 542669.258 | 540552.293 | 0.99 >> | | | | >> full.AESGCMBench.decrypt | 16384 | 307266.364 |390397.778 | 1.27 >> full.AESGCMBench.encrypt | 16384 | 311491.901 | 397279.681 | 1.27 >> small.AESGCMBench.decrypt | 16384 | 306257.801 | 389531.665 |1.27 >> small.AESGCMBench.encrypt | 16384 | 311468.972 | 397804.753 | 1.27 >> full.AESGCMBench.decryptMultiPart | 16384 | 159634.341 | 181271.487 | 1.13 >> full.AESGCMBench.encryptMultiPart | 16384 | 308980.992 | 385606.113 | 1.24 >> small.AESGCMBench.decryptMultiPart | 16384 | 160476.064 |181019.205 | 1.12 >> small.AESGCMBench.encryptMultiPart | 16384 | 308382.656 | 391126.417 | 1.26 >> | | | | >> full.AESGCMBench.decrypt | 32768 | 162284.703 | 213257.481 |1.31 >> full.AESGCMBench.encrypt | 32768 | 164833.104 | 215568.639 | 1.30 >> small.AESGCMBench.decrypt | 32768 | 164416.491 | 213422.347 | 1.29 >> small.AESGCMBench.encrypt | 32768 | 166619.205 | 214584.208 |1.28 >> full.AESGCMBench.decryptMultiPart | 32768 | 83306.239 | 93762.988 |1.12 >> full.AESGCMBench.encryptMultiPart | 32768 | 166109.391 |211701.969 | 1.27 >> small.AESGCMBench.decryptMultiPart | 32768 | 83792.559 | 94530.786 | 1.12 >> small.AESGCMBench.encryptMultiPart | 32768 | 162975.904 |212085.047 | 1.30 >> | | | | >> full.AESGCMBench.decrypt | 65536 | 85765.835 | 112244.611 | 1.30 >> full.AESGCMBench.encrypt | 65536 | 86471.805 | 113320.536 |1.31 >> small.AESGCMBench.decrypt | 65536 | 84490.816 | 112122.358 |1.32 >> small.AESGCMBench.encrypt | 65536 | 85403.025 | 112741.811 | 1.32 >> full.AES... > > Smita Kamath has updated the pull request incrementally with one additional > commit since the last revision: > > Reorganized code as per comments, added new instruction addb src/hotspot/cpu/x86/stubGenerator_x86_64_aes.cpp line 354: > 352: // Save rbp and rsp > 353: __ push(rbp); > 354: __ movq(rbp, rsp); This line breaks stack walking code, at least on Linux; rbp is supposed to be the frame pointer throughout the stub. src/hotspot/cpu/x86/stubGenerator_x86_64_aes.cpp line 362: > 360: __ vzeroupper(); > 361: __ movq(rsp, rbp); > 362: __ pop(rbp); If you remove `movq(rbp, rsp)` above, you can replace this with: Suggestion: __ lea(rsp, Address (rbp, WINDOWS_ONLY(-7) NOT_WINDOWS(-5) * wordSize)); src/hotspot/cpu/x86/stubGenerator_x86_64_aes.cpp line 3342: > 3340: __ movdqu(xmm1, xmm2); > 3341: __ vpslldq(xmm2, xmm2, 8, Assembler::AVX_128bit); > 3342: __ vpsrldq(xmm1, xmm1, 8, Assembler::AVX_128bit); You could save a few bytes here: Suggestion: __ vpsrlq(xmm1, xmm6, 63, Assembler::AVX_128bit); __ vpsllq(xmm6, xmm6, 1, Assembler::AVX_128bit); __ vpslldq(xmm2, xmm1, 8, Assembler::AVX_128bit); __ vpsrldq(xmm1, xmm1, 8, Assembler::AVX_128bit); src/hotspot/cpu/x86/stubGenerator_x86_64_aes.cpp line 3710: > 3708: > 3709: // Generate 8 constants for htbl > 3710: __ call(generate_htbl_8_blks, relocInfo::none); why didn't you inline `generateHtbl_8_block_avx2` here? This method is only used here as far as I can tell. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/15410#discussion_r1348346370 PR Review Comment: https://git.openjdk.org/jdk/pull/15410#discussion_r1348347451 PR Review Comment: https://git.openjdk.org/jdk/pull/15410#discussion_r1348372462 PR Review Comment: https://git.openjdk.org/jdk/pull/15410#discussion_r1348349329
