RFR: 8318217: RISC-V: C2 VectorizedHashCode [v8]

[Yuri Gaevsky]

On Tue, 5 Dec 2023 10:37:43 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Yuri Gaevsky has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Removed comment and break clause from default switch case.
>
> 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.

> 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.

> src/hotspot/cpu/riscv/riscv.ad line 10312:
> 
>> 10310:   match(Set result (VectorizedHashCode (Binary ary cnt) (Binary result basic_type)));
>> 10311:   effect(TEMP tmp1, TEMP tmp2, TEMP tmp3, TEMP tmp4,
>> 10312:          USE_KILL ary, USE_KILL cnt, USE basic_type, KILL cr);
> 
> I don't think we need to add `USE basic_type` to the effect list. It is an immediate input which is already there in the match rule.

I've borrowed that part from its X86 counterpart:

instruct arrays_hashcode(rdi_RegP ary1, rdx_RegI cnt1, rbx_RegI result, immU8 basic_type,
                         ...,
                         legRegD tmp_vec13, rRegI tmp1, rRegI tmp2, rRegI tmp3, rFlagsReg cr)
%{
  predicate(UseAVX >= 2);
  match(Set result (VectorizedHashCode (Binary ary1 cnt1) (Binary result basic_type)));
  effect(...,
         USE basic_type, KILL cr);

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16629#discussion_r1415365533
PR Review Comment: https://git.openjdk.org/jdk/pull/16629#discussion_r1415365678
PR Review Comment: https://git.openjdk.org/jdk/pull/16629#discussion_r1415365817