RFR: 8291752: AArch64: Remove check_emit_size parameter from trampoline_call

[Evgeny Astigeevich]

On Fri, 5 Aug 2022 17:53:28 GMT, Vladimir Kozlov <kvn 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.
>
> 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.

The original code looks strange to me.
If we need a stub but we have failed to create it, we set dest anyway. This will lead to an incorrect instruction.
The behaviour is undefined. We might fail executing the instruction or jump somewhere.

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

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