On Tue, 23 May 2023 15:21:02 GMT, Martin Doerr <mdo...@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. Update: 
>> Resolved after merging of 
>> [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>> 
>> 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 separat...
>
> Martin Doerr has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Improve comments about the Parameter Save Area.

Thanks a lot for all discussions, feedback and the reviews! This was really 
helpful! I'm planning to integrate tomorrow.

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

PR Comment: https://git.openjdk.org/jdk/pull/12708#issuecomment-1560069523

Reply via email to