On Wed, 1 Mar 2023 06:37:45 GMT, Martin Doerr <mdo...@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
