RFR: 8357554: Enable vectorization of Bool -> CMove with different type size (on riscv)

Hamlin Li mli at openjdk.org
Mon Jun 2 14:13:51 UTC 2025


On Wed, 28 May 2025 09:20:03 GMT, Emanuel Peter <epeter at openjdk.org> wrote:

> @Hamlin-Li Thanks for working on this!

@eme64
Sorry for the delayed reply, I've been on vacation.
Thank you for having a look!

> Can you please provide the the JMH benchmark results for your measurements?

Sure, I have the data in https://github.com/openjdk/jdk/pull/25341, I can copy the data here. But it won't impact jmh result until https://github.com/openjdk/jdk/pull/25341 is pushed in. I'll add more jmh test and data for integral types.

> It would also be good to have some IR tests, that cover the newly vectorized cases.

You're right, will add more IR tests.

> src/hotspot/cpu/riscv/matcher_riscv.hpp line 204:
> 
>> 202:   static bool supports_vectorize_cmove_bool_unconditionally() {
>> 203:     return true;
>> 204:   }
> 
> Does RISCV support the use of any input vector element type, including 8bit, 16bit, 32bit and 64bit masks, and any elements we would be blending, incl `byte, short, char, int, long, HF, F, D`?
> 
> Because it sounds you are promissing this really "unconditionally". Or what exactly do you mean by "unconditionally"?

( In this pr, it should return false for riscv too and be enabled in the riscv pr. I'll modify it. )

> Does RISCV support the use of any input vector element type, including 8bit, 16bit, 32bit and 64bit masks, and any elements we would be blending, incl byte, short, char, int, long, HF, F, D?

Good question! I'll add some additional tests to double check and reflect this.

I think the answer should be yes, i.e. on riscv all size of source inputs (comparing operands) and all size of dest outputs (blending result) are supported.
But for HF, it's a bit special, the underlying payload is a short, so in theory it should be supported too, but it's not supported in this pr and the related riscv pr (https://github.com/openjdk/jdk/pull/25341).

> Because it sounds you are promissing this really "unconditionally". Or what exactly do you mean by "unconditionally"?

I mean it's really "unconditionally", but if you feel it's better to add an argument, like `supports_vectorize_cmove_bool_unconditionally(BasicType src, BasicType dst)`, I can do it.
And I need to modify the `vectornode.cpp` as below too, I'll check it and modify this pr.
```  case Op_CMoveI:
    return (is_integral_type(bt) && bt != T_LONG ? Op_VectorBlend : 0);

> src/hotspot/share/opto/superword.cpp line 2363:
> 
>> 2361:       VectorNode::is_vectorize_cmove_bool_unconditionally_supported()) {
>> 2362:     return true;
>> 2363:   }
> 
> Can you please list which additional cases this now allows?
> I suppose `D/F` comparison for the `Bool`, and then `D/F` inputs for `CMove`, but we can mismatch, e.g. compare `F` but blend `D`, right?

Sure, I'll add this list.

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

PR Comment: https://git.openjdk.org/jdk/pull/25336#issuecomment-2930920880
PR Review Comment: https://git.openjdk.org/jdk/pull/25336#discussion_r2121272954
PR Review Comment: https://git.openjdk.org/jdk/pull/25336#discussion_r2121273227


More information about the hotspot-compiler-dev mailing list