RFR: 8332900: RISC-V: refactor nativeInst_riscv.cpp and macroAssembler_riscv.cpp [v3]
Hamlin Li
mli at openjdk.org
Fri May 31 07:13:03 UTC 2024
On Wed, 29 May 2024 18:54:27 GMT, Hamlin Li <mli at openjdk.org> wrote:
>> Hi,
>> Can you help to review the patch?
>> Currently, code in nativeInst_riscv.cpp and macroAssembler_riscv.cpp call each other, which is not right for readability and maintainance.
>> After refactoring, basically only code in nativeInst_riscv.cpp calls code in macroAssembler_riscv.cpp, but not in reverse direction.
>>
>> Thanks!
>>
>> * Tests are still running, so far so good.
>
> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
>
> restrict accessbility
I'm not sure, maybe because you're already familiar with current code, but I have a different opinion.
The only uses of `extract_rs1` are in static functions in `nativeInst_riscv.hpp`, so it's just a utility function, and does not matter where it's put.
But, e.g. things related to movptr[1/2] are currently separated in nativeInst_riscv and macroAssembler_riscv, which is not right, in one side it brings no benefit for reading the code, in the other side when you modify some code you need do code change at both files, https://github.com/openjdk/jdk/pull/19246 is an example.
The only things must be in nativeInst_riscv are the interfaces/fuctions it exposes to outside, but some implementation is not necessary to be in it, and it's better to be put in macroAssembler_riscv, as they're tightly coupled (e.g. movptr things).
-------------
PR Comment: https://git.openjdk.org/jdk/pull/19459#issuecomment-2141363282
More information about the hotspot-dev
mailing list