Re: Re: Re: [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension
Rob Bradford wrote: I'm using table 27.1 in the published PDF which is the PDF in this release: https://github.com/riscv/riscv-isa-manual/releases/tag/Ratified-IMAFDQC It looks like it was removed in this commit (which is a set of backports): We would retain the previously documented canonical order with B between C and P and that table updated on ratification. regards ved
Re: Re: Re: [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension
On Fri, 2024-01-12 at 17:08 +0100, Andrew Jones wrote: > On Thu, Jan 11, 2024 at 03:17:25PM +, Rob Bradford wrote: > > + Ved > > > > On Thu, 2024-01-11 at 14:14 +0100, Andrew Jones wrote: > > > On Thu, Jan 11, 2024 at 02:07:34PM +0100, Andrew Jones wrote: > > > > On Tue, Jan 09, 2024 at 05:07:35PM +, Rob Bradford wrote: > > > > > Add the infrastructure for the 'B' extension which is the > > > > > union > > > > > of the > > > > > Zba, Zbb and Zbs instructions. > > > > > > > > > > Signed-off-by: Rob Bradford > > > > > --- > > > > > target/riscv/cpu.c | 5 +++-- > > > > > target/riscv/cpu.h | 1 + > > > > > target/riscv/tcg/tcg-cpu.c | 1 + > > > > > 3 files changed, 5 insertions(+), 2 deletions(-) > > > > > > > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > > > > > index b07a76ef6b..22f8e527ff 100644 > > > > > --- a/target/riscv/cpu.c > > > > > +++ b/target/riscv/cpu.c > > > > > @@ -38,9 +38,9 @@ > > > > > #include "tcg/tcg.h" > > > > > > > > > > /* RISC-V CPU definitions */ > > > > > -static const char riscv_single_letter_exts[] = > > > > > "IEMAFDQCPVH"; > > > > > +static const char riscv_single_letter_exts[] = > > > > > "IEMAFDQCBPVH"; > > > > > > > > Is there a corresponding proposed change to table 29.1 of the > > > > nonpriv spec > > > > which states B comes after C and before P? If so, can you > > > > provide a > > > > link > > > > to it? Otherwise, how do we know that? > > > > > > Oh, I see. The unpriv spec B chapter comes after the C chapter > > > (and > > > before > > > J, P, ...). I still wonder if we'll have a 29.1 table update with > > > the > > > ratification of this extension though. > > > > > > > > > > I agree it's a bit confusing - but the order is established by the > > table in the unprivileged spec and the table explanation also makes > > this clear. > > > > """ > > Table 27.1: Standard ISA extension names. The table also defines > > the > > canonical order in which > > extension names must appear in the name string, with top-to-bottom > > in > > table indicating first-to-last > > in the name string, e.g., RV32IMACV is legal, whereas RV32IMAVC is > > not. > > """ > > Yes, this is the table I was referring to when I referenced "table > 29.1 of > the nonpriv spec". Since there's a chance I was looking at too old a > spec > I've now gone straight to the source, > > https://github.com/riscv/riscv-isa-manual/blob/main/src/naming.adoc > > but I still don't see B there. Do you see B in the table you're > looking > at? > > > > > The proposed B specification does not make any remarks about the > > ordering in the ISA definition string. [1] I would worry there > > would be > > a lot of software churn if this ordering were to be changed. > > The ordering shouldn't change, but I can't see where it's documented > (beyond the B chapter coming after the C chapter). I'm using table 27.1 in the published PDF which is the PDF in this release: https://github.com/riscv/riscv-isa-manual/releases/tag/Ratified-IMAFDQC It looks like it was removed in this commit (which is a set of backports): https://github.com/riscv/riscv-isa-manual/commit/6ecd735338241583d53396b7065eab7c9ace68aa Cheers, Rob > > Thanks, > drew
Re: Re: Re: [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension
On Thu, Jan 11, 2024 at 03:17:25PM +, Rob Bradford wrote: > + Ved > > On Thu, 2024-01-11 at 14:14 +0100, Andrew Jones wrote: > > On Thu, Jan 11, 2024 at 02:07:34PM +0100, Andrew Jones wrote: > > > On Tue, Jan 09, 2024 at 05:07:35PM +, Rob Bradford wrote: > > > > Add the infrastructure for the 'B' extension which is the union > > > > of the > > > > Zba, Zbb and Zbs instructions. > > > > > > > > Signed-off-by: Rob Bradford > > > > --- > > > > target/riscv/cpu.c | 5 +++-- > > > > target/riscv/cpu.h | 1 + > > > > target/riscv/tcg/tcg-cpu.c | 1 + > > > > 3 files changed, 5 insertions(+), 2 deletions(-) > > > > > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > > > > index b07a76ef6b..22f8e527ff 100644 > > > > --- a/target/riscv/cpu.c > > > > +++ b/target/riscv/cpu.c > > > > @@ -38,9 +38,9 @@ > > > > #include "tcg/tcg.h" > > > > > > > > /* RISC-V CPU definitions */ > > > > -static const char riscv_single_letter_exts[] = "IEMAFDQCPVH"; > > > > +static const char riscv_single_letter_exts[] = "IEMAFDQCBPVH"; > > > > > > Is there a corresponding proposed change to table 29.1 of the > > > nonpriv spec > > > which states B comes after C and before P? If so, can you provide a > > > link > > > to it? Otherwise, how do we know that? > > > > Oh, I see. The unpriv spec B chapter comes after the C chapter (and > > before > > J, P, ...). I still wonder if we'll have a 29.1 table update with the > > ratification of this extension though. > > > > > > I agree it's a bit confusing - but the order is established by the > table in the unprivileged spec and the table explanation also makes > this clear. > > """ > Table 27.1: Standard ISA extension names. The table also defines the > canonical order in which > extension names must appear in the name string, with top-to-bottom in > table indicating first-to-last > in the name string, e.g., RV32IMACV is legal, whereas RV32IMAVC is not. > """ Yes, this is the table I was referring to when I referenced "table 29.1 of the nonpriv spec". Since there's a chance I was looking at too old a spec I've now gone straight to the source, https://github.com/riscv/riscv-isa-manual/blob/main/src/naming.adoc but I still don't see B there. Do you see B in the table you're looking at? > > The proposed B specification does not make any remarks about the > ordering in the ISA definition string. [1] I would worry there would be > a lot of software churn if this ordering were to be changed. The ordering shouldn't change, but I can't see where it's documented (beyond the B chapter coming after the C chapter). Thanks, drew
Re: Re: [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension
+ Ved On Thu, 2024-01-11 at 14:14 +0100, Andrew Jones wrote: > On Thu, Jan 11, 2024 at 02:07:34PM +0100, Andrew Jones wrote: > > On Tue, Jan 09, 2024 at 05:07:35PM +, Rob Bradford wrote: > > > Add the infrastructure for the 'B' extension which is the union > > > of the > > > Zba, Zbb and Zbs instructions. > > > > > > Signed-off-by: Rob Bradford > > > --- > > > target/riscv/cpu.c | 5 +++-- > > > target/riscv/cpu.h | 1 + > > > target/riscv/tcg/tcg-cpu.c | 1 + > > > 3 files changed, 5 insertions(+), 2 deletions(-) > > > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > > > index b07a76ef6b..22f8e527ff 100644 > > > --- a/target/riscv/cpu.c > > > +++ b/target/riscv/cpu.c > > > @@ -38,9 +38,9 @@ > > > #include "tcg/tcg.h" > > > > > > /* RISC-V CPU definitions */ > > > -static const char riscv_single_letter_exts[] = "IEMAFDQCPVH"; > > > +static const char riscv_single_letter_exts[] = "IEMAFDQCBPVH"; > > > > Is there a corresponding proposed change to table 29.1 of the > > nonpriv spec > > which states B comes after C and before P? If so, can you provide a > > link > > to it? Otherwise, how do we know that? > > Oh, I see. The unpriv spec B chapter comes after the C chapter (and > before > J, P, ...). I still wonder if we'll have a 29.1 table update with the > ratification of this extension though. > > I agree it's a bit confusing - but the order is established by the table in the unprivileged spec and the table explanation also makes this clear. """ Table 27.1: Standard ISA extension names. The table also defines the canonical order in which extension names must appear in the name string, with top-to-bottom in table indicating first-to-last in the name string, e.g., RV32IMACV is legal, whereas RV32IMAVC is not. """ The proposed B specification does not make any remarks about the ordering in the ISA definition string. [1] I would worry there would be a lot of software churn if this ordering were to be changed. Cheers, Rob > Thanks, > drew [1] - https://github.com/riscv/riscv-b
Re: [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension
On Tue, Jan 09, 2024 at 05:07:35PM +, Rob Bradford wrote:
> Add the infrastructure for the 'B' extension which is the union of the
> Zba, Zbb and Zbs instructions.
>
> Signed-off-by: Rob Bradford
> ---
> target/riscv/cpu.c | 5 +++--
> target/riscv/cpu.h | 1 +
> target/riscv/tcg/tcg-cpu.c | 1 +
> 3 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index b07a76ef6b..22f8e527ff 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -38,9 +38,9 @@
> #include "tcg/tcg.h"
>
> /* RISC-V CPU definitions */
> -static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
> +static const char riscv_single_letter_exts[] = "IEMAFDQCBPVH";
> const uint32_t misa_bits[] = {RVI, RVE, RVM, RVA, RVF, RVD, RVV,
> - RVC, RVS, RVU, RVH, RVJ, RVG, 0};
> + RVC, RVS, RVU, RVH, RVJ, RVG, RVB, 0};
>
> /*
> * From vector_helper.c
> @@ -1251,6 +1251,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)")
> };
>
> static int riscv_validate_misa_info_idx(uint32_t bit)
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 2725528bb5..756a345513 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -69,6 +69,7 @@ typedef struct CPUArchState CPURISCVState;
> #define RVH RV('H')
> #define RVJ RV('J')
> #define RVG RV('G')
> +#define RVB RV('B')
>
> extern const uint32_t misa_bits[];
> const char *riscv_get_misa_ext_name(uint32_t bit);
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 8a35683a34..fda54671d5 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -791,6 +791,7 @@ static const RISCVCPUMisaExtConfig misa_ext_cfgs[] = {
> MISA_CFG(RVJ, false),
> MISA_CFG(RVV, false),
> MISA_CFG(RVG, false),
> +MISA_CFG(RVB, false)
> };
>
> /*
> --
> 2.43.0
>
>
Reviewed-by: Andrew Jones
Re: Re: [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension
On Thu, Jan 11, 2024 at 02:07:34PM +0100, Andrew Jones wrote: > On Tue, Jan 09, 2024 at 05:07:35PM +, Rob Bradford wrote: > > Add the infrastructure for the 'B' extension which is the union of the > > Zba, Zbb and Zbs instructions. > > > > Signed-off-by: Rob Bradford > > --- > > target/riscv/cpu.c | 5 +++-- > > target/riscv/cpu.h | 1 + > > target/riscv/tcg/tcg-cpu.c | 1 + > > 3 files changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c > > index b07a76ef6b..22f8e527ff 100644 > > --- a/target/riscv/cpu.c > > +++ b/target/riscv/cpu.c > > @@ -38,9 +38,9 @@ > > #include "tcg/tcg.h" > > > > /* RISC-V CPU definitions */ > > -static const char riscv_single_letter_exts[] = "IEMAFDQCPVH"; > > +static const char riscv_single_letter_exts[] = "IEMAFDQCBPVH"; > > Is there a corresponding proposed change to table 29.1 of the nonpriv spec > which states B comes after C and before P? If so, can you provide a link > to it? Otherwise, how do we know that? Oh, I see. The unpriv spec B chapter comes after the C chapter (and before J, P, ...). I still wonder if we'll have a 29.1 table update with the ratification of this extension though. Thanks, drew
Re: [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension
On Tue, Jan 09, 2024 at 05:07:35PM +, Rob Bradford wrote:
> Add the infrastructure for the 'B' extension which is the union of the
> Zba, Zbb and Zbs instructions.
>
> Signed-off-by: Rob Bradford
> ---
> target/riscv/cpu.c | 5 +++--
> target/riscv/cpu.h | 1 +
> target/riscv/tcg/tcg-cpu.c | 1 +
> 3 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index b07a76ef6b..22f8e527ff 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -38,9 +38,9 @@
> #include "tcg/tcg.h"
>
> /* RISC-V CPU definitions */
> -static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
> +static const char riscv_single_letter_exts[] = "IEMAFDQCBPVH";
Is there a corresponding proposed change to table 29.1 of the nonpriv spec
which states B comes after C and before P? If so, can you provide a link
to it? Otherwise, how do we know that?
Thanks,
drew
> const uint32_t misa_bits[] = {RVI, RVE, RVM, RVA, RVF, RVD, RVV,
> - RVC, RVS, RVU, RVH, RVJ, RVG, 0};
> + RVC, RVS, RVU, RVH, RVJ, RVG, RVB, 0};
>
> /*
> * From vector_helper.c
> @@ -1251,6 +1251,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)")
> };
>
> static int riscv_validate_misa_info_idx(uint32_t bit)
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 2725528bb5..756a345513 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -69,6 +69,7 @@ typedef struct CPUArchState CPURISCVState;
> #define RVH RV('H')
> #define RVJ RV('J')
> #define RVG RV('G')
> +#define RVB RV('B')
>
> extern const uint32_t misa_bits[];
> const char *riscv_get_misa_ext_name(uint32_t bit);
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 8a35683a34..fda54671d5 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -791,6 +791,7 @@ static const RISCVCPUMisaExtConfig misa_ext_cfgs[] = {
> MISA_CFG(RVJ, false),
> MISA_CFG(RVV, false),
> MISA_CFG(RVG, false),
> +MISA_CFG(RVB, false)
> };
>
> /*
> --
> 2.43.0
>
>
Re: [PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension
On 1/9/24 14:07, Rob Bradford wrote:
Add the infrastructure for the 'B' extension which is the union of the
Zba, Zbb and Zbs instructions.
Signed-off-by: Rob Bradford
---
target/riscv/cpu.c | 5 +++--
target/riscv/cpu.h | 1 +
target/riscv/tcg/tcg-cpu.c | 1 +
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index b07a76ef6b..22f8e527ff 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -38,9 +38,9 @@
#include "tcg/tcg.h"
/* RISC-V CPU definitions */
-static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
+static const char riscv_single_letter_exts[] = "IEMAFDQCBPVH";
const uint32_t misa_bits[] = {RVI, RVE, RVM, RVA, RVF, RVD, RVV,
- RVC, RVS, RVU, RVH, RVJ, RVG, 0};
+ RVC, RVS, RVU, RVH, RVJ, RVG, RVB, 0};
/*
* From vector_helper.c
@@ -1251,6 +1251,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)")
};
static int riscv_validate_misa_info_idx(uint32_t bit)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 2725528bb5..756a345513 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -69,6 +69,7 @@ typedef struct CPUArchState CPURISCVState;
#define RVH RV('H')
#define RVJ RV('J')
#define RVG RV('G')
+#define RVB RV('B')
extern const uint32_t misa_bits[];
const char *riscv_get_misa_ext_name(uint32_t bit);
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 8a35683a34..fda54671d5 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -791,6 +791,7 @@ static const RISCVCPUMisaExtConfig misa_ext_cfgs[] = {
MISA_CFG(RVJ, false),
MISA_CFG(RVV, false),
MISA_CFG(RVG, false),
+MISA_CFG(RVB, false)
Please add a comma at the end:
+MISA_CFG(RVB, false),
This way, when a new bit is added, the change is limited to the new entry.
With this nit fixed:
Reviewed-by: Daniel Henrique Barboza
};
/*
[PATCH 1/3] target/riscv: Add infrastructure for 'B' MISA extension
Add the infrastructure for the 'B' extension which is the union of the
Zba, Zbb and Zbs instructions.
Signed-off-by: Rob Bradford
---
target/riscv/cpu.c | 5 +++--
target/riscv/cpu.h | 1 +
target/riscv/tcg/tcg-cpu.c | 1 +
3 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index b07a76ef6b..22f8e527ff 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -38,9 +38,9 @@
#include "tcg/tcg.h"
/* RISC-V CPU definitions */
-static const char riscv_single_letter_exts[] = "IEMAFDQCPVH";
+static const char riscv_single_letter_exts[] = "IEMAFDQCBPVH";
const uint32_t misa_bits[] = {RVI, RVE, RVM, RVA, RVF, RVD, RVV,
- RVC, RVS, RVU, RVH, RVJ, RVG, 0};
+ RVC, RVS, RVU, RVH, RVJ, RVG, RVB, 0};
/*
* From vector_helper.c
@@ -1251,6 +1251,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)")
};
static int riscv_validate_misa_info_idx(uint32_t bit)
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 2725528bb5..756a345513 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -69,6 +69,7 @@ typedef struct CPUArchState CPURISCVState;
#define RVH RV('H')
#define RVJ RV('J')
#define RVG RV('G')
+#define RVB RV('B')
extern const uint32_t misa_bits[];
const char *riscv_get_misa_ext_name(uint32_t bit);
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 8a35683a34..fda54671d5 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -791,6 +791,7 @@ static const RISCVCPUMisaExtConfig misa_ext_cfgs[] = {
MISA_CFG(RVJ, false),
MISA_CFG(RVV, false),
MISA_CFG(RVG, false),
+MISA_CFG(RVB, false)
};
/*
--
2.43.0
