RFR: 8280872: Reorder code cache segments to improve code density [v2]

[Evgeny Astigeevich]

On Tue, 1 Mar 2022 16:43:32 GMT, Boris Ulasevich <bulasevich at openjdk.org> wrote:

>> src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 393:
>> 
>>> 391:   assert(CodeCache::find_blob(entry.target()) != NULL,
>>> 392:          "destination of far call not found in code cache");
>>> 393:   assert(CodeCache::is_non_nmethod(entry.target()), "must be a call to the code stub");
>> 
>> This restricts far calls to be calls of non-nmethod code.
>
> Yes. In fact the function is used for non-method code calls only. I put an assert here to be check this fact for future code updates.

I don't understand why we should restrict uses of `far_call` to calls of non-nmethod code. Could you please explain this?

>> src/hotspot/cpu/aarch64/nativeInst_aarch64.cpp line 533:
>> 
>>> 531:   address stub = NULL;
>>> 532: 
>>> 533:   if (a.codecache_branch_needs_far_jump()
>> 
>> I prefer it to be `a.target_needs_far_jump(dest)`. `codecache_branch` looks like code cache  branches need far jumps. It is strange because the code cache is just a storage. It is the code generator has to use far jumps.
>
> With this patch I do not change trampoline calls. I change far_jump and far_call procedures only.
> Instead of far_branches() function we have two functions:
> - codecache_branch_needs_far_jump to find if we need a far jump for intra-codecache branches
> - codestub_branch_needs_far_jump to find if we need a far branch for codecache-to-nonmethodEntrypoint branch
> So in this place I leave codecache_branch_needs_far_jump as exact equivalent of former far_branches() call.

I understand the changes. My comment is about names. `MacroAssembler` only needs to know if it needs a far jump. Details "why" are not needed here.
We ask `MacroAssembler`. `MacroAssembler` gets `CodeCache` configuration info and checks whether a far jump is needed.

>> src/hotspot/share/code/codeCache.cpp line 898:
>> 
>>> 896: }
>>> 897: 
>>> 898: size_t CodeCache::max_distance_to_codestub() {
>> 
>> `max_distance_to_non_nmethod_heap`?
>> As this is public API, it sounds strange without the start point.
>> If someone changes positions of the heap, would it work as expected?
>
>> max_distance_to_non_nmethod_heap?
>> As this is public API, it sounds strange without the start point.
> 
> Start point is any point in the CodeCache. Will the comment below help?
> // maximum distance from any point in the CodeCache to any entry point in the non_nmethod CodeCache segment
> This is really too many words for a self-explanatory function name.
> 
>> If someone changes positions of the heap, would it work as expected?
> 
> Sure

Can we moved the code to `codestub_branch_needs_far_jump`? It is the only place where the code is used.
We might need to make either `get_code_heap` public or `MacroAssembler` a friend of `CodeCache`.

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

PR: https://git.openjdk.java.net/jdk/pull/7517