Re: [PATCH 0/2] target/riscv: Separate implicitly-enabled and explicitly-enabled extensions

[liweiwei]

On 2023/4/10 21:48, Daniel Henrique Barboza wrote:
> Hi,
>
> On 4/10/23 00:35, Weiwei Li wrote:
> > The patch tries to separate the multi-letter extensions that may 
> > implicitly-enabled by misa.EXT from the explicitly-enabled cases, so 
> > that the misa.EXT can truely disabled by write_misa().
> > With this separation, the implicitly-enabled zve64d/f and zve32f 
> > extensions will no work if we clear misa.V. And clear misa.V will 
> > have no effect on the explicitly-enalbed zve64d/f and zve32f extensions.
>
> For this particular case of write_misa() I'm not sure if we need all 
> that. If we want
> to grant user choice on write_misa(), let's say that the user wants to 
> enable/disable
> RVV, we can enable/disable all RVV related Z-extensions by hand. It's 
> just a matter
> of writing enable/disable code that write_misa() would use.
>
> In the end, write_misa() is also an user choice. If write_misa() wants 
> to disable RVV,
> this means that the user wants to disable RVV, so it doesn't matter 
> whether the user
> enabled zve32f on the command line or not - we disable zve32f as well. 
> Same thing for
> RVC and its related Z-extensions.

I just find we should also separate the cases here. Zcmp/Zcmt require Zca.

If we disable Zca when misa.C is cleared, whether we need disable 
Zcmp/Zcmt?

If yes, then we need another new parameter(to indicate they are diabled by

clearing misa.C ) to decide whether we should re-enable them when misa.C 
is set.

If not, we should make clearing misa.C failed in this case.

Regards,

Weiwei Li

> The reason why I didn't do this particular code for RVC and RVV is 
> because we have
> pending work in the ML that I would like to get it merged first. And 
> there's a few
> caveats we need to decide what to do, e.g. what if the user disables F 
> but V is
> enabled? Do we refuse write_misa()? Do we disable RVV?
>
>
> All this said, patch 1 is still a good addition to make it easier to 
> distinguish
> the Z-extensions we're enabling due to MISA bits. I believe we should use
> set_implicit_extensions_from_ext() in the future for all similar 
> situations.
>
>
>
> Thanks,
>
> Daniel
>
>
>
> > Weiwei Li (2):
> >    target/riscv: Add set_implicit_extensions_from_ext() function
> >    target/riscv: Add ext_z*_enabled for implicitly enabled extensions
> >
> >   target/riscv/cpu.c                      | 73 +++++++++++++++----------
> >   target/riscv/cpu.h                      |  8 +++
> >   target/riscv/cpu_helper.c               |  2 +-
> >   target/riscv/csr.c                      |  2 +-
> >   target/riscv/insn_trans/trans_rvd.c.inc |  2 +-
> >   target/riscv/insn_trans/trans_rvf.c.inc |  2 +-
> >   target/riscv/insn_trans/trans_rvi.c.inc |  5 +-
> >   target/riscv/insn_trans/trans_rvv.c.inc | 16 +++---
> >   target/riscv/translate.c                |  4 +-
> >   9 files changed, 68 insertions(+), 46 deletions(-)