RFR: 8340241: RISC-V: Returns mispredicted [v15]

[Robbin Ehn]

On Thu, 17 Oct 2024 08:06:26 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Robbin Ehn has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 18 additional commits since the last revision:
>> 
>>  - Merge branch 'master' into remove_t0
>>  - Allow x1 as Rs
>>  - Comments fixes
>>  - Merge branch 'master' into remove_t0
>>  - ICData move to t1, removed dead code
>>  - Revert clinit_barrier
>>  - Fixed no explicit use of default t1
>>  - Revert clinit_barrier t1
>>  - Upstream comment
>>  - Fixed no explicit use of default t1
>>  - ... and 8 more: https://git.openjdk.org/jdk/compare/9424a955...52b02cb8
>
> src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 4333:
> 
>> 4331:   IncompressibleRegion ir(this);
>> 4332:   Register receiver = j_rarg0;
>> 4333:   Register data = t0;
> 
> As `ic_check` is used by `MachUEPNode::emit`, I think you need to update IC data register usage in `MachUEPNode::format` as well.

Yes, good, fixed the prints.

> src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 5162:
> 
>> 5160:   assert(is_power_of_2(zero_words_block_size), "adjust this");
>> 5161:   assert(ptr == x28 && cnt == x29, "mismatch in register usage");
>> 5162:   assert_different_registers(cnt, t0, t1);
> 
> Here is another case. The call site of `MacroAssembler::zero_words` is `instruct clearArray_reg_reg` which is also missing a `KILL cr` effect after this change (Also see https://bugs.openjdk.org/browse/JDK-8247979). This is similar with the previous reported `instruct partialSubtypeCheckConstSuper` case.
> 
> (PS: I haven't checked all the `rt_call` assembler routine call sites, not sure if they bear similar issues (clobber rflags or ICdata silently))

icdata only needs be present in the handful first assebmly lines in the entry, as ic_check is the first thing we do as long as that do not clobber t0 we are good to go.

Fixed the ones you found thanks!
I'll look tomorrow if there are anymore!

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

PR Review Comment: https://git.openjdk.org/jdk/pull/21406#discussion_r1804845802
PR Review Comment: https://git.openjdk.org/jdk/pull/21406#discussion_r1804849269