RFR: 8294729: [s390] Implement nmethod entry barriers

[Lutz Schmidt]

On Fri, 7 Oct 2022 09:22:26 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

>> This draft PR implements native method barriers on s390. When complete, this will fix the build, and bring the other benefits of [JDK-8290025](https://bugs.openjdk.org/browse/JDK-8290025) to that platform.
>
> src/hotspot/cpu/s390/gc/shared/barrierSetAssembler_s390.cpp line 95:
> 
>> 93: 
>> 94:     // Conditional Jump
>> 95:     __ z_bcr(Assembler::bcondNotEqual, Z_R1_scratch); // 2 bytes
> 
> This is only a jump. We would need a call which sets Z_R14 = return address. It should be possible to set Z_R14 manually to the return address before the jump (z_lghrl). Alternatively, you could implement a stub like on x86_64.

I you really want to use a nerdy hack, I would suggest you use 
`z_larl(Z_R14, (instr_len((unsigned long)LARL_ZOPC) + instr_len((unsigned long)BCR_ZOPC)) / 2);`
`z_bcr(Assembler::bcondNotEqual, Z_R1_scratch);
`

> src/hotspot/cpu/s390/stubGenerator_s390.cpp line 2869:
> 
>> 2867:     // Save caller's sp & return_pc
>> 2868:     __ push_frame(frame::z_abi_16_size);
>> 2869:     __ z_stmg(Z_R14, Z_R15, _z_abi16(callers_sp), Z_SP);
> 
> Please use `save_return_pc()`. Z_R15 = Z_SP is already saved by push_frame.

You should not rely on Z_R14 being the return register in all circumstances and for all future. There are comments in the Principles of Operation and elsewhere that Z_R7 might assume a similar role. With save_return_pc() you are on the safe side and the code speaks for itself (no knowledge about Z_R14 required).

> src/hotspot/cpu/s390/stubGenerator_s390.cpp line 2875:
> 
>> 2873:     // We construct a pointer to the location of R14 stored above.
>> 2874:     __ z_xgr(Z_R2, Z_R2);
>> 2875:     __ z_ag(Z_R2, _z_abi(return_pc), 0, Z_SP);
> 
> Please use a better fitting instruction like z_la.

The code as written here would load the contents of the stack location reserved for the return pc. Less complicated: would load the return pc. If you want to load the address of the storage location, you would use 
`z_la (Z_R2, _z_abi(return_pc), Z_R0, Z_SP);`
Load Address in it's various forms just does the address calculation, it does not access storage.

And please, use Z_R0 (not just 0) to specify an optional, unspecified register - here and at all other places.

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

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