RFR: 8345110: RISC-V: Optimize and and clean up byte reverse assembler routines

[Hamlin Li]

On Wed, 27 Nov 2024 12:28:40 GMT, Fei Yang <fyang at openjdk.org> wrote:

> Hi, Could you please help review this change?
> 
> MacroAssembler routines `revb_w` and `revb_w_w` will emit 32 and 17 instructions respectively without Zbb.
> 
> Major changes:
> 1. There is no need for `revb_w` to reverse bytes for each 32-bit word in the 64-bit register.
>    All the callers only care about the lower 32-bit word. This reimplement `revb_w` so that it only
>    reverses bytes in lower 32-bit word and sign-extends the result (14 instructions). This makes `revb_w_w` duplicate.
> 2. Removed some assembler routines that are unused or become unnecessary (`revb_h`, `revb_h_helper`, `revb_h_h`, `revb_w_w`).
> 3. Adapted callers of these removed assembler routines in file `riscv_b.ad` to emit Zbb instructions directly.
> 
> Will look into `revb_h_h_u` and `revb_h_w_u` after this change. Seems to me they could be factored out as well.
> 
> Testing on linux-riscv64 platform:
> - [x] tier1 (release)
> - [x] hotspot:tier1 (fastdebug)
> - [x] non-trivial benchmark workloads like Dacapo, SpecJBB, Renaissance (release)

Thanks for cleanup!
Not sure if I understand correctly, just have some questions, in upstream code at [1] [2], seems they both not expect a signed-extending?

[1] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/riscv/templateTable_riscv.cpp#L1935

  // set counter
  __ lwu(x11, Address(x9, BytesPerInt));
  __ revb_w(x11, x11);
  __ j(loop_entry);
  // table search
  __ bind(loop);
  __ shadd(t0, x11, x9, t0, 3);

[2] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/riscv/templateTable_riscv.cpp#L2009

  // Initialize i & j
  __ mv(i, zr);                            // i = 0
  __ lwu(j, Address(array, -BytesPerInt)); // j = length(array)

  // Convert j into native byteordering
  __ revb_w(j, j);

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

PR Review: https://git.openjdk.org/jdk/pull/22410#pullrequestreview-2467411571