On Fri, 28 Jul 2023 03:59:26 GMT, sid8606 <d...@openjdk.org> wrote:

>> Implementation of "Foreign Function & Memory API" for s390x (Big Endian).
>
> sid8606 has updated the pull request incrementally with one additional commit 
> since the last revision:
> 
>   Preserve and restore register Z_R6
>   
>   Though Z_R6 is argument register it is a saved register
>   so preserve and restore Z_R6 register

src/hotspot/cpu/s390/upcallLinker_s390.cpp line 45:

> 43:     if (reg == Z_SP) continue;
> 44:     // though Z_R6 is argument register it is a saved register
> 45:     if (!abi.is_volatile_reg(reg) || reg == Z_R6) {

So, is the prior assumption that all argument registers are also volatile 
incorrect?

If so, I think it would be better to change `ABIDescriptor::is_volatile_reg` 
([1]) to only look at the `_integer_additional_volatile_registers` list (maybe 
rename it too), and then list all the volatile regs in  LinuxS390CallArranger 
explicitly, excluding R6 ([2]). That way all the information about which regs 
are volatile is in one place (LinuxS390CallArranger).

[1]: 
https://github.com/openjdk/jdk/pull/14801/files#diff-7096e1975de20baa3219d616506f26ba2b4500bf5ad28c331e3d6049a32a461eR39-R42
[2]: 
https://github.com/openjdk/jdk/pull/14801/files#diff-09da9016992b04bab73b6cc6aad8ca719e86c90c398af4e000583c1f8220a99bR73

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14801#discussion_r1278932461

Reply via email to