RFR: 8303040: linux PPC64le: Implementation of Foreign Function & Memory API (Preview) [v3]
Jorn Vernee
jvernee at openjdk.org
Mon Mar 6 16:42:10 UTC 2023
On Wed, 1 Mar 2023 06:37:45 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:
>>> * Uploaded my simple reproducer to [JDK-8303017](https://bugs.openjdk.org/browse/JDK-8303017)
>>
>> Thanks!
>>
>>> * Using oversized load / stores is problematic. Don't forget that OpenJDK still supports Big Endian platforms (AIX, s390x).
>>
>> You're right. I realized that it's also problematic for heap segments, for which we can't do oversized accesses. I am working on another solution that splits up the loads/stores into power-of-two sized chunks: https://github.com/openjdk/panama-foreign/compare/foreign-memaccess+abi...JornVernee:panama-foreign:OOB That patch is just a POC at this point though. Also, I don't think it works for BE at the moment (need to flip the offset for BE, I think. Just like we do in Unsafe).
>>
>>> * The result of `NativeCallingConvention::calling_convention` is interpreted as size, but it returns the max offset. That's off by one slot. Should I file a bug for that? (PPC64 is not affected because it doesn't use the result.)
>>
>> I'm not sure there's an issue there. Note that the 'max offset' is computed as `reg.offset() + reg.stack_size()`, so that should get us the size we need to allocate for the stack arguments. (e.g. 2 ints being passed at offset 0 and 4, would make max offset 4 + 4 = 8, which gives the size needed for the 2 ints). Computing the max offset instead of just summing the sizes of the stack arguments is needed since stack arguments can be sparsely placed in some cases on Mac/AArch64.
>>
>>> * Since the membar on the return path was mentioned: I think it would be good to enable UseSystemMemoryBarrier by default on operating systems which support it. Maybe we should discuss this with @robehn.
>>
>> ~I don't think we've done that much testing with UseSystemMemoryBarrier since it was added~. I'm a bit nervous about turning it on by default since it's currently also used for JNI. Let's see what Robbin thinks.
>
> @JornVernee: Thanks a lot for your detailed review! I have quite a few TODOs which include:
> - Include my tests for the HFA corner cases.
> - Try to improve handling of the overlapping registers as you suggested.
> - Check nesting of HFA.
>
> There will surely be more when looking into Big Endian support after merging with your recent work on https://github.com/openjdk/panama-foreign/compare/foreign-memaccess+abi...JornVernee:panama-foreign:OOB
> We should get rid of oversized accesses on PPC64, too.
> Thanks for sharing your plans to intrisify `linkToNative` in C2 later. I guess we should do more preparation work on all platforms when that gets addressed.
@TheRealMDoerr I've moved the support for structs/unions that are not a power of 2 in size to this repo, so you should be able to merge the master branch to get it now.
-------------
PR: https://git.openjdk.org/jdk/pull/12708
More information about the core-libs-dev
mailing list