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

[Daniel Henrique Barboza]

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.

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(-)