RFR: 8345110: RISC-V: Optimize and and clean up byte reverse assembler routines
Robbin Ehn
rehn at openjdk.org
Thu Nov 28 12:00:55 UTC 2024
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!
I liked that you used rev8 directly in AD because it makes it very clear that we do not clobber t1. (as it otherwise it is one of the default registers)
As all uses of revb_w use dst == src, please consider a helper version which only needs one registers passed.
I.e. "__ revb_w(temp);"
-------------
Marked as reviewed by rehn (Reviewer).
PR Review: https://git.openjdk.org/jdk/pull/22410#pullrequestreview-2467916836
More information about the hotspot-dev
mailing list