RFR: 8344306: RISC-V: Add zicond

Hamlin Li mli at openjdk.org
Tue Nov 26 10:29:39 UTC 2024


On Tue, 26 Nov 2024 09:00:34 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

> Hi, please consider.
> 
> In cpu models we save ~1 cycle per removed branch.
> This patch removes ~0.1% of branches in generic C2 generated code.
> We should probably investigate if we can improve/add peephole optimization to remove more branches.
> 
> As the C1 cmov code is a bit tricky I left that as a followup.
> 
> I added gtests for the cmovs.
> (we should add coverage for more of masm in this gtest suit)
> Pro tip, invoke the gtestLauncher directly (you only need to build exploded):
> `gtestLauncher -jdk build/linux-riscv64-server-fastdebug/jdk/ --gtest_break_on_failure --gtest_filter="*RiscV*" -XX:+UnlockDiagnosticVMOptions -XX:+UseZicond -XX:ParallelGCThreads=1 -XX:ConcGCThreads=1 -XX:CICompilerCount=2`
> 
> Tested on Spacemit X60, gtests and tier1.
> 
> Thanks, Robbin

Thanks for adding the support for this extension!
The code change looks good. Just have some questions below.

src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 2047:

> 2045:       assert(false, "unsupported compare condition");
> 2046:       ShouldNotReachHere();
> 2047:   }

Seems we can use the similar way in `C2_MacroAssembler::cmp_branch` +`conditional_branches` to simplify the code.

src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 1143:

> 1141:   bne(cmp1, cmp2, no_set);
> 1142:   mv(dst, src);
> 1143:   bind(no_set);

Do we have performance data comparison between `-UseZicond ` and `+ UseZicond `?

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

PR Review: https://git.openjdk.org/jdk/pull/22386#pullrequestreview-2461030531
PR Review Comment: https://git.openjdk.org/jdk/pull/22386#discussion_r1858210922
PR Review Comment: https://git.openjdk.org/jdk/pull/22386#discussion_r1858207974


More information about the hotspot-dev mailing list