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

[Fei Yang]

On Thu, 31 Oct 2024 10:18:24 GMT, Robbin Ehn <rehn 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)
>
> 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.

@robehn @feilongjiang : Thanks for the review! I also tried NetBeans and Eclipse. Both IDEs work fine with this change. Let's move on.

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

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