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

[Martin Doerr]

On Tue, 28 Feb 2023 20:00:15 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/upcallLinker_ppc.cpp line 240:
> 
>> 238:   __ ld(call_target_address, in_bytes(Method::from_compiled_offset()), R19_method);
>> 239:   __ mtctr(call_target_address);
>> 240:   __ bctrl();
> 
> Ok, I see. I guess there is some special purpose register called `CTR` which we are moving to for `bctrl` here. Does ABIv2 require that move to always come from `R12`? (from the comment in downcallLinker).
> 
> (I'm trying to understand the requirements for possibly tweaking shared code).

There's no instruction which can use a GP reg as branch target directly. That's why we use CTR.
In this case, using R12 is not required since we call Java and our PPC64 VM code does not rely on it.
If we were calling C, using R12 would be required by ABI v2.

> src/hotspot/cpu/ppc/upcallLinker_ppc.cpp line 347:
> 
>> 345:   FunctionDescriptor* fd = (FunctionDescriptor*)fd_addr;
>> 346:   fd->set_entry(fd_addr + sizeof(FunctionDescriptor));
>> 347: #endif
> 
> Had to do a double take. Looks like we're not the only one who are using the name `FunctionDescriptor` :)

Yeah, ABI v1 (Big Endian) treats function pointers as pointers to a structure called `FunctionDescriptor` which contains the entry point plus additional information like a pointer to a constant table.

> src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/CallArranger.java line 286:
> 
>> 284:                 // "no partial DW rule": Mark first stack slot to get filled.
>> 285:                 // Note: Can only happen with forArguments = true.
>> 286:                 VMStorage overlappingReg = null;
> 
> `overlappingReg` is initialized along all branches, so it's not needed to assign `null` here (and then javac will check it is actually assigned before use)

Removed.

> src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/TypeClass.java line 66:
> 
>> 64:     }
>> 65: 
>> 66:     static boolean isHomogeneousFloatAggregate(MemoryLayout type, boolean useABIv2) {
> 
> Note that we had to make some changes to this routine on AArch64, since it didn't properly account for nested structs/unions and arrays. See: https://github.com/openjdk/panama-foreign/pull/780
> 
> Just as a heads up, in case PPC needs changes too.

Thanks for the hint! I just added the code since it is known to be needed, small and doesn't break any of the existing tests. I'll need to test and possibly debug the various cases, yet.

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

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