RFR: 8320697: RISC-V: Small refactoring for runtime calls

Fei Yang fyang at openjdk.org
Thu Nov 30 07:30:05 UTC 2023


On Sun, 26 Nov 2023 10:52:18 GMT, Feilong Jiang <fjiang at openjdk.org> wrote:

> Hi, please review this refactoring for runtime calls.
> Major changes:
> 1. Unified the runtime calls with the existing MacroAssembler::rt_call. This will remove the duplicate code like `relocate(target.rspec() [&] {...}` to emit uncompressed instructions.
> 2. Removed MacroAssembler::far_branches and made the call sites default to far branches. `branch_range` is 1MB for riscv, and `ReservedCodeCacheSize` will always bigger than `branch_range` in practice. We should remove this unnecessary check and simplify the code logic.
> 3. Renamed MacroAssembler::la_patchable with MacroAssembler::auipc making it explicit that this will emit pc-relative addressing code.
> 4. `far_call` in `rt_call` should use `tmp` instead of the default temporary register `t0`
> 
> 
> Testing:
> - [x] Tier1-3 tested on hifive unmatched board (release)
> - [x] Run non-trivial benchmark workloads (fastdebug)

Another way which I think might resolve our concerns is to simply rename `la_patchable` as `la` and overload the
existing three `la` assembler functions [1][2][3].

In fact, this `la_patchable` is very similar to the existing `MacroAssembler::la(Register Rd, const address addr)` [1].
The only difference is that `la_patchable` will do some distance checks based on high/low bound of code cache and return
a displacement through its third reference parameter `offset` for the succeeding addi/jalr/ld/st instructions.
While `la` [1] is mainly used by `la`[2] for loading address of an label which is more simpler compared with `la_patchable`.
At the same time, I think `la_patchable` should receive a `address` instead of `Address` since it's always expecting a
literal address. This will also help distinguish with `la`[3] which receives an `Address`.

So the replacement for `la_patchable` looks like `void MacroAssembler::la(Register Rd, const address addr, int32_t &offset)`
And I have tried following add-on change and it seems to work.
[16816-addon.diff.txt](https://github.com/openjdk/jdk/files/13509737/16816-addon.diff.txt)


[1] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp#L720
[2] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp#L755
[3] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp#L730

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

PR Comment: https://git.openjdk.org/jdk/pull/16816#issuecomment-1833236921


More information about the hotspot-compiler-dev mailing list