RFR: 8332689: RISC-V: Use load instead of trampolines [v11]

[Fei Yang]

On Thu, 13 Jun 2024 17:26:35 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>> Hi all, please consider!
>> 
>> Today we do JAL to **dest** if **dest** is in reach (+/- 1 MB).
>> Using a very small application or running very short time we have fast patchable calls.
>> But any normal application running longer will increase the code size and code chrun/fragmentation.
>> So whatever or not you get hot fast calls rely on luck.
>> 
>> To be patchable and get code cache reach we also emit a stub trampoline which we can point the JAL to.
>> This would be the common case for a patchable call.
>> 
>> Code stream:
>> JAL <trampo>
>> Stubs:
>> AUIPC
>> LD
>> JALR
>> <DEST>
>> 
>> 
>> On some CPUs L1D and L1I can't contain the same cache line, which means the tramopline stub can bounce from L1I->L1D->L1I, which is expensive.
>> Even if you don't have that problem having a call to a jump is not the fastest way.
>> Loading the address avoids the pitsfalls of cmodx.
>> 
>> This patch suggest to solve the problems with trampolines, we take small penalty in the naive case of JAL to **dest**,
>> and instead do by default:
>> 
>> Code stream:
>> AUIPC
>> LD
>> JALR
>> Stubs:
>> <DEST>
>> 
>> An experimental option for turning trampolines back on exists.
>> 
>> It should be possible to enhanced this with the WIP [Zjid](https://github.com/riscv/riscv-j-extension) by changing the JALR to JAL and nop out the auipc+ld (as the current proposal of Zjid forces the I-fetcher to fetch instruction in order (meaning we will avoid a lot issues which arm has)) when in reach and vice-versa.
>> 
>> Numbers from VF2 (I have done them a few times, they are always overall in favor of this patch):
>> 
>> fop                                        (msec)    2239       |  2128       =  0.950424
>> h2                                         (msec)    18660      |  16594      =  0.889282
>> jython                                     (msec)    22022      |  21925      =  0.995595
>> luindex                                    (msec)    2866       |  2842       =  0.991626
>> lusearch                                   (msec)    4108       |  4311       =  1.04942
>> lusearch-fix                               (msec)    4406       |  4116       =  0.934181
>> pmd                                        (msec)    5976       |  5897       =  0.98678
>> jython                                     (msec)    22022      |  21925      =  0.995595
>> Avg:                                       0.974112                              
>> fop(xcomp)                                 (msec)    2721       |  2714       =  0.997427
>> h2(xcomp) ...
>
> Robbin Ehn has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 16 commits:
> 
>  - Merge branch 'master' into 8332689
>  - Merge branch 'master' into 8332689
>  - Merge branch 'master' into 8332689
>  - Merge branch 'master' into 8332689
>  - Remove tmp file
>  - Prepare for dynamic NativeCall size
>  - Only allow one calling convetion, i.e. fixed sized
>  - Merge branch 'master' into 8332689
>  - Review comments
>  - Move shart/far code to cpp
>  - ... and 6 more: https://git.openjdk.org/jdk/compare/5d2a19de...bb7249b8

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

> 3730: // Maybe emit a call via a trampoline. If the code cache is small
> 3731: // trampolines won't be emitted.
> 3732: address MacroAssembler::patchable_far_call(Address entry) {

It doesn't look nice to me for `UseTrampolines` checks to be spread across this `MacroAssembler::patchable_far_call` function. I would suggest to keep the original `MacroAssembler::trampoline_call` and let `MacroAssembler::patchable_far_call` delegate work to it under `UseTrampolines`. What do you think?

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

> 4640:       }
> 4641:     } else {
> 4642:       rt_call(zero_blocks.target(), t0);

Maybe simply: `rt_call(zero_blocks.target());` as `t0` is the default temp register for `rt_call`.

src/hotspot/cpu/riscv/macroAssembler_riscv.hpp line 1201:

> 1199:   //
> 1200:   // Old patchable far calls: (-XX:+UseTrampolines)
> 1201:   //   - trampoline call:

How about combine the two lines? Like:
`- trampoline call (old patchable far call / -XX:+UseTrampolines):`

src/hotspot/cpu/riscv/macroAssembler_riscv.hpp line 1240:

> 1238: 
> 1239:   // Emit a direct call if the entry address will always be in range,
> 1240:   // otherwise a patachable far call.

s/patachable/patchable/

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19453#discussion_r1639360499
PR Review Comment: https://git.openjdk.org/jdk/pull/19453#discussion_r1639349761
PR Review Comment: https://git.openjdk.org/jdk/pull/19453#discussion_r1639337696
PR Review Comment: https://git.openjdk.org/jdk/pull/19453#discussion_r1639330072