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

[Vladimir Kozlov]

Yes, this is much better.

Thanks,
Vladimir K

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
> -----------
>