RFR: 8350095: RISC-V: Refactor string_compare [v2]

Hamlin Li mli at openjdk.org
Fri Feb 21 15:02:52 UTC 2025


On Thu, 20 Feb 2025 19:19:44 GMT, Hamlin Li <mli at openjdk.org> wrote:

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

I fixed the issue.
And also add another simple test case to help catch the potential issue.
We could add more tests, but maybe later in another pr.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23633#discussion_r1965630989


More information about the hotspot-compiler-dev mailing list