On Mon, 17 Nov 2025 13:38:40 GMT, Hamlin Li <[email protected]> wrote:

>> src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 2133:
>> 
>>> 2131:       break;
>>> 2132:     case BoolTest::ge:
>>> 2133:       assert(false, "Should go to BoolTest::le case");
>> 
>> I am not sure if it's safe to have these assertions for `ge` and `gt`. It 
>> seems to me that we should handle all possible condition codes here. Check 
>> this bug: https://bugs.openjdk.org/browse/JDK-8358892. We have added 
>> handling for `ge` and `gt` in `C2_MacroAssembler::enc_cmove_cmp_fp` to fix 
>> it.
>
> Make sense! Thanks!
> I'll add the implementation for these condition codes.

I added some code and tests.
But the code path for `ge`/`gt` can not be triggerred (I added some new test 
based on previous tests added in https://bugs.openjdk.org/browse/JDK-8358892). 
So for now, I think it's safer for us to keep the `assert`, in this way, in the 
future when we get it triggerred by some code we can compse a jtreg test and 
fix it.
How do you think about it?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/28309#discussion_r2534803387

Reply via email to