RFR: 8294729: [s390] Implement nmethod entry barriers

Tyler Steele tsteele at openjdk.org
Wed Oct 26 20:00:40 UTC 2022


On Fri, 7 Oct 2022 10:11:46 GMT, Lutz Schmidt <lucy at openjdk.org> wrote:

>> 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).

Makes sense. I've made this change.

>> 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.

Thanks for the feedback. I agree this is better, and have made this change as well.

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

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


More information about the hotspot-dev mailing list