RFR: 8318218: RISC-V: C2 CompressBits [v5]

[Fei Yang]

On Thu, 9 Nov 2023 10:35:12 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> Hi,
>> Can you review the change to add intrinsic for CompressBits for Long & Integer?
>> Thanks!
>> 
>> ## Test
>> pass jtreg test:
>> test/jdk/java/lang/CompressExpand*.java
>
> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
> 
>   remove extra new line

Changes requested by fyang (Reviewer).

src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 1687:

> 1685:   Assembler::SEW sew = is_long ? Assembler::e64 : Assembler::e32;
> 1686:   // intrinsic is enabled when MaxVectorSize >= 16
> 1687:   Assembler::LMUL lmul = is_long ? Assembler::m4 : Assembler::m2;

A `lmul` of `m2` or `m4` means vector register group of 2 or 4 registers respectively. So <`v4`, `v5`> and <`v8`, `v9`> will be used in the case of `m2`. But I only see `v4` and `v8` are reserved for CompressBits nodes. You should also reserve `v5` and `v9` for this case. Similar for the `m4` case.

src/hotspot/cpu/riscv/c2_MacroAssembler_riscv.cpp line 1694:

> 1692:   vmv_s_x(v0, src);
> 1693:   // reset the src data(in bytes) to zero.
> 1694:   mv(tmp, len);

Can we use scratch register `t0` instead of `tmp` in this function?

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

> 2882: 
> 2883: instruct compressBitsI(iRegINoSp dst, iRegIorL2I src, iRegIorL2I mask, iRegPNoSp tmp, vRegMask_V0 v0, vReg_V4 v4, vReg_V8 v8) %{
> 2884:   predicate(UseRVV);

Seems this UseRVV check is also redundant. It has already been checked in `Matcher::match_rule_supported` for this node. But seems that this is not the first one. So we might want do this cleanup in your next PR: https://github.com/openjdk/jdk/pull/16580

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

> 2907: 
> 2908: instruct compressBitsL(iRegLNoSp dst, iRegL src, iRegL mask, iRegPNoSp tmp, vRegMask_V0 v0, vReg_V4 v4, vReg_V8 v8) %{
> 2909:   predicate(UseRVV);

Similar here.

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

PR Review: https://git.openjdk.org/jdk/pull/16481#pullrequestreview-1724331115
PR Review Comment: https://git.openjdk.org/jdk/pull/16481#discussion_r1389112263
PR Review Comment: https://git.openjdk.org/jdk/pull/16481#discussion_r1389113911
PR Review Comment: https://git.openjdk.org/jdk/pull/16481#discussion_r1389143653
PR Review Comment: https://git.openjdk.org/jdk/pull/16481#discussion_r1389143814