RFR: 8332900: RISC-V: refactor nativeInst_riscv.cpp and macroAssembler_riscv.cpp [v3]
Fei Yang
fyang at openjdk.org
Fri May 31 01:15:01 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
TBH, I don't think this is a change in the right direction. Classes like `NativeInstruction` and `MacroAssembler` are supposed to be there for different purposes, so I think it's not a issue for them to call each other where necessary. It's looks really strange to me to move functions like `extract_rs1` from `NativeInstruction` to `MacroAssembler`. It's a thing which is supposed to be in the domain of class `NativeInstruction`.
-------------
PR Review: https://git.openjdk.org/jdk/pull/19459#pullrequestreview-2089652180
More information about the hotspot-dev
mailing list