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

[Fei Yang]

On Fri, 31 May 2024 08:43:17 GMT, Hamlin Li <mli at openjdk.org> wrote:

> As I said, we can refer to other platforms in case they're well implemented. But, I see no benefit and reason to put e.g. `movptr1` and `patch_addr_in_movptr1` in macroAssembler and `is_movptr1_at` in nativeInst, I can not find the rational to do so, just because aarch64 do the similar thing? seems that's not a good reason; if it is, then how about other platforms, do they all follow aarch64 implementation details? Anyway, it's an implementation detail, we don't have to follow other platforms unless it's a good design choice for riscv too.

Apparently that's not what I mean. I just want say that it's a commonly adopted approach by other CPUs not just aarch64 and I am just fine with it. But your opinion from a OO design perspective does make sense to me. Anyway, here might be a compromise for us when I see you still kept following interfaces for `NativeInstruction`: 

  bool is_jal()                             const { return MacroAssembler::is_jal_at(addr_at(0));         }
  bool is_movptr()                          const { return MacroAssembler::is_movptr1_at(addr_at(0)) ||
                                                           MacroAssembler::is_movptr2_at(addr_at(0));     }
  bool is_movptr1()                         const { return MacroAssembler::is_movptr1_at(addr_at(0));     }
  bool is_movptr2()                         const { return MacroAssembler::is_movptr2_at(addr_at(0));     }
  bool is_auipc()                           const { return MacroAssembler::is_auipc_at(addr_at(0));       }
  bool is_call()                            const { return MacroAssembler::is_call_at(addr_at(0));        }
  bool is_jump()                            const { return MacroAssembler::is_jump_at(addr_at(0));        }


Can you do similar thing for other interfaces like `MacroAssembler::is_li16u_at`? Like `NativeInstruction::is_li16u` which delegates work to `MacroAssembler::is_li16u_at`. And we use these interfaces from `NativeInstruction` where appropriate at the callsites like in file src/hotspot/cpu/riscv/gc/z/zBarrierSetAssembler_riscv.cpp. Hopefully, the code will be more unified in style and resolves both of our concerns.

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

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