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