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

[Jorn Vernee]

On Wed, 22 Feb 2023 05:31:46 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

> Implementation of "Foreign Function & Memory API" for linux on Power (Little Endian) according to "Power Architecture 64-Bit ELF V2 ABI Specification".
> 
> This PR does not include code for VaList support because it's supposed to get removed by [JDK-8299736](https://bugs.openjdk.org/browse/JDK-8299736). I've kept the related tests disabled for this platform and throw an exception instead. Note that the ABI doesn't precisely specify variable argument lists. Instead, it refers to `<stdarg.h>` (2.2.4 Variable Argument Lists).
> 
> Big Endian support is implemented to some extend, but not complete. E.g. structs with size not divisible by 8 are not passed correctly (see `useABIv2` in CallArranger.java). Big Endian is excluded by selecting `ARCH.equals("ppc64le")` (CABI.java) only.
> 
> There's another limitation: This PR only accepts structures with size divisible by 4. (An `IllegalArgumentException` gets thrown otherwise.) I think arbitrary sizes are not usable on other platforms, either, because `SharedUtils.primitiveCarrierForSize` only accepts powers of 2.
> 
> The ABI has some tricky corner cases related to HFA (Homogeneous Float Aggregate). The same argument may need to get passed in both, a FP reg and a GP reg or stack slot (see "no partial DW rule"). This cases are not covered by the existing tests.
> 
> I had to make changes to shared code and code for other platforms:
> 1. Pass type information when creating `VMStorage` objects from `VMReg`. This is needed for the following reasons:
> - PPC64 ABI requires integer types to get extended to 64 bit (also see CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to know the type or at least the bit width for that.
> - Floating point load / store instructions need the correct width to select between the correct IEEE 754 formats. The register representation in single FP registers is always IEEE 754 double precision on PPC64.
> - Big Endian also needs usage of the precise size. Storing 8 Bytes and loading 4 Bytes yields different values than on Little Endian!
> 2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with byteSize() == 0) while running TestUpcallScope. Hence, existing size checks don't work (see MemorySegment.java). As a workaround, I'm just skipping the check in this particular case. Please check if this makes sense or if there's a better fix (possibly as separate RFE).

I will do a more thorough review soon.

Some preliminary comments:

> The ABI has some tricky corner cases related to HFA (Homogeneous Float Aggregate). The same argument may need to get passed in both, a FP reg and a GP reg or stack slot (see "no partial DW rule"). This cases are not covered by the existing tests.

FWIW, we have to do this for Windows vararg floats as well ([here](https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/internal/foreign/abi/x64/windows/CallArranger.java#L231-L239))

This can be done by `dup`-ing the value, and using 2 `vmStore`s. (each `vmStore` corresponding to a single register/stack location). Doing something similar might be simpler than the `INTEGER_AND_FLOAT` and `STACK_AND_FLOAT` storage types you're using right now. I'm not sure if that is related to the other limitations you mention? Might be interesting to look into. (perhaps as a separate RFE. I don't have a big issue since the current approach stays in PPC-only code)

> I had to make changes to shared code and code for other platforms:
> 
>     1. Pass type information when creating `VMStorage` objects from `VMReg`. This is needed for the following reasons:
> 
> 
>     * PPC64 ABI requires integer types to get extended to 64 bit (also see CCallingConventionRequiresIntsAsLongs in existing hotspot code). We need to know the type or at least the bit width for that.
> 
>     * Floating point load / store instructions need the correct width to select between the correct IEEE 754 formats. The register representation in single FP registers is always IEEE 754 double precision on PPC64.
> 
>     * Big Endian also needs usage of the precise size. Storing 8 Bytes and loading 4 Bytes yields different values than on Little Endian!

I think supplying the `BasicType` is fine. `VMReg` doesn't have any width information attached to it, and that's why a complementary `BasicType` is needed. I'm glad to see that you could make it work with the register masks for `VMStorage` :)

WRT the extension of int -> long. This could potentially also be handled in Java by adding the conversion as a `Cast` binding variant, and then adding the widening casts in `CallArranger`. (I'd be happy to implement the needed changes in shared code if you want, since it touches `BindingSpecializer` which is pretty dense). Since the extension seems to be a figment of the C ABI, that could be preferable, since it has the benefit of the VM code staying ABI-agnostic. This is potentially important if we want to add other ABIs in the future. But, we can also cross that bridge when we get to it (and there are probably more bridges to cross in that case too). So, up to you, really. (It's similar to the discussion surrounding floats for RISCV, if you followed that)

>     2. It happens that a `NativeMemorySegmentImpl` is used as a raw pointer (with byteSize() == 0) while running TestUpcallScope. Hence, existing size checks don't work (see MemorySegment.java). As a workaround, I'm just skipping the check in this particular case. Please check if this makes sense or if there's a better fix (possibly as separate RFE).

Zero-length memory segments are supposed to be resized before they are written to or read from (see [Zero-length memory segments](https://download.java.net/java/early_access/jdk20/docs/api/java.base/java/lang/foreign/MemorySegment.html#wrapping-addresses)). We shouldn't disable the check for them, as that would have far-reaching implications for the safety design of the memory access API.

Can you explain a bit more about where/why/how the issue occurs?

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

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