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

[Hamlin Li]

On Mon, 1 Jul 2024 09:36:15 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 2361:
>> 
>>> 2359:   mv(result, false);
>>> 2360: 
>>> 2361:   element_compare(a1, a2, result, cnt, tmp1, tmp2, v2, v4, v2, true, DONE, Assembler::m2);
>> 
>> It's not apparent to me why you use m2 here. Can you add a comment?
>
> Agree. I will verify the string equals later soon, in that patch, I will add the comment accordingly based on test result. Here, it's the previous value which I just passed in as explicit value.

I'd like to discuss another thing related to this. Currently, in riscv_v.ad file, we explicitly ask for the temp v registers we need, but in corresponding macro assembler code, there is no such explicit statement about it. So there is kind of a risk here, i.e. a future patch could accidentally change the code from `element_compare(a1, a2, result, cnt, tmp1, tmp2, v2, v4, v2, true, DONE, Assembler::m2);` to `element_compare(a1, a2, result, cnt, tmp1, tmp2, v4, v8, v4, true, DONE, Assembler::m4);`, on the other hand it could fail to change corresponding code in riscv_v.ad file, i.e forget to reserve the v6-v11.
I think it helps to make all such calls (i.e. vector code that use more than one v register in one register group) explicit, i.e. in riscv_v.ad file, we pass all the temporary vector registers explicitly when calling into functions in macroAssemble functions. It will make the code more tedious, but it’s safer than before, else it’s error-prone, and the potential bugs will be random. Of course, I will put the patch in a new pr to address all existing such code.
How do you think about it? Maybe there is other better way to address such issue? Please kindly share your suggestion.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19825#discussion_r1660762969