[riscv-port] RFR: 8278322: riscv: Support RVC: compressed instructions
Xiaolin Zheng
xlinzheng at openjdk.java.net
Thu Dec 9 14:23:35 UTC 2021
On Thu, 9 Dec 2021 13:51:41 GMT, Yadong Wang <yadongwang at openjdk.org> wrote:
> > > > Thank you for your explanation, Felix. :-) Seems we finally reach a consensus now.
> > > > > I guess maybe the best places to do this would be C1 and C2.
> > > >
> > > >
> > > > I think I could get your meaning this time.
> > > > > I think we can then further consider the "CompressibleRegion" solution suggested by Yadong.
> > > >
> > > >
> > > > Yadong's plan works for me. I am quite in favor of his strategy.
> > >
> > >
> > > I am happy that we are agreed on this solution ;-) We need to be safe at this very early stage, so I would suggest UseRVC disabled by default like UseRVV even if the hardware RVC extension is there. I hope you could understand that. In the future, we will consider auto-enable UseRVC and UseRVV after enough testing and evaluation.
> > > > I will implement this approach ASAP. Maybe next week I might open another PR including this implementation and the definition of compressed instructions presented in the `assembler_riscv_cext.hpp` part for a better review. I wonder if this is okay for you.
> > > > I am going to close this PR, save it for future reference, and may open another PR and start from scratch. Before that, just one more question: would you mind my keeping definitions of C-Ext instructions into one separate file like `assembler_riscv_cext.hpp`, as the current implementation? I think we might better keep different RISCV extensions into different files.
> > >
> > >
> > > Yes, I think `assembler_riscv_c.hpp` will do here.
> >
> >
> > Totally agree -- Thanks again for your reviews. Gonna close this PR and the related issue.
>
> @zhengxiaolinX I think a seperate file for rvc is a good choice, and maybe we should do the same thing for rvv later. By the way, the Open/Closed principle may be suitable for your design. We can use a RVC_Assembler extend the Assembler, override all instructions that may be compressed, and then deliver to the super Assembler if the "compression" failed. Maybe you can swap the context easier when entering or leaving the "CompressedRegion". Just for suggestion, and thank you for your big effort.
Thanks for you suggestion, Yadong. In fact I nearly have the same consideration about this. RISC-V is a modularized ISA with pluggable extensions. Now that extensions will be supported one by one, I think the codebase will inevitably full of lots of unreadable `if (A-Ext enabed)...else... if (B-Ext enabled)...else...` unified in one VM in order to handle all possible cases. The runtime costs of performance will graduately get bigger and bigger as time goes. So we might do the same modularized things in the Assembler as your words from the beginning to prevent regrets, for at last code might be extremely difficult to get refactored. I have noticed that at every instruction I need to check one `UseRVC` now, but the workload might be so big to change that so I didn't determine if I should do that. After your words, I realize if we are thinking about the same thing, we might start to consider changes from the beginning. Thanks again for your advice and I will take this into a careful co
nsideration.
-------------
PR: https://git.openjdk.java.net/riscv-port/pull/24
More information about the riscv-port-dev
mailing list