RFR: 8349908: RISC-V: C2 SelectFromTwoVector

[Hamlin Li]

On Mon, 17 Feb 2025 11:13:09 GMT, Fei Yang <fyang 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>
>
> 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.

Logically, yes. It's not necessary, my thought is to make the code explicit, so friendly to read code. But I can remove it if you think it's better to do so.

> 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.

Yes, I think so.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23614#discussion_r1958266514
PR Review Comment: https://git.openjdk.org/jdk/pull/23614#discussion_r1958269972