RFR: 8313419: Template interpreter produces no safepoint check for return bytecodes

[Fredrik Bredberg]

On Thu, 17 Aug 2023 16:57:17 GMT, Patricio Chilano Mateo <pchilanomate at openjdk.org> wrote:

>> The template interpreter produces a safepoint check for return bytecodes (TemplateTable::_return(TosState state)) on x86, ppc64le and s390, but not on aarch64, arm32, and riscv64.
>> 
>> This PR adds the missing  safepoint check to  aarch64, arm32, and riscv64.
>> 
>> Tested tier1-tier7 on aarch64. Both arm32, and riscv64 was sanity tested using Qemu.
>
> src/hotspot/cpu/aarch64/templateTable_aarch64.cpp line 2206:
> 
>> 2204:     __ push(state);
>> 2205:     __ push_cont_fastpath(rthread);
>> 2206:     __ call_VM(noreg, CAST_FROM_FN_PTR(address, InterpreterRuntime::at_safepoint));
> 
> Looking at the code generated for the existing safepoint poll (`TemplateInterpreterGenerator::generate_safept_entry_for()`) I see we add a full memory barrier after the return from `InterpreterRuntime::at_safepoint()`. That would call for adding it here too although I don't see why we need that. The SafepointMechanism logic already executes the proper barriers after we process pending operations. Same thing for riscv.

That's an interesting find. I had a discussion with Erik (@fisk), and as you he didn't see any reason why it should be needed. He also very much doubted the need for the membar in `TemplateInterpreterGenerator::generate_safept_entry_for()`.

In my view x86 is quite forgiving if you forget to add a membar, but PowerPC tend not to be. Since there's no membar in `generate_safept_entry_for()` on PowerPC and it still works ok, it does seem like it's not needed.

So, for this reason I will not add any additional membar.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15248#discussion_r1298378973