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

[Hamlin Li]

On Fri, 10 Nov 2023 09:08:00 GMT, Fei Yang <fyang at openjdk.org> wrote:

>> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   remove extra new line
>
> 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.

Good catch, fixed.

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

done

> 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

Sure, let me clean it there.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16481#discussion_r1389289424
PR Review Comment: https://git.openjdk.org/jdk/pull/16481#discussion_r1389289518
PR Review Comment: https://git.openjdk.org/jdk/pull/16481#discussion_r1389289964