On Tue, 1 Nov 2022 23:17:46 GMT, Vladimir Ivanov <vliva...@openjdk.org> wrote:
>> vpaprotsk has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> invalidkeyexception and some review comments
>
> src/hotspot/cpu/x86/stubGenerator_x86_64.cpp line 2002:
>
>> 2000: }
>> 2001:
>> 2002: address StubGenerator::generate_poly1305_masksCP() {
>
> I suggest to turn it into a C++ literal constant and move the declaration
> next to `poly1305_process_blocks_avx512` where they are used. As an example,
> here's how it is handled in GHASH stubs:
>
> https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/stubGenerator_x86_64_ghash.cpp#L35
>
> That would allow to avoid to simplify the code a bit (no need in
> `StubRoutines::x86::_poly1305_mask_addr`/`poly1305_mask_addr()` and no need
> to generate the constants during VM startup).
>
> You could split it into 3 constants, but then using a single base register
> (`polyCP`) won't work anymore.
> Thinking more about it, I'm not sure why you can't just do the split and use
> address literals instead to access individual constants (and repurpose `r13`
> to be used as a scratch register when RIP-relative addressing mode doesn't
> work).
The case of AES stubs may be even a better fit here:
https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/x86/stubGenerator_x86_64_aes.cpp#L47
It doesn't use/introduce any shared constants, so declaring a constant and a
local accessor (to save on pointer to address casts at use sites) is enough.
-------------
PR: https://git.openjdk.org/jdk/pull/10582
