[External] : Re: premain: Possible solutions to use runtime G1 region grain size in AOT code (JDK-8335440)

[Andrew Dinn]

On 20/08/2024 18:35, Vladimir Kozlov wrote:
> Yes, this is much better.

Ok, thanks. I'll push this into the premain branch.

regards,


Andrew Dinn
-----------
Red Hat Distinguished Engineer
Red Hat UK Ltd
Registered in England and Wales under Company Registration No. 03798903
Directors: Michael Cunningham, Michael ("Mike") O'Neill

> On 8/20/24 7:11 AM, Andrew Dinn wrote:
>> On 15/08/2024 23:30, Vladimir Kozlov wrote:
>>
>>>> Why you still use base+offsets for constants address? Is it for 
>>>> Aarch64?
>>>>
>>>> What I want is something like this:
>>>>
>>>> static address grain_shift_address() { return 
>>>> &_aot_runtime_constants::_grain_shift; }
>>
>> Hmm, I thought I had made the changes to use a direct address but they 
>> were not in the aarch64 version I pushed. I have pushed a new version 
>> that fixes the macroassembler and g1 stubs to load the field address 
>> as a constant. I removed the xxx_offset() methods from 
>> AOTRuntimeConstants and replaced them with xxx_address() methods to 
>> make sure it can only be done that way.
>>
>>>> And you will not need `#if INCLUDE_CDS` if you put 
>>>> AOTRuntimeConstants into SCCache.cpp.
>>
>> Yes, SCCache.hpp/cpp is a much better place for this class than 
>> stubRoutines.hpp/cpp. The need to generate AOT code variants that 
>> employ constants from the current runtime is not specific to 
>> stubGenerator stubs. However, it always relates to use of the AOT cache.
>>
>>>> I don't see the need to separate create_field_address_list() code 
>>>> from field_addresses_list().
>>
>> Sorry, this was a hangover from an initial implementation. The getter 
>> now returns the array from a static field initialized in SCCache.cpp.
>>
>>>> Add assert to field_addresses_list() to check that values are 
>>>> initialized.
>>>
>>> Actually you don't need assert since we need only addresses of fields.
>>
>> Left as is.
>>
>>> I don't see use of aot_runtime_constants_at() anymore.
>>
>> It is redundant and has been removed.
>>
>>> Also why you need aot_runtime_constants()? Why not have 
>>> _aot_runtime_constants as static field of AOTRuntimeConstants class 
>>> which you can initialize in (again) SCCache.cpp?
>> Agreed. The static singleton is now only used internally to the class. 
>> So, we no longer need aot_runtime_constants() any more.
>>
>> Latest code:
>>
>>
>> https://urldefense.com/v3/__https://github.com/adinn/leyden/compare/premain...adinn:leyden:JDK-8335440-load-via-codecache?expand=1__;!!ACWV5N9M2RV99hQ!JvSVWm0OpYY_RBUQMwmBhuNFku_mjcqP_anYtWrigTz-Tp1SNf2agHStFtr6FqGYCUNX3LKSRIG3IeE90aw$
>> This passes tier1 tests on aarch64 and successfully improves 
>> performance for the javac benchmark.
>>
>> regards,
>>
>>
>> Andrew Dinn
>> -----------
>>
>