RFR: 8357551: RISC-V: support CMoveF/D vectorization

Emanuel Peter epeter at openjdk.org
Fri Nov 14 16:07:06 UTC 2025


On Thu, 13 Nov 2025 21:34:30 GMT, Hamlin Li <mli at openjdk.org> wrote:

> Hi,
> 
> This pr add CMoveF/D on riscv, which enable vectorization of statement like: `op_1 bop op_2 ? res_f_d_1 : res_f_d_2 in a loop`.
> 
> This pr is also a preparation for further vectorization in https://github.com/openjdk/jdk/pull/28231.
> 
> Previously it's https://github.com/openjdk/jdk/pull/25341, but at that time, C2 SLP has some issue with unsigned comparison, which is now fixed, so it's good to continue the work.
> 
> # Test
> ## Jtreg
> 
> in progress...
> 
> ## Performance
> 
> Column names meanings:
> * p: with patch
> * p+v: with patch, `-XX:+UseVectorCmov -XX:+UseCMoveUnconditionally` turned on
> * m: without patch
> * m+v: without patch, `-XX:+UseVectorCmov -XX:+UseCMoveUnconditionally` turned on
> 
> #### Average improvement
> 
> NOTE: With only this PR, it brings performance benefit in case of `CMoveF+CmpF`, `CMoveD+ComD`, `CMoveF+CmpI`, `CMoveD+CmpL`. The data below is based on fullly implmenting the vectorization of `CMoveI/L/F/D+CmpI/L/F/D`, which will be achieved by https://github.com/openjdk/jdk/pull/28231.
> 
> For details, check the performance data in https://github.com/openjdk/jdk/pull/25341 on riscv.
> <google-sheets-html-origin style="caret-color: rgb(0, 0, 0); color: rgb(0, 0, 0); font-style: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration: none;">
> Opt (m/p) | Opt (m+v/p+v) | Opt (p/p+v) | Opt (m/p+v)
> -- | -- | -- | --
> 1.022782609 | 2.198717391 | 2.162673913 | 2.199
> 
> </google-sheets-html-origin>

I won't be able to review the RISCV part, so you'll have to find someone else there. I just dropped 2 drive-by comments about the tests :)

test/hotspot/jtreg/compiler/c2/irTests/TestScalarConditionalMove.java line 36:

> 34:  * @test
> 35:  * @summary Test conditional move.
> 36:  * @requires vm.simpleArch == "riscv64"

I would prefer if you could enable the test on all platforms, but just require the specific platform on the IR rules.
What would be even more fantastic: if you were able to also enable the IR rules for `x64` and `aarch64`, but we can also file a follow-up RFE for that.

test/hotspot/jtreg/compiler/c2/irTests/TestScalarConditionalMove.java line 49:

> 47:                                    "-XX:+UnlockExperimentalVMOptions", "-XX:-UseCompactObjectHeaders");
> 48:         TestFramework.runWithFlags("-XX:+UseCMoveUnconditionally", "-XX:-UseVectorCmov",
> 49:                                    "-XX:+UnlockExperimentalVMOptions", "-XX:+UseCompactObjectHeaders");

Wait. Is this just a copy of the existing vector test, but run with CMove vectorization disabled?
If so, we could just add these additional runs to the existing test, and guard the IR test with corresponding flags:
Have an IR rule for `-XX:-UseVectorCmov` and one for `-XX:+UseVectorCmov`.

That would allow us to reduce some code duplication. And it would also avoid letting the two tests go out of sync when people add more to one but not the other.

What do you think?

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

PR Review: https://git.openjdk.org/jdk/pull/28309#pullrequestreview-3465590460
PR Review Comment: https://git.openjdk.org/jdk/pull/28309#discussion_r2528003621
PR Review Comment: https://git.openjdk.org/jdk/pull/28309#discussion_r2528011154


More information about the core-libs-dev mailing list