On Fri, 13 Oct 2023 14:48:51 GMT, Martin Doerr <mdo...@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
