RFR: 8343242: RISC-V: Refactor materialization of literal address

[Robbin Ehn]

On Wed, 30 Oct 2024 00:28:16 GMT, Fei Yang <fyang at openjdk.org> wrote:

> Hi, please consider this refactoring work.
> 
> Currently, we have both `la(address)` and `la(Address)`. They both accept and materialize literal address.
> While `la(Address)` always emit a movptr sequence, `la(address)` is a bit complex: it checks whether the target is
> reachable from anywhere within the code cache (`is_32bit_offset_from_codecache`) and emits pc-relative auipc+addi
> pair or movptr sequence. This makes it not that obvious at places where we want a more accurate estimation about
> the size of code emitted like when we prepare code stubs like C2SafepointPollStub or C2EntryBarrierStub (See: https://github.com/openjdk/jdk/pull/21732).
> 
> I would suggest we keep `la(address)` simple and let it only emit auipc+addi pair, which will be consistent with the
> `RISC-V Assembly Programmer's Handbook`. I think It's more reasonable to move the distance check to `la(Address)`.
> Furthermore, I would also suggest that we make the distance check simpler. The approach taken here is to only check
> whether the target is inside the code cache. This way we will be more certain about code emitted with `la(Address)`
> as well. This will help keep the risk of this change low as all `la(Address)` callsites with literal address outside of code
> cache won't be affected. And I don't think the original distance check from code cache will benefit us much more.
> Eyeballed all `la(Address)` callsites, I think we are fine.
> 
> Benefits:
> 1. We have a simpler `la(address)` which is kind of spec compatible;
> 2. We are consistent in target distance check when doing `j(Address)`, `la(Address)`, `ld/st(Address)` and `rt_call`;
> 3. We don't need explicit use of `relocate` with lambda for most of the places;
> 
> Note that there are several places where we want explicit `movptr` sequence:
> 1. In `SignatureHandlerGenerator::generate()` where the emitted code is copied without relocation [1];
> 2. In `movoop` and `mov_metadata` where we want patching afterwards;
> 
> [1] https://github.com/openjdk/jdk/blob/master/src/hotspot/share/interpreter/interpreterRuntime.cpp#L1313
> 
> Testing on linux-riscv64:
> - [x] hotspot:tier1 (fastdebug)
> - [x] tier1-tier3, hotspot:tier4 (release)
> - [x] specjbb2015, dacapo, renaissance (release)

Did you run any micro benchmarks? Maybe skynet just to check VTs.
And did you do any measurement on code cache size?

I can't find anything that would matter, but I'll look a bit more.

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

PR Comment: https://git.openjdk.org/jdk/pull/21777#issuecomment-2449507657