RFR: 8350095: RISC-V: Refactor string_compare
Fei Yang
fyang at openjdk.org
Tue Feb 18 07:48:08 UTC 2025
On Fri, 14 Feb 2025 14:32:07 GMT, Hamlin Li <mli at openjdk.org> wrote:
> Hi,
> Can you help to review this patch?
>
> Currently, `string_compare` code is a bit complicated, main reasons include:
> 1. it has 2 piece of code respectively for LU and UL case, this is not necessary, basically LU and UL behaviour quite similar.
> 2. it mixed LL/UU and LU/UL case together, better to separate them, as they are quite different from each other.
>
> This is not good for code reading and maintaining.
>
>
> So, this patch does following refactoring:
> 1. merge LU and UL code into one, i.e. remove UL code.
> 2. seperate the code into 2 methods: LL/UU and LU/UL.
> 3. some other misc improvement.
>
> I could do the following refactoring in another following pr, as in this pr I'm just moving code and removing code, it's easier to do it and review it. In particular the first one, as it needs to rewrite the existing code for UL/LU case.
> 1. move alignment code of `generate_compare_long_string_different_encoding` upwards to `string_compare_long_LU`.
> 2. make `SHORT_STRING` case simpler.
>
>
>
> Thanks
Hi, there is a difference between LU and UL when calculating the difference [1]. The `tmp1` and `tmp2` here represent characters from the first and second string respectively. So the order of the two strings matters here. But that doesn't seem to be reflected by this change?
BTW: Seems more readable if we move the difference calcuation code to `string_compare_long_LU` and `string_compare_long_LL_UU` at the same time.
[1] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp#L1571
-------------
PR Comment: https://git.openjdk.org/jdk/pull/23633#issuecomment-2664840634
More information about the hotspot-compiler-dev
mailing list