On Thu, 19 Oct 2023 13:37:09 GMT, Lutz Schmidt <l...@openjdk.org> wrote:
>> Jorn Vernee has updated the pull request incrementally with one additional >> commit since the last revision: >> >> add s390 support > > src/hotspot/cpu/s390/downcallLinker_s390.cpp line 100: > >> 98: Address offset_addr(callerSP, FP_BIAS + reg_offset.offset()); >> 99: __ mem2reg_opt(r_tmp1, offset_addr, true); >> 100: __ z_agr(reg_oop_reg, r_tmp1); > > Please note that s390 is a CISC architecture. It provides instructions for > almost everything. :-) > Here, I would suggest to add the offset to reg_oop_reg directly from memory - > without first loading the offset into a temp register (that is RISC style). > It's shorter and faster: > ` __ z_ag(reg_oop_reg, offset_addr);` That's right @RealLucy. Thanks for reviewing. > src/hotspot/cpu/s390/downcallLinker_s390.cpp line 112: > >> 110: __ mem2reg_opt(r_tmp2, oop_addr, true); >> 111: __ z_agr(r_tmp1, r_tmp2); >> 112: __ reg2mem_opt(r_tmp1, oop_addr, true); > > Similar to above. You need to load only one operand into a register. > > __ mem2reg_opt(r_tmp2, oop_addr, true); > __ z_ag(r_tmp2, offset_addr); > __ reg2mem_opt(r_tmp2, oop_addr, true); Fixed. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16201#discussion_r1365916526 PR Review Comment: https://git.openjdk.org/jdk/pull/16201#discussion_r1365916660
