RFR: 8318217: RISC-V: C2 VectorizedHashCode [v8]
Yuri Gaevsky
duke at openjdk.org
Tue Dec 5 12:57:11 UTC 2023
On Tue, 5 Dec 2023 10:59:48 GMT, Yuri Gaevsky <duke at openjdk.org> wrote:
>> src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 1507:
>>
>>> 1505: #define DO_ELEMENT_LOAD(reg, idx) \
>>> 1506: switch (eltype) { \
>>> 1507: case T_BOOLEAN: lb(reg, Address(ary, idx * elsize)); break; \
>>
>> Since `T_BOOLEAN` is used to signify unsigned bytes [1], shouldn't we use `lbu` instead of `lb` here? Seems that the existing tests didn't cover this case?
>>
>> [1] https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/internal/util/ArraysSupport.java#L192
>
> Agreed.
Fixed.
>> src/hotspot/cpu/riscv/riscv.ad line 10308:
>>
>>> 10306: instruct arrays_hashcode(iRegP_R11 ary, iRegI_R12 cnt, iRegI_R10 result, immI basic_type,
>>> 10307: iRegLNoSp tmp1, iRegINoSp tmp2,
>>> 10308: iRegINoSp tmp3, iRegLNoSp tmp4, rFlagsReg cr)
>>
>> Maybe declare `tmp2` and `tmp3` as `iRegLNoSp`? I see these two are used as 64-bit registers in `C2_MacroAssembler::arrays_hashcode`.
>
> Agreed.
Fixed.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16629#discussion_r1415559969
PR Review Comment: https://git.openjdk.org/jdk/pull/16629#discussion_r1415559639
More information about the hotspot-dev
mailing list