RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v3]

[Martin Doerr]

On Tue, 28 Feb 2023 19:45:28 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> Martin Doerr has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Remove size restriction for structs. Add TODO for Big Endian.
>
> src/hotspot/cpu/ppc/downcallLinker_ppc.cpp line 133:
> 
>> 131:   Register callerSP            = R2, // C/C++ uses R2 as TOC, but we can reuse it here
>> 132:            tmp                 = R11_scratch1, // same as shuffle_reg
>> 133:            call_target_address = R12_scratch2; // same as _abi._scratch2 (ABIv2 requires this reg!)
> 
> Do I understand correctly that the ABI requires the register to be used for the call to be `R12`? How does that make a difference? I guess in some cases the callee might want to know the address through which it is called? (so it looks at `R12`)

ABI v2 requires R12 to point to the function entry point. It is used to access constants relative to it.

> src/hotspot/cpu/ppc/downcallLinker_ppc.cpp line 154:
> 
>> 152:   // (abi_reg_args is abi_minframe plus space for 8 argument register spill slots)
>> 153:   assert(_abi._shadow_space_bytes == frame::abi_minframe_size, "expected space according to ABI");
>> 154:   int allocated_frame_size = frame::abi_minframe_size + MAX2(_input_registers.length(), 8) * BytesPerWord;
> 
> This is hard-coding an assumption about the ABI that's being called. Ok for now.
> 
> If it needs to be addressed in the future, it could be done by adding another field to `ABIDescriptor` like `min_stack_arg_bytes`, or something like that (which is set to zero for other ABIs). It seems to be different from `shadow_space` since it's also used by the caller to put stack arguments.

Yeah, I think this should be done on demand.

> src/hotspot/cpu/ppc/foreignGlobals_ppc.cpp line 229:
> 
>> 227: 
>> 228: void ArgumentShuffle::pd_generate(MacroAssembler* masm, VMStorage tmp, int in_stk_bias, int out_stk_bias, const StubLocations& locs) const {
>> 229:   Register callerSP = as_Register(tmp); // preset
> 
> It looks like `tmp` is being used to hold the caller's SP. I'm guessing this can not be computed the same way as we do on x86 and aarch64? (based on `RBP`, `RFP_BIAS`)
> 
> If you want, you could add another register argument to `pd_generate` that is just invalid/unused on other platforms. That way you could use `tmp` for the shuffling instead of having to go through the stack. (looks like `R0` is already used in some cases as a temp register)

There's no BP register. It could get loaded from the back chain, but would still need a register. R0 is always available as scratch reg, so there's no need to pass it. (Note that I'm not going through stack because of the lack of registers.)
Yeah, maybe passing callerSP separately would make it better readable on PPC64. Not sure if it's worth changing.

> src/hotspot/cpu/ppc/upcallLinker_ppc.cpp line 137:
> 
>> 135:   ArgumentShuffle arg_shuffle(in_sig_bt, total_in_args, out_sig_bt, total_out_args, &in_conv, &out_conv, shuffle_reg);
>> 136:   // The Java call uses the JIT ABI, but we also call C.
>> 137:   int out_arg_area = MAX2(frame::jit_out_preserve_size + arg_shuffle.out_arg_bytes(), (int)frame::abi_reg_args_size);
> 
> We need `frame::abi_reg_args_size` since we call `on_entry`/`on_exit` which require the stack space I guess?

Correct. C functions are allowed to write into the space which is part of the caller's frame.

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

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