RFR: 8311630: [s390] Implementation of Foreign Function & Memory API (Preview) [v2]

[Jorn Vernee]

On Fri, 7 Jul 2023 14:54:54 GMT, sid8606 <duke at openjdk.org> wrote:

>> src/hotspot/cpu/s390/upcallLinker_s390.cpp line 141:
>> 
>>> 139: 
>>> 140:   // The Java call uses the JIT ABI, but we also call C.
>>> 141:   int out_arg_area = MAX2(frame::z_jit_out_preserve_size + arg_shuffle.out_arg_bytes(), (int)frame::z_abi_160_size);
>> 
>> What do you mean here with "but we also call C"? Upcall stubs are always calling into Java, though the source ABI is unknown.
>
> We do native calls in this stub, so make sure allocated stack frame size is big enough.

Ah, right thanks. That's good.

>> test/jdk/java/foreign/TestClassLoaderFindNative.java line 63:
>> 
>>> 61:     public void testVariableSymbolLookup() {
>>> 62:         MemorySegment segment = SymbolLookup.loaderLookup().find("c").get().reinterpret(ByteOrder.nativeOrder() == ByteOrder.LITTLE_ENDIAN ? 1 : 4);
>>> 63:         assertEquals(segment.get(JAVA_BYTE, ByteOrder.nativeOrder() == ByteOrder.LITTLE_ENDIAN ? 0 : 3), 42);
>> 
>> Could you explain why this is needed? It looks like the lookup is returning the wrong address?
>
> Since  s390x runs in Big Endian mode. We get LSB on higher address of integer size.

Ah, wait, now I see. The native side uses `int` as a type, but we try to load it as a `JAVA_BYTE`. I think this is a bug in the test. The Java side should use `JAVA_INT` instead, and the size passed to `reinterpret` should be `4` (which matches the native type). What happens if you make that change instead? Does the test pass then?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14801#discussion_r1256224573
PR Review Comment: https://git.openjdk.org/jdk/pull/14801#discussion_r1256219721