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

[Martin Doerr]

On Tue, 16 May 2023 14:44:25 GMT, Richard Reingruber <rrich at openjdk.org> wrote:

>> Martin Doerr has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add NONZERO check for downcall_stub_address_offset_in_bytes().
>
> src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/CallArranger.java line 28:
> 
>> 26: package jdk.internal.foreign.abi.ppc64;
>> 27: 
>> 28: import java.lang.foreign.AddressLayout;
> 
> Imports are not grouped and ordered alphabetically.
> (Very much as the aarch64 version)

Done.

> src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/CallArranger.java line 73:
> 
>> 71:     public static final int MAX_FLOAT_REGISTER_ARGUMENTS = 13;
>> 72: 
>> 73:     // This is derived from the 64-Bit ELF V2 ABI spec, restricted to what's
> 
> The comment says ABI V2 but the code seems to handle V1 too.

It's derived from ABI v2, but v1 is compatible. Added that to the comment.

> src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/CallArranger.java line 158:
> 
>> 156:     class StorageCalculator {
>> 157:         private final boolean forArguments;
>> 158:         private boolean forVarArgs = false;
> 
> Seems to be not used.

I had kept it in case another PPC64 OS would need it, but I guess it's unlikely. So, I just removed it. Could get added back easily if needed.

> src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/CallArranger.java line 221:
> 
>> 219:             // !useABIv2() && layout.byteSize() > 8 && layout.byteSize() % 8 != 0
>> 220: 
>> 221:             // Allocate individual fields as gp slots (regs and stack).
> 
> You explained to me, it's not individual (struct) fields that are handled here. Looks like registers and 8 byte stack slots are allocated to completely cover the struct. Would be good if you could change the comment and names in the code to better reflect this.

Adapted to aarch64 implementation (using `MAX_COPY_SIZE`).

> src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/linux/LinuxPPC64leLinker.java line 41:
> 
>> 39: 
>> 40:     public static LinuxPPC64leLinker getInstance() {
>> 41:         if (instance == null) {
> 
> Other platforms optimized this to return a constant (probably after you forked off the port).

Good catch. Adapted.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1199271594
PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1199271941
PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1199272912
PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1199273422
PR Review Comment: https://git.openjdk.org/jdk/pull/12708#discussion_r1199273918