RFR: 8334554: RISC-V: verify & fix perf of string comparison [v3]
Ludovic Henry
luhenry at openjdk.org
Tue Jul 2 13:16:20 UTC 2024
On Tue, 2 Jul 2024 11:36:23 GMT, Fei Yang <fyang at openjdk.org> wrote:
>> 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.
Generally speaking, it would be great for C2 to be more register-group-aware. So if we specify `vReg_V2m2` it includes both `v2` and `v3` for example, and marking a `vReg_V2m2` as TEMP marks both `v2` and `v3` as TEMP implicitly.
We already have the concept of `operand` ([`vRef_V2`](https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/riscv/riscv.ad#L3439-L3447) for example), but that expresses a `vRef_V2` is any of the register in the list `[v2_reg]` (aka `v2_reg` only). I don't know that there is a concept for a `vRef_V2m2` being a group of register (aka all register in `[v2_reg, v3_reg]`).
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19825#discussion_r1662493847
More information about the hotspot-compiler-dev
mailing list