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

[sid8606]

On Fri, 7 Jul 2023 12:06:39 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

>> sid8606 has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Address Amit's review comments
>
> src/hotspot/cpu/s390/foreignGlobals_s390.cpp line 186:
> 
>> 184:     case StorageType::FRAME_DATA: {
>> 185:       switch (from_reg.stack_size()) {
>> 186:         case 8: __ mem2reg_opt(Z_R0_scratch, Address (callerSP, reg2offset(from_reg, in_stk_bias)), true); break;
> 
> A potential issue here is that Z_R0_scratch could be used by the target ABI, that's why the shuffle register is passed as an argument on other platforms. (It also makes it clearer in the calling code that that register is used somehow).

Now using Z_R11 frame pointer and passing shuffle register to use as temp.

> src/hotspot/cpu/s390/upcallLinker_s390.cpp line 172:
> 
>> 170:   //      |---------------------| = frame_bottom_offset = frame_size
>> 171:   //      | (optional)          |
>> 172:   //      | ret_buf             |
> 
> There's no return buffer, so this can be removed.

Fixed

> src/hotspot/cpu/s390/upcallLinker_s390.cpp line 232:
> 
>> 230: 
>> 231:   // return value shuffle
>> 232:   if (!needs_return_buffer) {
> 
> I suggest using an assert here instead.

Added an assert, thanks

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14801#discussion_r1257230962
PR Review Comment: https://git.openjdk.org/jdk/pull/14801#discussion_r1257231066
PR Review Comment: https://git.openjdk.org/jdk/pull/14801#discussion_r1257231195