RFR: 8293566: RISC-V: Clean up push and pop registers

[Fei Yang]

On Fri, 9 Sep 2022 08:31:02 GMT, Feilong Jiang <fjiang at openjdk.org> wrote:

> Currently, riscv port has two implementations of `push_reg` and `pop_reg`.
> The bitset version can be replaced with the RegSet version for better readability.
> 
> For example, if we want to push x10 and x11, pushing register with bitset would be like:
> 
> push_reg(0xc00, sp)
> 
> 
> while pushing register with RegSet will be more clear:
> 
> push_reg(RegSet::of(x10, x11), sp);
> 
> 
> So we decide to remove the bitset version of push and pop registers.
> 
> Testing:
> 
> - hotspot and jdk ter1 on QEMU without new failures

Thanks for the cleanup. I have two comments here.

src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 955:

> 953: 
> 954:   unsigned char regs[32];
> 955:   int count = bitset_to_regs(regset.bits(), regs);

I think it's better to have a zero-check for regs.bits() before we run into the loop in MacroAssembler::bitset_to_regs. So I would suggest make the old API "int MacroAssembler::push_reg(unsigned int bitset, Register stack)" a private member function and implement the new API as:

void MacroAssembler::push_reg(RegSet regset, Register stack) { if (regs.bits()) push_reg(regs.bits(), stack); }

Similar for pop_reg and other APIs here.

src/hotspot/cpu/riscv/macroAssembler_riscv.hpp line 484:

> 482: #ifdef COMPILER2
> 483:   void push_vp(VectorRegSet regset, Register stack);
> 484:   void pop_vp(VectorRegSet regset, Register stack);

The function name push_vp / pop_vp look a bit strange to me. I think we should rename them to push_v / pop_v.

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

Changes requested by fyang (Reviewer).

PR: https://git.openjdk.org/jdk/pull/10227