RFR: 8343242: RISC-V: Refactor materialization of literal address [v2]

[Robbin Ehn]

On Fri, 1 Nov 2024 01:10:11 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 simply 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)
>
> Fei Yang has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits:
> 
>  - Merge remote-tracking branch 'upstream/master' into JDK-8343242
>  - 8343242: RISC-V: Refactor materialization of literal address

Looks good, thanks!

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

Marked as reviewed by rehn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/21777#pullrequestreview-2409869937