RFR: 8317545: AIX PPC64: Implementation of Foreign Function & Memory API [v2]

[Martin Doerr]

On Fri, 13 Oct 2023 14:48:51 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:

>> Discussed this a bit with Maurizio. There are a few things to consider:
>> 
>> - `cannonicalLayouts` allows for just a single mapping from type name to layout. I think `double` should map to the 8-byte aligned layout. (the same alignment you get from `_Alignof(double)` in C).
>> - We have to relax the checking done by the linker on AIX. Maybe change `AbstractLinker::checkLayoutRecursive` call some `checkStructMemberLayout` method, that by default just calls `checkLayoutRecursive`, but which the AIX linker can override to implement its special rules for doubles.
>> - We need a small spec update to allow for struct field layouts that are not canonical layouts. I'll take a stab at that, and then get back to you.
>
> Thanks for your outstanding support! I'm awaiting feedback from IBM and a solution for the subtask https://bugs.openjdk.org/browse/JDK-8317799. There shouldn't be much work to be done after that, I guess.

So, your proposal should look like this?

diff --git a/src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java b/src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java
index dbd9a3f67a4..ec1639938e8 100644
--- a/src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java
+++ b/src/java.base/share/classes/jdk/internal/foreign/abi/AbstractLinker.java
@@ -180,6 +180,11 @@ private void checkLayout(MemoryLayout layout) {
         }
     }
 
+    // Platforms can override.
+    protected MemoryLayout processGroupLayoutMember(MemoryLayout member, long offset) {
+        return member;
+    }
+
     private void checkLayoutRecursive(MemoryLayout layout) {
         if (layout instanceof ValueLayout vl) {
             checkSupported(vl);
@@ -191,6 +196,7 @@ private void checkLayoutRecursive(MemoryLayout layout) {
                 // check element offset before recursing so that an error points at the
                 // outermost layout first
                 checkMemberOffset(sl, member, lastUnpaddedOffset, offset);
+                member = processGroupLayoutMember(member, offset);
                 checkLayoutRecursive(member);
 
                 offset += member.byteSize();
diff --git a/src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/aix/AixPPC64Linker.java b/src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/aix/AixPPC64Linker.java
index c24d2553ec0..ddd4049c814 100644
--- a/src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/aix/AixPPC64Linker.java
+++ b/src/java.base/share/classes/jdk/internal/foreign/abi/ppc64/aix/AixPPC64Linker.java
@@ -46,7 +46,7 @@ public final class AixPPC64Linker extends AbstractLinker {
     static {
         HashMap<String, MemoryLayout> layouts = new HashMap<>();
         layouts.putAll(SharedUtils.canonicalLayouts(ValueLayout.JAVA_LONG, ValueLayout.JAVA_LONG, ValueLayout.JAVA_INT));
-        layouts.put("double", ValueLayout.JAVA_DOUBLE.withByteAlignment(4));
+        layouts.put("double4bytealigned", ValueLayout.JAVA_DOUBLE.withByteAlignment(4));
         CANONICAL_LAYOUTS = Map.copyOf(layouts);
     }
 
@@ -81,4 +81,13 @@ protected ByteOrder linkerByteOrder() {
     public Map<String, MemoryLayout> canonicalLayouts() {
         return CANONICAL_LAYOUTS;
     }
+
+    @Override
+    protected MemoryLayout processGroupLayoutMember(MemoryLayout member, long offset) {
+        // Change alignment of double members to 4 except at the beginning.
+        if ((offset > 0) && (member instanceof ValueLayout vl && vl.carrier() == double.class)) {
+            return ValueLayout.JAVA_DOUBLE.withByteAlignment(4);
+        }
+        return member;
+    }
 }

That will require AIX specific changes in the tests which use double values in structures. Should we use the `double4bytealigned` only on AIX in these tests or what do you suggest?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16179#discussion_r1359400910