RFR: 8294492: RISC-V: Use li instead of patchable movptr at non-patchable callsites [v2]

[Fei Yang]

On Wed, 28 Sep 2022 06:58:15 GMT, Xiaolin Zheng <xlinzheng at openjdk.org> wrote:

>> A cleanup which can also reduce some code size. The main purpose is to separate the current mixed movptr usages: movptr's semantics allow it to be used for fixed-length patchable addresses. In the current backend several callsites which are not patchable use movptr (6 instructions) as well, but that's unnecessary for we can use li(1~6 instructions) to substitute them.
>> 
>> This patch extracts the last 12-bit offset from the li(), a recursive function, to merge it into the rest "jalr/ld/sd..." that can accept a 12-bit offset as an operand, same as the movptr_with_offset before.
>> 
>> Originally some non-patchable callsites:
>> 
>> ;; 0x401ab1dc40
>> 0x000000401372b324:   lui	t0,0x201
>> 0x000000401372b328:   addi	t0,t0,-680 # 0x0000000000200d58
>> 0x000000401372b32c:   slli	t0,t0,0xb
>> 0x000000401372b32e:   addi	t0,t0,1905
>> 0x000000401372b332:   slli	t0,t0,0x6
>> 0x000000401372b334:   jalr	t0
>> 
>> Now:
>> 
>> 0x000000401b2bd9f4:   lui	t0,0x4003
>> 0x000000401b2bd9f8:   addiw	t0,t0,-1275
>> 0x000000401b2bd9fc:   slli	t0,t0,0xc
>> 0x000000401b2bda00:   jalr	1080(t0) # 0x0000000004003438
>> 
>> Not so much, but the main purpose is to clarify the usage of movptr and li.
>> 
>> The first commit has gone through a hotspot tier1~tier4; the other ones are testing under a hotspot tier1.
>
> Xiaolin Zheng has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fix

I have several comments.

src/hotspot/cpu/riscv/assembler_riscv.cpp line 119:

> 117: }
> 118: 
> 119: void Assembler::li(Register Rd, int64_t imm, int32_t &offset) {

It looks to me that we don't need to introduce another 'li' function here. I think you can mask out the lower 12bits of the address and move the result to register with existing 'mv' function. And keep the lower 12bits in offset at the same time. This could be implemented in your new 'mv' function.

src/hotspot/cpu/riscv/assembler_riscv.cpp line 228:

> 226:   }
> 227: 
> 228:   INSN(call, x1);

Better to move functions like 'ret' and 'call' to macroAssembler_riscv.hpp

src/hotspot/cpu/riscv/macroAssembler_riscv.cpp line 567:

> 565:                                        Label *retaddr) {
> 566:   push_reg(RegSet::of(t0, xmethod), sp);   // push << t0 & xmethod >> to sp
> 567:   call(entry_point, t0);

Looks like we could use the default temp register 't0' for all use of 'call'.

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

Changes requested by fyang (Reviewer).

PR: https://git.openjdk.org/jdk/pull/10462