RFR: 8307609: RISC-V: Added support for Extract, Compress, Expand and other nodes for Vector API [v3]

[Feilong Jiang]

On Mon, 15 May 2023 07:42:08 GMT, Dingli Zhang <dzhang at openjdk.org> wrote:

>> Hi all,
>> 
>> We have added support for Extract, Compress, Expand and other nodes for Vector
>> API. It was implemented by referring to RVV v1.0 [1]. Please take a look and
>> have some reviews. Thanks a lot.
>> 
>> In this PR, we will support these new nodes:
>> 
>> CompressM/CompressV/ExpandV
>> LoadVectorGather/StoreVectorScatter/LoadVectorGatherMasked/StoreVectorScatterMasked
>> Extract
>> VectorLongToMask/VectorMaskToLong
>> PopulateIndex
>> VectorLongToMask/VectorMaskToLong
>> VectorMaskTrueCount/VectorMaskFirstTrue
>> VectorInsert
>> 
>> 
>> At the same time, we refactored methods such as
>> `match_rule_supported_vector_mask`. All implemented vector nodes support mask
>> operations by default now, so we also added mask nodes for all implemented
>> nodes. 
>> 
>> By the way, we will implement the VectorTest node in the next PR.
>> 
>> We can use the tests under `test/jdk/jdk/incubator/vector` to print the 
>> compilation log for most of the new nodes. And we can use the following 
>> command to print the compilation log of a jtreg test case:
>> 
>> 
>> $ jtreg \
>> -v:default \
>> -concurrency:16 -timeout:50 \
>> -javaoption:-XX:+UnlockExperimentalVMOptions \
>> -javaoption:-XX:+UseRVV \
>> -javaoption:-XX:+PrintOptoAssembly \
>> -javaoption:-XX:LogFile=log_name.log \
>> -jdk:build/linux-riscv64-server-fastdebug/jdk \
>> -compilejdk:build/linux-x86_64-server-release/images/jdk \
>> <test-case>
>> 
>> 
>> 
>> 
>> ### CompressM/CompressV/ExpandV
>> 
>> There is no inverse vdecompress provided in RVV, as this operation can be
>> readily synthesized using iota and a masked vrgather in `ExpandV`.
>> 
>> We can use `test/jdk/jdk/incubator/vector/Float256VectorTests.java` to emit
>> these nodes and the compilation log is as follows:
>> 
>> 
>> ## CompressM
>> 2aa     addi  R29, R10, #16	# ptr, #@addP_reg_imm
>> 2ae     mcompress V0, V30	# KILL R30
>> 2c2     vstoremask V2, V0
>> 2ce     storeV [R7], V2	# vector (rvv)
>> 2d6     bgeu  R29, R28, B47	#@cmpP_branch  P=0.000100 C=-1.000000
>> 
>> ## CompressV
>> 0ee     addi  R29, R10, #16	# ptr, #@addP_reg_imm
>> 0f2     vcompress V1, V2, V0
>> 0fe     storeV [R7], V1	# vector (rvv)
>> 106     bgeu  R29, R28, B10	#@cmpP_branch  P=0.000100 C=-1.000000
>> 
>> ## ExpandV
>> 0ee     addi  R29, R10, #16	# ptr, #@addP_reg_imm
>> 0f2     vexpand V3, V2, V0
>> 102     storeV [R7], V3	# vector (rvv)
>> 10a     bgeu  R29, R28, B10	#@cmpP_branch  P=0.000100 C=-1.000000
>> 
>> 
>> 
>> 
>> ### LoadVectorGather/StoreVectorScatter/LoadVectorGatherMasked/StoreVectorScatterMasked
>> 
>> We use the vs...
>
> Dingli Zhang has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Remove debug warning

Overall looks good, with some comments.

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

> 2893: // vector sqrt - predicated
> 2894: 
> 2895: instruct vsqrt_masked(vReg dst_src, vRegMask_V0 v0) %{

Maybe it's better to split predicated version into `vsqrtF_masked` and `vsqrtD_masked` just like vector sqrt did.

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

> 3362: %}
> 3363: 
> 3364: instruct vmaskAllI_masked(vRegMask dst, iRegI src, vRegMask_V0 v0, vReg tmp) %{

`iRegI` -> `iRegIorL2I`

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

> 4047: // ------------------------------ Populate Index to a Vector -------------------
> 4048: 
> 4049: instruct populateindex(vReg dst, iRegI src1, iRegI src2, vReg tmp1) %{

`iRegI` -> `iRegIorL2I`

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

> 4066: // BYTE, SHORT, INT
> 4067: 
> 4068: instruct insertI_index_lt32(vReg dst, vReg src, iRegI val, immI idx, vRegMask_V0 v0) %{

`iRegI` -> `iRegIorL2I`

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

> 4085: %}
> 4086: 
> 4087: instruct insertI_index(vReg dst, vReg src, iRegI val, iRegI idx, vReg tmp1, vRegMask_V0 v0) %{

`iRegI` -> `iRegIorL2I`

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

> 4123: %}
> 4124: 
> 4125: instruct insertL_index(vReg dst, vReg src, iRegL val, iRegI idx, vReg tmp1, vRegMask_V0 v0) %{

Can the reg type of `idx` be `iRegIorL2I`?

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

> 4158: %}
> 4159: 
> 4160: instruct insertF_index(vReg dst, vReg src, fRegF val, iRegI idx, vReg tmp1, vRegMask_V0 v0) %{

Same here for reg type of `idx`.

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

> 4192: %}
> 4193: 
> 4194: instruct insertD_index(vReg dst, vReg src, fRegD val, iRegI idx, vReg tmp1, vRegMask_V0 v0) %{

Same here for reg type of `idx`.

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

PR Review: https://git.openjdk.org/jdk/pull/13862#pullrequestreview-1426532344
PR Review Comment: https://git.openjdk.org/jdk/pull/13862#discussion_r1193818885
PR Review Comment: https://git.openjdk.org/jdk/pull/13862#discussion_r1193822075
PR Review Comment: https://git.openjdk.org/jdk/pull/13862#discussion_r1193830583
PR Review Comment: https://git.openjdk.org/jdk/pull/13862#discussion_r1193830974
PR Review Comment: https://git.openjdk.org/jdk/pull/13862#discussion_r1193831496
PR Review Comment: https://git.openjdk.org/jdk/pull/13862#discussion_r1193834535
PR Review Comment: https://git.openjdk.org/jdk/pull/13862#discussion_r1193835527
PR Review Comment: https://git.openjdk.org/jdk/pull/13862#discussion_r1193837758