RFR: 8320697: RISC-V: Small refactoring for runtime calls
Feilong Jiang
fjiang at openjdk.org
Tue Nov 28 09:52:09 UTC 2023
On Tue, 28 Nov 2023 09:46:21 GMT, Robbin Ehn <rehn at openjdk.org> wrote:
> > `Renamed MacroAssembler::la_patchable with MacroAssembler::auipc making it explicit that this will emit pc-relative addressing code.`
>
> >
>
> > First, above is not true, as it may emit movptr, it's now an implicit movptr instead. When you thought you were getting auipc. Secondly auipc is not a mnemonic AFAIK, it means just the instruction auipc. Having it as both is confusing. Third the one version is now called "la" while the other one is called "auipc" ?
>
> >
>
> > Sorry, but this part is worse now in IMHO.
>
>
>
> Yeah, I see that too :-) I guess the intention is to try to map to the aarch64 counterpart MacroAssembler::adrp [1].
>
> I did some history searching and seems that the else block is there only to handle some corner cases [2].
>
> We might want to find a better name for this function but seems not easy to do :-(
>
>
>
> [1] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp#L4968
>
> [2] https://mail.openjdk.org/pipermail/hotspot-compiler-dev/2015-November/019895.html
Thanks Fei.
Yes, that's also part of the reason why I rename this function to the new name auipc. The other consideration which I didn't mention is that the old name la_patchable is kind of confusing because we also have other la function which are also pachable too like MacroAssembler::la [1].
I am happy if we can find a better name, suggestions are welcomed.
[1] https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/riscv/macroAssembler_riscv.cpp#L730
-------------
PR Comment: https://git.openjdk.org/jdk/pull/16816#issuecomment-1829459361
More information about the hotspot-compiler-dev
mailing list