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

Hamlin Li mli at openjdk.org
Mon Jun 3 08:19:08 UTC 2024


On Fri, 31 May 2024 14:40:15 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:
> 
>   adjust accessibility

> Hi Halim, this will cause merge hell for me in #19453.
> 
> You have a bunch of stuff in hpp files which can be moved to cpp file, to keep headers clean and not export so. E.g. check_movptr1_data_dependency is only used in macroAssembler_riscv.cpp, now everyone including MASM.hpp get copy of this method (which is compiled away). I.e. if you want it to be a member method move the definition to cpp and just keep the declaration in hpp.

I'm not sure.
If this is a problem in this patch, then in original implementation, MASM.hpp includes nativeInst.hpp, it will cause the similar issue?
In another hand, will unused methods in a header file still exist in the compiled binary?

The reason I do it this way is because I just want to refactor necessary things (the pricipal is to only nativeInst depends on macroAssembler, not in reverse direction), for other things, I try to keep it as original ones. So if there is other optimization opportunities, it's better to do it in separate pr's.

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

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


More information about the hotspot-dev mailing list