RFR: 8313438: [s390x] build broken after JDK-8301996 [v2]
Lutz Schmidt
lucy at openjdk.org
Fri Oct 6 11:17:53 UTC 2023
On Fri, 6 Oct 2023 10:08:56 GMT, Martin Doerr <mdoerr at openjdk.org> wrote:
>> Amit Kumar has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains one commit:
>>
>> s390 Invoke Field Port
>
> src/hotspot/cpu/s390/templateTable_s390.cpp line 82:
>
>> 80: __ z_larl(br_tab_temp, (int64_t)0); /* Check current address alignment. */\
>> 81: __ z_slgr(br_tab_temp, br_tab); /* Current Address must be equal */\
>> 82: if (tos_state != Z_R0_scratch) { \
>
> Where can tos_state be R0?
This was an intermediate state. In my review (not complete yet) I request this special case to be removed.
> src/hotspot/cpu/s390/templateTable_s390.cpp line 86:
>
>> 84: } \
>> 85: __ z_brc(Assembler::bcondLogZero, 4);/* skip trap if ok. */ \
>> 86: __ z_illtrap(0x55); \
>
> Is z_illtrap duplicated by intention? If so, please explain in a comment.
The duplication is to circumvent a disassembler shortcoming. It does not honor the instruction length for unknown (illegal) instructions. One of them should be removed now.
> src/hotspot/cpu/s390/templateTable_s390.cpp line 3006:
>
>> 3004: // Displacement is 0. No need to care about limited displacement range.
>> 3005: const Address field(fieldAddr);
>> 3006: __ lgr_if_needed(fieldAddr, off);
>
> Was there a problem with the range? Why did it work in the old implementation?
The comment is related to the uses of field. Depending on the instruction, disc is limited to be uimm12 or simm20. But the Address object is with displacement == 0.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/15885#discussion_r1348564608
PR Review Comment: https://git.openjdk.org/jdk/pull/15885#discussion_r1348566930
PR Review Comment: https://git.openjdk.org/jdk/pull/15885#discussion_r1348570715
More information about the hotspot-compiler-dev
mailing list