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

Hamlin Li mli at openjdk.org
Mon Jun 3 15:27:05 UTC 2024


On Mon, 3 Jun 2024 12:43:27 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

> I'm not disagreeing with your intent. I'm just saying I duplicating the constants is not good solution, and the code in some cases will be worse.

Thanks for clarifying, I get your point.
But still think we'd better have no dependency from macroAssembler to nativeInst. And I don't see issue in following code (in your example above)

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;

`MacroAssembler` should only know fields or methods in itself, not in nativeInst. 
If some code need to access both `MacroAssembler` and `nativeInst`, it should know both of them.
And, if must choose between bidirectional dependencies and some data duplication, I prefer the latter.

And in existing code of nativeInst, there are some kind of similar data duplication codes, e.g. : https://github.com/openjdk/jdk/blob/master/src/hotspot/cpu/riscv/nativeInst_riscv.hpp#L446

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

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


More information about the hotspot-dev mailing list