some questions about vm code implement

[Jorn Vernee]

Hi Ziai,

(Sorry, I think I mistyped your name last time)

The principle of design of the VM code is that it should make no 
assumptions about the native ABI that is being used (though it can make 
assumptions about the Java ABI). How to link against an ABI should 
instead be entirely communicated from the Java side, through e.g. 
ABIDescriptor.

Since at the moment we don't pass information about which register parts 
should be saved, x86_64 conservatively saves the entire register if it 
is not volatile according to the ABI. This is perhaps a missed 
optimization opportunity. More information could be added to 
ABIDescriptor to describe which parts of a register should be saved.

The AArch64 code is less conservative, and only saves the lower 8 bytes 
of a FloatRegister. In practice this works out for the all the ABIs we 
target, which is currently only the platform's C ABI. But, this is 
essentially a hard-coded assumption, and should ideally be communicated 
through ABIDescriptor. (x86_64 also makes such an assumption about the 
mxcsr status register).

I think this area could be improved. I've filed: JDK-8293947 [1] to keep 
track of that.

For now, when adding support for new architectures I recommend 
conservatively saving the entire register if the ABIDescriptor says it 
is non-volatile (i.e. callee-saved).

HTH,
Jorn

[1]: https://bugs.openjdk.org/browse/JDK-8293947

On 16/09/2022 04:45, ziai li wrote:
> Hi all:
>
> In the process of reading the code, I have some doubts about the 
> implementation of vm code
> for example in cpu/aarch64/upcallLinker_aarch64.cpp
>
> // for callee saved regs, according to the caller's ABI
> static int compute_reg_save_area_size(const ABIDescriptor& abi) {
>
>   for (int i = 0; i < FloatRegisterImpl::number_of_registers; i++) {
>     FloatRegister reg = as_FloatRegister(i);
>     if (!abi.is_volatile_reg(reg)) {
>       // Only the lower 64 bits of vector registers need to be preserved.
>       size += 8; // bytes
>     }
>   }
> }
>
> static void preserve_callee_saved_registers(MacroAssembler* _masm, 
> const ABIDescriptor& abi, int reg_save_area_offset) {
>
>   for (int i = 0; i < FloatRegisterImpl::number_of_registers; i++) {
>     FloatRegister reg = as_FloatRegister(i);
>     if (!abi.is_volatile_reg(reg)) {
>       __ strd(reg, Address(sp, offset));
>       offset += 8;
>     }
>   }
> }
>
> static void restore_callee_saved_registers(MacroAssembler* _masm, 
> const ABIDescriptor& abi, int reg_save_area_offset) {
>
>   for (int i = 0; i < FloatRegisterImpl::number_of_registers; i++) {
>     FloatRegister reg = as_FloatRegister(i);
>     if (!abi.is_volatile_reg(reg)) {
>       __ ldrd(reg, Address(sp, offset));
>       offset += 8;
>     }
>   }
> }
>
> In the comments of the function compute_reg_save_area_size, only the 
> lower 64 bits of vector registers need to be preserved, so the size is 
> set to 8. Is this due to the calling convention of aarch64 or is it 
> because only floating point data needs to be saved？ I have the same 
> question about the restore_callee_saved_registers and 
> preserve_callee_saved_registers functions.
> The x86 code is implemented like this：
>
> static int compute_reg_save_area_size(const ABIDescriptor& abi) {
>
>   for (XMMRegister reg = as_XMMRegister(0); is_valid_XMM(reg); reg = 
> reg->successor()) {
>     if (!abi.is_volatile_reg(reg)) {
>       if (UseAVX >= 3) {
>         size += 64; // bytes
>       } else if (UseAVX >= 1) {
>         size += 32;
>       } else {
>         size += 16;
>       }
>     }
>   }
> }
>
> So I don't know what principle the value of size is set according to, 
> such as riscv, what should the value of size be, 8 or 16 or what?
>
> Looking forward to your reply
>
> ziai li