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

[Yuri Gaevsky]

On Tue, 14 Nov 2023 15:49:48 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> Yuri Gaevsky has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Minor cosmetic fixes.
>
> src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 1477:
> 
>> 1475:   case T_SHORT:   BLOCK_COMMENT("arrays_hashcode(short) {");         break;
>> 1476:   case T_INT:     BLOCK_COMMENT("arrays_hashcode(int) {");           break;
>> 1477:   default:        BLOCK_COMMENT("arrays_hashcode {");                break;
> 
> In `C2_MacroAssembler::arrays_hashcode_elsize`, default action is `ShouldNotReachHere();`, should it be consistent here?

Sure, thanks for catching!

> src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 1513:
> 
>> 1511:   bind(WIDE_LOOP);
>> 1512:   DO_ELEMENT_LOAD(tmp1, 0)
>> 1513:   DO_ELEMENT_LOAD(tmp3, 1)
> 
> Would it help to optimize the perf by moving `DO_ELEMENT_LOAD(tmp3, 1)` after `srli(tmp2, pow31_3_4, 32);`?

Sure, it makes the code more understandable even though it doesn't improve performance.

> src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 1523:
> 
>> 1521:   DO_ELEMENT_LOAD(tmp3, 3)
>> 1522:   srli(tmp2, pow31_1_2, 32);
>> 1523:   mulw(tmp1, tmp1, tmp2);          // 31^^1 * ary[i+2]
> 
> Could this line be optimized as `x<<5-x`? Just as TAIL_LOOP below.

Sure, good catch!

> src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 1527:
> 
>> 1525:   addw(result, result, tmp3);      // 31^^4 * h + 31^^3 * ary[i+0] + 31^^2 * ary[i+1]
>> 1526:                                    //           + 31^^1 * ary[i+2] + 31^^0 * ary[i+3]
>> 1527:   subw(chunk, chunk, stride);
> 
> Could chunk and ary be merged into one variable? so we don't need one sub and one add, but only one add here.

Could you please carify? I don't understand how that's possible.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16629#discussion_r1394403870
PR Review Comment: https://git.openjdk.org/jdk/pull/16629#discussion_r1394400845
PR Review Comment: https://git.openjdk.org/jdk/pull/16629#discussion_r1394401160
PR Review Comment: https://git.openjdk.org/jdk/pull/16629#discussion_r1394402664