RFR: 8349908: RISC-V: C2 SelectFromTwoVector

[Fei Yang]

On Thu, 13 Feb 2025 14:20:40 GMT, Hamlin Li <mli at openjdk.org> wrote:

> Hi,
> Can you help to review the patch?
> This optimization is mainly for the vector API.
> 
> Thanks
> 
> ## Test
> 
> ### jtreg
> test/jdk/jdk/incubator/vector/
> 
> ### Performance
> run on bananapi
> 
> master vs patch
> <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;">
> Benchmark | (size) | Mode | Cnt | Score -master | Error - master | Score - patch | Error - patch | Units | Improvement (master / patch)
> -- | -- | -- | -- | -- | -- | -- | -- | -- | --
> SelectFromBenchmark.selectFromByteVector | 1024 | avgt | 10 | 26422.495 | 674.565 | 721.604 | 1.036 | ns/op | 36.616
> SelectFromBenchmark.selectFromByteVector | 2048 | avgt | 10 | 53964.411 | 1751.618 | 1385.24 | 0.956 | ns/op | 38.957
> SelectFromBenchmark.selectFromDoubleVector | 1024 | avgt | 10 | 218430.616 | 1369.409 | 7739.774 | 14.408 | ns/op | 28.222
> SelectFromBenchmark.selectFromDoubleVector | 2048 | avgt | 10 | 387889.456 | 7889.791 | 16197.77 | 66.775 | ns/op | 23.947
> SelectFromBenchmark.selectFromFloatVector | 1024 | avgt | 10 | 103483.717 | 492.525 | 3580.358 | 29.127 | ns/op | 28.903
> SelectFromBenchmark.selectFromFloatVector | 2048 | avgt | 10 | 226125.02 | 3118.836 | 7797.025 | 4.346 | ns/op | 29.001
> SelectFromBenchmark.selectFromIntVector | 1024 | avgt | 10 | 97007.999 | 2607.711 | 2898.38 | 0.833 | ns/op | 33.47
> SelectFromBenchmark.selectFromIntVector | 2048 | avgt | 10 | 222303.308 | 3096.615 | 6398.214 | 30.345 | ns/op | 34.745
> SelectFromBenchmark.selectFromLongVector | 1024 | avgt | 10 | 245033.436 | 1652.527 | 6307.773 | 24.597 | ns/op | 38.846
> SelectFromBenchmark.selectFromLongVector | 2048 | avgt | 10 | 438503.547 | 5972.265 | 17215.996 | 167.442 | ns/op | 25.471
> SelectFromBenchmark.selectFromShortVector | 1024 | avgt | 10 | 53632.502 | 2159.433 | 1418.215 | 2.953 | ns/op | 37.817
> SelectFromBenchmark.selectFromShortVector | 2048 | avgt | 10 | 111764.327 | 1220.509 | 3061.386 | 14.716 | ns/op | 36.508
> 
> </google-sheets-html-origin>

Seems fine. I have two minor questions. Thanks.

src/hotspot/cpu/riscv/riscv_v.ad line 103:

> 101:         break;
> 102:       case Op_SelectFromTwoVector:
> 103:         return true;

Seems not necessary to add one more case here as the default will return true for this case.

src/hotspot/cpu/riscv/riscv_v.ad line 4447:

> 4445:     __ vsetvli_helper(bt, Matcher::vector_length(this));
> 4446:     __ vrgather_vv(as_VectorRegister($dst$$reg), as_VectorRegister($src1$$reg),
> 4447:                    as_VectorRegister($index$$reg));

I suppose the indices here comes with the same element width as the two vector sources? The spec says `vrgather.vv` uses same SEW/LMUL for both the data and indices.

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

PR Review: https://git.openjdk.org/jdk/pull/23614#pullrequestreview-2620754379
PR Review Comment: https://git.openjdk.org/jdk/pull/23614#discussion_r1958053692
PR Review Comment: https://git.openjdk.org/jdk/pull/23614#discussion_r1958066222