RFR: 8299162: Refactor shared trampoline emission logic [v3]

[Andrew Dinn]

On Mon, 6 Feb 2023 04:42:58 GMT, Xiaolin Zheng <xlinzheng at openjdk.org> wrote:

>> src/hotspot/cpu/aarch64/codeBuffer_aarch64.cpp line 71:
>> 
>>> 69: 
>>> 70:   assert(requests->number_of_entries() >= 1, "at least one");
>>> 71:   const int total_requested_size = MacroAssembler::trampoline_stub_size() * requests->number_of_entries();
>> 
>> There's a detail here that is worth mentioning. At first I was concerned that this calculation might overestimate the required size by one 32 bit word -- that happens where we are able generate the first stub without the need for any alignment padding. However, that won't cause any unnecessary failure because the rounding up will be from an odd to an even multiple of 32 bit words i.e. adding this extra half-word will not push the request over to consume an extra 64 bit word.
>
> Thanks for your review, Andrew - Yes, indeed a detail here. Although the real emitted code does not get affected, when allocating the requested trampolines, the pre-allocated size the code buffer is applying for could be slightly bigger than the previous real-time processing. I am not sure how useful it is to calculate the precise allocated request size, (though doable, seems we will lose the code simplicity) for in the context the current CodeBuffer is only a temporary one, which would be finally poured into the real nmethod; and also when the code buffer is expanding[1], a quite big and heuristic size could be applied for. I dumped the `requests->number_of_entries()` here, the trampoline count: seems the value is a small number, often less than `5`, sometimes `10`, if the `ReservedCodeCacheSize > branch_range` condition is true so that trampolines are generated.
> 
> I realized maybe I should rename the `trampoline_stub_size` to something like `max_trampoline_stub_size` to reflect its semantics. I was wondering if the final version still looks okay to you?
> 
> [1] https://github.com/openjdk/jdk/blob/b4cb6c8e8b4bb10d47fd4839c7abf13a552323f6/src/hotspot/share/asm/codeBuffer.cpp#L833-L845

@zhengxiaolinX
> I realized maybe I should rename the `trampoline_stub_size` to something like `max_trampoline_stub_size`to reflect its semantics. I was wondering if the final version still looks okay to you?

There is no need for any name change but it does no harm. My comment was only to record a detail on the PR that might help anyone who looks at this patch in future.

I agree there is no reason to complicate the size estimation code given that it is unlikely to save a significant amount of space.

So, yes, this version looks good. Ship it!

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

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