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

[Martin Doerr]

On Sat, 14 Oct 2023 18:01:00 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> I was thinking something 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..10b371457b3 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) {
>>          }
>>      }
>>  
>> +    // some ABIs have special handling for struct members
>> +    protected void checkStructMember(MemoryLayout member, long offset) {
>> +        checkLayoutRecursive(member);
>> +    }
>> +
>>      private void checkLayoutRecursive(MemoryLayout layout) {
>>          if (layout instanceof ValueLayout vl) {
>>              checkSupported(vl);
>> @@ -191,7 +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);
>> -                checkLayoutRecursive(member);
>> +                checkStructMember(member, offset);
>>  
>>                  offset += member.byteSize();
>>                  if (!(member instanceof PaddingLayout)) {
>> 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..f70af2bc025 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
>> @@ -36,19 +36,14 @@
>>  import java.lang.invoke.MethodHandle;
>>  import java.lang.invoke.MethodType;
>>  import java.nio.ByteOrder;
>> -import java.util.HashMap;
>>  import java.util.Map;
>>  
>>  public final class AixPPC64Linker extends AbstractLinker {
>>  
>> -    static final Map<String, MemoryLayout> CANONICAL_LAYOUTS;
>> +    static final Map<String, MemoryLayout> CANONICAL_LAYOUTS
>> +            = SharedUtils.canonicalLayouts(ValueLayout.JAVA_LONG, ValueLayout.JAVA_LONG, ValueLayout.JAVA_INT);
>>  
>> -    static {
>> -        HashMap<String, MemoryLayout> layouts = new HashMap<>();
>> -        layouts.putAll(SharedUtils.canonicalLayouts(ValueLayout.JAVA_LONG, ValueLayout.JAVA_LONG, ValueLayout.JAVA_INT));
>> -        layouts.put("do...
>
> Err, actually using `member.equals(C_DOUBLE)` in the above doesn't work since it still checks alignment. What you have with checking for `ValueLayout` and `carrier() == double.class` is better, but the byte order should also be checked at some point.

Please take a look at commit number 4. I think we need to support both, 4-byte and 8-byte aligned doubles in structures. IBM recommends to use "#pragma align (natural)": "The power suboption is the default to ensure compatibility with existing objects. If compatibility with earlier versions is not necessary, you should consider using natural alignment to improve potential application performance." https://www.ibm.com/docs/en/openxl-c-and-cpp-aix/17.1.1?topic=pragmas-pragma-align
We could also recommend to use "#pragma align (natural)" for the FFI (and possibly for tests). That would reduce incompatibility with other platforms. I can file a subtask for test adaptation if needed.

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

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