On Fri, 13 Oct 2023 10:24:58 GMT, Jorn Vernee <jver...@openjdk.org> wrote:
>> Martin Doerr has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Improve micro benchmarks and a comment.
>
> src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/aix/AixPPC64Linker.java
> line 48:
>
>> 46: static {
>> 47: HashMap<String, MemoryLayout> layouts = new HashMap<>();
>> 48:
>> layouts.putAll(SharedUtils.canonicalLayouts(ValueLayout.JAVA_LONG,
>> ValueLayout.JAVA_LONG, ValueLayout.JAVA_INT));
>
> You may also add an extra parameter for the double layout to
> SharedUtils::canonicalLayouts.
>
> Also, what about `jdouble`? It seems to be defined to a native `double` on
> AIX as well? (see src/java.base/share/native/include/jni.h)
Not sure how `jdouble` would be used. JNI doesn't support C structures and the
double alignment is only an issue in structures. Do we support embedding
`jdouble` in structures? I guess changing it would probably be better?
Note that we will need something which maps to the 8-Byte aligned `double`.
Otherwise we get an Exception when passing a `JAVA_DOUBLE` as normal argument:
IllegalArgumentException: Unsupported layout: D8
at
java.base/jdk.internal.foreign.abi.AbstractLinker.checkSupported(AbstractLinker.java:244)
at
java.base/jdk.internal.foreign.abi.AbstractLinker.checkLayoutRecursive(AbstractLinker.java:185)
at
java.base/jdk.internal.foreign.abi.AbstractLinker.checkLayout(AbstractLinker.java:179)
at java.base/java.lang.Iterable.forEach(Iterable.java:75)
at
java.base/jdk.internal.foreign.abi.AbstractLinker.checkLayouts(AbstractLinker.java:171)
at
java.base/jdk.internal.foreign.abi.AbstractLinker.downcallHandle0(AbstractLinker.java:98)
at
java.base/jdk.internal.foreign.abi.AbstractLinker.downcallHandle(AbstractLinker.java:85)
at TestDowncall.<clinit>(TestDowncall.java:127)
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16179#discussion_r1358201012
