RFR: 8291752: AArch64: Remove check_emit_size parameter from trampoline_call

[Vladimir Kozlov]

On Fri, 5 Aug 2022 16:59:08 GMT, Evgeny Astigeevich <duke at openjdk.org> wrote:

> `MacroAssembler::trampoline_call` can skip generation of a trampoline stub if C2 phase output is in the checking emitted code size mode. If it is not C2 compilation, `trampoline_call` always generates a trampoline stub. 
> 
> `gen_continuation_enter` as a part of C2 compilation needed to disable the functionality above. Now `trampoline_call` has the `check_emit_size` which allows to disable the functionality completely. Its default value is `true` which means the functionality is enabled.
> 
> There are problems:
> - C2 specific code which must be properly guarded if it is not C2 compilation. The code has the risk of undefined behaviour (UB). It is done with `is_c2_compile`. [JDK-8291654](https://bugs.openjdk.org/browse/JDK-8291654) is an example of changes caused UB.
> - The default value of `check_emit_size`. The current value `true` allows C1 to access the C2 specific code. Setting the default value to `false` will require to review and to update every existing use of `trampoline_call`. It will complicate uses of `trampoline_call`: each time a decision needs to be done - use or not use the default.
> 
> Summary of changes:
> - Have a virtual `emit_trampoline_stub` which is privately overridden in `C2_MacroAssembler`. The overridden  version does not emit a stub if C2 phase output is in the checking emitted code size mode.
> - `emit_trampoline_stub` returns `bool` instead of `address` because `address` is only used to check the success of a call.
> - `check_emit_size` is removed because it is not needed any more.
> - `NativeCall::trampoline_jump`:
>     - Changed to return `void` because the result is not used. It was returning the result of `emit_trampoline_stub`.
>     - Assert `emit_trampoline_stub`.
> 
> Tested with fastdebug/release builds:
> - `gtest`: Passed.
> - `tier1`...`tier4`: Passed.

I have few comments.

src/hotspot/cpu/aarch64/c2_MacroAssembler_aarch64.cpp line 1661:

> 1659: bool C2_MacroAssembler::emit_trampoline_stub(int insts_call_instruction_offset, address target) {
> 1660:   if (ciEnv::current()->task() != NULL) {
> 1661:     assert(is_c2_compile(ciEnv::current()->task()->comp_level()), "not C2 compilation task");

You don't need this assert. `C2_MacroAssembler` is used only in `.ad` files used by C2 only.

src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp line 864:

> 862:   // We might need a trampoline if branches are far.
> 863:   bool need_trampoline = far_branches();
> 864:   if (!need_trampoline && entry.rspec().type() == relocInfo::runtime_call_type && !CodeCache::contains(entry.target())) {

In `CodeCache::contains(entry.target())` you can use `target` value.

src/hotspot/cpu/aarch64/nativeInst_aarch64.cpp line 541:

> 539:   // If we generated no stub, patch this call directly to dest.
> 540:   // This will happen if we don't need far branches or if there
> 541:   // already was a trampoline.

I think this comment should be changed since you changed the code.

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

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