RFR: 8334554: RISC-V: verify & fix perf of string comparison [v2]

[Hamlin Li]

On Tue, 25 Jun 2024 14:57:22 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   refine lmul value
>
> src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 2455:
> 
>> 2453:     // Below construction of v regs and lmul is based on test on 2 different boards,
>> 2454:     // vlen == 128 and vlen == 256 respectively.
>> 2455:     if (!str1_isL && MaxVectorSize == 16) { // UU
> 
> Seems more readable to check `!encLL` instead of `!str1_isL`. As you see, we also pass this `encLL` to `element_compare` to indicate whether they are both Latin strings.

fixed.

> src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.hpp line 40:
> 
>> 38:                        VectorRegister vr1, VectorRegister vr2,
>> 39:                        VectorRegister vrs,
>> 40:                        bool is_latin, Label& DONE, Assembler::LMUL lmul = Assembler::m2);
> 
> I think it's safer to pass this `lmul` param explictly at all call sites. So no need for a default param.

Yes, I was thinking so too.

> src/hotspot/cpu/riscv/riscv_v.ad line 3566:
> 
>> 3564: %}
>> 3565: 
>> 3566: instruct vstring_compareUVlen16(iRegP_R11 str1, iRegI_R12 cnt1, iRegP_R13 str2, iRegI_R14 cnt2,
> 
> Maybe instruct names like `vstring_compareU_128b` which I think is more general.

Yes, it's better.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19825#discussion_r1653028354
PR Review Comment: https://git.openjdk.org/jdk/pull/19825#discussion_r1653028156
PR Review Comment: https://git.openjdk.org/jdk/pull/19825#discussion_r1653028101