[v1] target/riscv: Remove experimental prefix from "B" extension

[PATCH] target/riscv: Remove experimental prefix from "B" extension

Posted by Rob Bradford 6 months, 3 weeks ago

This extension has now been ratified:
https://jira.riscv.org/browse/RVS-2006 so the "x-" prefix can be
removed.

Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
---
 target/riscv/cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index eb1a2e7d6d..861d9f4350 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1396,7 +1396,7 @@ static const MISAExtInfo misa_ext_info_arr[] = {
     MISA_EXT_INFO(RVJ, "x-j", "Dynamic translated languages"),
     MISA_EXT_INFO(RVV, "v", "Vector operations"),
     MISA_EXT_INFO(RVG, "g", "General purpose (IMAFD_Zicsr_Zifencei)"),
-    MISA_EXT_INFO(RVB, "x-b", "Bit manipulation (Zba_Zbb_Zbs)")
+    MISA_EXT_INFO(RVB, "b", "Bit manipulation (Zba_Zbb_Zbs)")
 };
 
 static void riscv_cpu_validate_misa_mxl(RISCVCPUClass *mcc)
-- 
2.44.0

Re: [PATCH] target/riscv: Remove experimental prefix from "B" extension

Posted by Alistair Francis 6 months, 2 weeks ago

On Tue, May 7, 2024 at 8:28 PM Rob Bradford <rbradford@rivosinc.com> wrote:
>
> This extension has now been ratified:
> https://jira.riscv.org/browse/RVS-2006 so the "x-" prefix can be
> removed.
>
> Signed-off-by: Rob Bradford <rbradford@rivosinc.com>

Just to be clear, do you mind enabling RVB as part of
riscv_init_max_cpu_extensions() as part of this patch

Alistair

> ---
>  target/riscv/cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index eb1a2e7d6d..861d9f4350 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1396,7 +1396,7 @@ static const MISAExtInfo misa_ext_info_arr[] = {
>      MISA_EXT_INFO(RVJ, "x-j", "Dynamic translated languages"),
>      MISA_EXT_INFO(RVV, "v", "Vector operations"),
>      MISA_EXT_INFO(RVG, "g", "General purpose (IMAFD_Zicsr_Zifencei)"),
> -    MISA_EXT_INFO(RVB, "x-b", "Bit manipulation (Zba_Zbb_Zbs)")
> +    MISA_EXT_INFO(RVB, "b", "Bit manipulation (Zba_Zbb_Zbs)")
>  };
>
>  static void riscv_cpu_validate_misa_mxl(RISCVCPUClass *mcc)
> --
> 2.44.0
>
>

Re: [PATCH] target/riscv: Remove experimental prefix from "B" extension

Posted by Andrew Jones 6 months, 2 weeks ago

On Tue, May 07, 2024 at 11:27:21AM GMT, Rob Bradford wrote:
> This extension has now been ratified:
> https://jira.riscv.org/browse/RVS-2006 so the "x-" prefix can be
> removed.
> 
> Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
> ---
>  target/riscv/cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index eb1a2e7d6d..861d9f4350 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -1396,7 +1396,7 @@ static const MISAExtInfo misa_ext_info_arr[] = {
>      MISA_EXT_INFO(RVJ, "x-j", "Dynamic translated languages"),
>      MISA_EXT_INFO(RVV, "v", "Vector operations"),
>      MISA_EXT_INFO(RVG, "g", "General purpose (IMAFD_Zicsr_Zifencei)"),
> -    MISA_EXT_INFO(RVB, "x-b", "Bit manipulation (Zba_Zbb_Zbs)")
> +    MISA_EXT_INFO(RVB, "b", "Bit manipulation (Zba_Zbb_Zbs)")
>  };
>  
>  static void riscv_cpu_validate_misa_mxl(RISCVCPUClass *mcc)
> -- 
> 2.44.0
> 
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

I think we should also either change the false to true for RVB in
misa_ext_cfgs[] or at least ensure RVB is set for the 'max' cpu
type in riscv_init_max_cpu_extensions().

Thanks,
drew

Re: [PATCH] target/riscv: Remove experimental prefix from "B" extension

Posted by Daniel Henrique Barboza 6 months, 2 weeks ago


On 5/8/24 08:22, Andrew Jones wrote:
> On Tue, May 07, 2024 at 11:27:21AM GMT, Rob Bradford wrote:
>> This extension has now been ratified:
>> https://jira.riscv.org/browse/RVS-2006 so the "x-" prefix can be
>> removed.
>>
>> Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
>> ---
>>   target/riscv/cpu.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index eb1a2e7d6d..861d9f4350 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -1396,7 +1396,7 @@ static const MISAExtInfo misa_ext_info_arr[] = {
>>       MISA_EXT_INFO(RVJ, "x-j", "Dynamic translated languages"),
>>       MISA_EXT_INFO(RVV, "v", "Vector operations"),
>>       MISA_EXT_INFO(RVG, "g", "General purpose (IMAFD_Zicsr_Zifencei)"),
>> -    MISA_EXT_INFO(RVB, "x-b", "Bit manipulation (Zba_Zbb_Zbs)")
>> +    MISA_EXT_INFO(RVB, "b", "Bit manipulation (Zba_Zbb_Zbs)")
>>   };
>>   
>>   static void riscv_cpu_validate_misa_mxl(RISCVCPUClass *mcc)
>> -- 
>> 2.44.0
>>
>>
> 
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> 
> I think we should also either change the false to true for RVB in
> misa_ext_cfgs[] or at least ensure RVB is set for the 'max' cpu
> type in riscv_init_max_cpu_extensions().

I prefer if we keep misa_ext_cfgs[] as is. Changing the defaults in this array
will also change the defaults for rv64. IMO we should enable RVB manually in
riscv_init_max_cpu_extensions().

We already have some precedence for it: RVV is enabled in 'max' while is default
'false' for rv64.


Thanks,

Daniel


> 
> Thanks,
> drew

Re: [PATCH] target/riscv: Remove experimental prefix from "B" extension

Posted by Andrew Jones 6 months, 2 weeks ago

On Thu, May 09, 2024 at 02:23:42PM GMT, Daniel Henrique Barboza wrote:
> 
> 
> On 5/8/24 08:22, Andrew Jones wrote:
> > On Tue, May 07, 2024 at 11:27:21AM GMT, Rob Bradford wrote:
> > > This extension has now been ratified:
> > > https://jira.riscv.org/browse/RVS-2006 so the "x-" prefix can be
> > > removed.
> > > 
> > > Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
> > > ---
> > >   target/riscv/cpu.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > > index eb1a2e7d6d..861d9f4350 100644
> > > --- a/target/riscv/cpu.c
> > > +++ b/target/riscv/cpu.c
> > > @@ -1396,7 +1396,7 @@ static const MISAExtInfo misa_ext_info_arr[] = {
> > >       MISA_EXT_INFO(RVJ, "x-j", "Dynamic translated languages"),
> > >       MISA_EXT_INFO(RVV, "v", "Vector operations"),
> > >       MISA_EXT_INFO(RVG, "g", "General purpose (IMAFD_Zicsr_Zifencei)"),
> > > -    MISA_EXT_INFO(RVB, "x-b", "Bit manipulation (Zba_Zbb_Zbs)")
> > > +    MISA_EXT_INFO(RVB, "b", "Bit manipulation (Zba_Zbb_Zbs)")
> > >   };
> > >   static void riscv_cpu_validate_misa_mxl(RISCVCPUClass *mcc)
> > > -- 
> > > 2.44.0
> > > 
> > > 
> > 
> > Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> > 
> > I think we should also either change the false to true for RVB in
> > misa_ext_cfgs[] or at least ensure RVB is set for the 'max' cpu
> > type in riscv_init_max_cpu_extensions().
> 
> I prefer if we keep misa_ext_cfgs[] as is. Changing the defaults in this array
> will also change the defaults for rv64. IMO we should enable RVB manually in
> riscv_init_max_cpu_extensions().
> 
> We already have some precedence for it: RVV is enabled in 'max' while is default
> 'false' for rv64.

But do we care if rv64 gets B? rv64 doesn't have any particular set of
extensions, afaik. And B seems like it should be generally adopted enough
to be in a "general" cpu type like rv64. Anyway, either way works for me
as long as 'max' gets B one way or another.

Thanks,
drew

Re: [PATCH] target/riscv: Remove experimental prefix from "B" extension

Posted by Daniel Henrique Barboza 6 months, 2 weeks ago


On 5/10/24 05:29, Andrew Jones wrote:
> On Thu, May 09, 2024 at 02:23:42PM GMT, Daniel Henrique Barboza wrote:
>>
>>
>> On 5/8/24 08:22, Andrew Jones wrote:
>>> On Tue, May 07, 2024 at 11:27:21AM GMT, Rob Bradford wrote:
>>>> This extension has now been ratified:
>>>> https://jira.riscv.org/browse/RVS-2006 so the "x-" prefix can be
>>>> removed.
>>>>
>>>> Signed-off-by: Rob Bradford <rbradford@rivosinc.com>
>>>> ---
>>>>    target/riscv/cpu.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>>>> index eb1a2e7d6d..861d9f4350 100644
>>>> --- a/target/riscv/cpu.c
>>>> +++ b/target/riscv/cpu.c
>>>> @@ -1396,7 +1396,7 @@ static const MISAExtInfo misa_ext_info_arr[] = {
>>>>        MISA_EXT_INFO(RVJ, "x-j", "Dynamic translated languages"),
>>>>        MISA_EXT_INFO(RVV, "v", "Vector operations"),
>>>>        MISA_EXT_INFO(RVG, "g", "General purpose (IMAFD_Zicsr_Zifencei)"),
>>>> -    MISA_EXT_INFO(RVB, "x-b", "Bit manipulation (Zba_Zbb_Zbs)")
>>>> +    MISA_EXT_INFO(RVB, "b", "Bit manipulation (Zba_Zbb_Zbs)")
>>>>    };
>>>>    static void riscv_cpu_validate_misa_mxl(RISCVCPUClass *mcc)
>>>> -- 
>>>> 2.44.0
>>>>
>>>>
>>>
>>> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
>>>
>>> I think we should also either change the false to true for RVB in
>>> misa_ext_cfgs[] or at least ensure RVB is set for the 'max' cpu
>>> type in riscv_init_max_cpu_extensions().
>>
>> I prefer if we keep misa_ext_cfgs[] as is. Changing the defaults in this array
>> will also change the defaults for rv64. IMO we should enable RVB manually in
>> riscv_init_max_cpu_extensions().
>>
>> We already have some precedence for it: RVV is enabled in 'max' while is default
>> 'false' for rv64.
> 
> But do we care if rv64 gets B? rv64 doesn't have any particular set of
> extensions, afaik. And B seems like it should be generally adopted enough
> to be in a "general" cpu type like rv64. Anyway, either way works for me
> as long as 'max' gets B one way or another.

Yes, as long as we enable it in 'max' it's good enough for this patch.

It's not like we're enabling extensions in rv64 just because they were ratified
(e.g. RVV isn't enabled in rv64), so even if we want to enable RVB in rv64 I'd
rather do it in a separated patch with a proper justification.


Thanks,

Daniel

> 
> Thanks,
> drew