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