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

[Hamlin Li]

On Mon, 3 Jun 2024 10:48:09 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

> > 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?
> 
> It's about having readable header, havin long private, not part of API, method in header clobber it.

Not sure, as it's in its own section, not mixed with other sections.
But, it's fine, I'll move it to the bottom of the file.

> They will not exists, but compiling away methods is not free as it prolongs the compile time.

As I said, MASM.hpp included nativeInst.hpp before, so it's not a issue (or new issue)?

> 
> > 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.
> 
> If this is a simple move of code, then I suggest you wait until my patch, as that changes names and doubles the methods which you want to move.

It's just a merge, either in this pr or that pr, not that complicated?

> 
> Also now NativeCall::instruction_size, which is used in shared code is nativeInst, but NativeInstruction::instruction_size is (duplicated) in MASM. EDIT, better example:
> 
> ```
> CodeBuffer* PhaseOutput::init_buffer() {
> ...
>   int pad_req   = NativeCall::instruction_size;
> ```
> 
> So there is duplication if all size both in nativeInst and masm.

I don't get the point, can you clarify?

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

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