On Wed, 9 Nov 2022 02:19:29 GMT, Volodymyr Paprotski <d...@openjdk.org> wrote:

>>> Did not split it up into individual constants. The main 'problem' is that 
>>> Address and ExternalAddress are not compatible. 
>> 
>> There's a reason for that and it's because RIP-relative addressing doesn't 
>> always work, so additional register may be needed.
>> 
>>> Most instructions do not take AddressLiteral, so can't use ExternalAddress 
>>> to refer to those constants. 
>> 
>> I counted 4 instructions accessing the constants (`evpandq`, `andq`, 
>> `evporq`, and `vpternlogq`) in your patch. 
>> 
>> `macroAssembler_x86.hpp` is the place for `AddressLiteral`-related overloads 
>> (there are already numerous cases present) and it's trivial to add new ones. 
>> 
>>> (If I did get the instructions I use to take AddressLiteral, I think we 
>>> would end up with more lea(rscratch)s generated; but that's more of a 
>>> silver-lining)
>> 
>> It depends on memory layout. If constants end up placed close enough in the 
>> address space, there'll be no additional instructions generated.
>> 
>> Anyway, it doesn't look like something important from throughput 
>> perspective. Overall, I find it clearer when the code refers to individual 
>> constants through `AddressLiteral`s, but I'm also fine with it as it is now.
>
> Makes sense to me, that would indeed be cleaner, will add a couple more 
> overloads. (Still getting used to what is 'clean' in this code base).

done

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

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

Reply via email to