RFR: 8332900: RISC-V: refactor nativeInst_riscv.cpp and macroAssembler_riscv.cpp [v4]

[Hamlin Li]

On Mon, 3 Jun 2024 11:59:38 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

> > I don't get the point, can you clarify?
> 
> I think don't de-coupling by duplicating the constant is better than keeping the depedancy. As long as the shared code expects the NativeXXX::instruction_size to work we can't move them.
> 
> E.g. (partly from my patch) nativeInst.cpp
> 
> ```
> if (NativeShortCall::is_at(addr)) {
>    return NativeShortCall::instruction_size;
> } else if (NativeFarCall::is_at(addr)) {
>    return NativeFarCall::instruction_size;
> }
> ```
> 
> In masm related code:
> 
> ```
> int MacroAssembler::max_patchable_far_call_stub_size() {                          
>   // Max stub size: alignment nop, TrampolineStub.
>   if (UseTrampolines) {
>     return NativeInstruction::instruction_size + NativeShortCall::trampoline_size;
> ```
> 
> This would now look like:
> 
> ```
> int MacroAssembler::max_patchable_far_call_stub_size() {                          
>   // Max stub size: alignment nop, TrampolineStub.
>   if (UseTrampolines) {
>     return Masm::instruction_size + Masm::native_short_call_trampoline_size;
> ```
> 
> Using different constant here is not an improvement.

What I want is to let MASM have no dependency on nativeInst, that's one of the principals I stick to in this pr, as dependency on nativeInst from MASM makes no sense from the point of view of code logic, and bidirectional dependency makes code dependency complicated which will also lead to issues of readability and maintainance. And if you are going to do it, I hope to do it thoroughly.

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

PR Comment: https://git.openjdk.org/jdk/pull/19459#issuecomment-2145035926