RFR: 8350095: RISC-V: Refactor string_compare [v2]
Hamlin Li
mli at openjdk.org
Thu Feb 20 19:21:56 UTC 2025
On Wed, 19 Feb 2025 01:39:23 GMT, Fei Yang <fyang at openjdk.org> wrote:
>> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
>>
>> fix temp registers; move code
>
> src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 1474:
>
>> 1472:
>> 1473: // Compare longwords
>> 1474: void C2_MacroAssembler::string_compare_long_LU(Register result, Register strL, Register strU,
>
> And rename this to `C2_MacroAssembler::string_compare_long_different_encoding`. We can pass one extra param (say `const bool isLU`) to distinguish the two different cases. Also I think we need to pass the `str1` and `str2` from the callsite directly as the final difference calculation needs to repect the order. The current approach doesn't seem correct: it can only distinguish L and U from the two strings, but it doesn't know the order of the two strings at all.
>
> Java program that hopefully helps demo the effect of the order of the two strings:
>
> String author = "author";
> String book = "book";
> String duplicateBook = "book";
>
> assertThat(author.compareTo(book))
> .isEqualTo(-1);
> assertThat(book.compareTo(author))
> .isEqualTo(1);
> assertThat(duplicateBook.compareTo(book))
> .isEqualTo(0);
I'll fix this, Thanks!
I was wondering why the issue is not caught, seems to me there is some gap in test case for U.compareTo(L), so I created https://github.com/openjdk/jdk/pull/23705, do you mind to help to check it too?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23633#discussion_r1964206741
More information about the hotspot-compiler-dev
mailing list