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

Fei Yang fyang at openjdk.org
Tue Jul 2 11:39:20 UTC 2024


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

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

Maybe we can create a class for vector register groups and pass references instead? That would help eliminate the problem of passing too many parameters for vector registers.

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

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


More information about the hotspot-compiler-dev mailing list