1. Patchew
  2. QEMU
  3. View series

[PATCH] target/mips: Fix msaregnames and mxuregnames off-by-one

Anton Johansson via posted 1 patch 2 weeks, 2 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250912142927.20919-1-anjo@rev.ng
Maintainers: "Philippe Mathieu-Daudé" <philmd@linaro.org>, Aurelien Jarno <aurelien@aurel32.net>, Jiaxun Yang <jiaxun.yang@flygoat.com>, Aleksandar Rikalo <arikalo@gmail.com>
There is a newer version of this series
target/mips/tcg/msa_translate.c | 2 +-
target/mips/tcg/mxu_translate.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
[PATCH] target/mips: Fix msaregnames and mxuregnames off-by-one
Posted by Anton Johansson via 2 weeks, 2 days ago 
The msaregnames and mxuregnames arrays contains strings of 7 bytes
("w10.d0", ...) and 5 bytes ("XR10", ...) in length including the
NULL byte.

Signed-off-by: Anton Johansson <anjo@rev.ng>
---
 target/mips/tcg/msa_translate.c | 2 +-
 target/mips/tcg/mxu_translate.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/mips/tcg/msa_translate.c b/target/mips/tcg/msa_translate.c
index 82b149922f..0e947125a0 100644
--- a/target/mips/tcg/msa_translate.c
+++ b/target/mips/tcg/msa_translate.c
@@ -32,7 +32,7 @@ static inline int plus_2(DisasContext *s, int x)
 /* Include the auto-generated decoder.  */
 #include "decode-msa.c.inc"
 
-static const char msaregnames[][6] = {
+static const char msaregnames[][7] = {
     "w0.d0",  "w0.d1",  "w1.d0",  "w1.d1",
     "w2.d0",  "w2.d1",  "w3.d0",  "w3.d1",
     "w4.d0",  "w4.d1",  "w5.d0",  "w5.d1",
diff --git a/target/mips/tcg/mxu_translate.c b/target/mips/tcg/mxu_translate.c
index 35ebb0397d..7e8cc8b06f 100644
--- a/target/mips/tcg/mxu_translate.c
+++ b/target/mips/tcg/mxu_translate.c
@@ -609,7 +609,7 @@ enum {
 static TCGv mxu_gpr[NUMBER_OF_MXU_REGISTERS - 1];
 static TCGv mxu_CR;
 
-static const char mxuregnames[NUMBER_OF_MXU_REGISTERS][4] = {
+static const char mxuregnames[NUMBER_OF_MXU_REGISTERS][5] = {
     "XR1",  "XR2",  "XR3",  "XR4",  "XR5",  "XR6",  "XR7",  "XR8",
     "XR9",  "XR10", "XR11", "XR12", "XR13", "XR14", "XR15", "XCR",
 };
-- 
2.51.0
Re: [PATCH] target/mips: Fix msaregnames and mxuregnames off-by-one
Posted by Alex Bennée 1 week, 6 days ago 
Anton Johansson via <qemu-devel@nongnu.org> writes:

> The msaregnames and mxuregnames arrays contains strings of 7 bytes
> ("w10.d0", ...) and 5 bytes ("XR10", ...) in length including the
> NULL byte.
>
> Signed-off-by: Anton Johansson <anjo@rev.ng>
> ---
>  target/mips/tcg/msa_translate.c | 2 +-
>  target/mips/tcg/mxu_translate.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/mips/tcg/msa_translate.c b/target/mips/tcg/msa_translate.c
> index 82b149922f..0e947125a0 100644
> --- a/target/mips/tcg/msa_translate.c
> +++ b/target/mips/tcg/msa_translate.c
> @@ -32,7 +32,7 @@ static inline int plus_2(DisasContext *s, int x)
>  /* Include the auto-generated decoder.  */
>  #include "decode-msa.c.inc"
>  
> -static const char msaregnames[][6] = {
> +static const char msaregnames[][7] = {
>      "w0.d0",  "w0.d1",  "w1.d0",  "w1.d1",
>      "w2.d0",  "w2.d1",  "w3.d0",  "w3.d1",
>      "w4.d0",  "w4.d1",  "w5.d0",  "w5.d1",
> diff --git a/target/mips/tcg/mxu_translate.c b/target/mips/tcg/mxu_translate.c
> index 35ebb0397d..7e8cc8b06f 100644
> --- a/target/mips/tcg/mxu_translate.c
> +++ b/target/mips/tcg/mxu_translate.c
> @@ -609,7 +609,7 @@ enum {
>  static TCGv mxu_gpr[NUMBER_OF_MXU_REGISTERS - 1];
>  static TCGv mxu_CR;
>  
> -static const char mxuregnames[NUMBER_OF_MXU_REGISTERS][4] = {
> +static const char mxuregnames[NUMBER_OF_MXU_REGISTERS][5] = {
>      "XR1",  "XR2",  "XR3",  "XR4",  "XR5",  "XR6",  "XR7",  "XR8",
>      "XR9",  "XR10", "XR11", "XR12", "XR13", "XR14", "XR15", "XCR",
>  };

Maybe not quite so silly question. Why are we setting 2D dimensions on
the regnames anyway? Shouldn't we just use static const char * and be
done with it?

AFAICT all the references only care about the number anyway.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH] target/mips: Fix msaregnames and mxuregnames off-by-one
Posted by Anton Johansson via 1 week, 5 days ago 
On 15/09/25, Alex Bennée wrote:
> Anton Johansson via <qemu-devel@nongnu.org> writes:
> 
> > The msaregnames and mxuregnames arrays contains strings of 7 bytes
> > ("w10.d0", ...) and 5 bytes ("XR10", ...) in length including the
> > NULL byte.
> >
> > Signed-off-by: Anton Johansson <anjo@rev.ng>
> > ---
> >  target/mips/tcg/msa_translate.c | 2 +-
> >  target/mips/tcg/mxu_translate.c | 2 +-
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/target/mips/tcg/msa_translate.c b/target/mips/tcg/msa_translate.c
> > index 82b149922f..0e947125a0 100644
> > --- a/target/mips/tcg/msa_translate.c
> > +++ b/target/mips/tcg/msa_translate.c
> > @@ -32,7 +32,7 @@ static inline int plus_2(DisasContext *s, int x)
> >  /* Include the auto-generated decoder.  */
> >  #include "decode-msa.c.inc"
> >  
> > -static const char msaregnames[][6] = {
> > +static const char msaregnames[][7] = {
> >      "w0.d0",  "w0.d1",  "w1.d0",  "w1.d1",
> >      "w2.d0",  "w2.d1",  "w3.d0",  "w3.d1",
> >      "w4.d0",  "w4.d1",  "w5.d0",  "w5.d1",
> > diff --git a/target/mips/tcg/mxu_translate.c b/target/mips/tcg/mxu_translate.c
> > index 35ebb0397d..7e8cc8b06f 100644
> > --- a/target/mips/tcg/mxu_translate.c
> > +++ b/target/mips/tcg/mxu_translate.c
> > @@ -609,7 +609,7 @@ enum {
> >  static TCGv mxu_gpr[NUMBER_OF_MXU_REGISTERS - 1];
> >  static TCGv mxu_CR;
> >  
> > -static const char mxuregnames[NUMBER_OF_MXU_REGISTERS][4] = {
> > +static const char mxuregnames[NUMBER_OF_MXU_REGISTERS][5] = {
> >      "XR1",  "XR2",  "XR3",  "XR4",  "XR5",  "XR6",  "XR7",  "XR8",
> >      "XR9",  "XR10", "XR11", "XR12", "XR13", "XR14", "XR15", "XCR",
> >  };
> 
> Maybe not quite so silly question. Why are we setting 2D dimensions on
> the regnames anyway? Shouldn't we just use static const char * and be
> done with it?
> 
> AFAICT all the references only care about the number anyway.

Previously a 1D array of pointers was used, here's the original patch
introducing the 2D array

  https://lists.gnu.org/archive/html/qemu-devel/2021-06/msg04706.html

It's an optimization to reduce memory usage and runtime relocations.

Assuming a 64-bit linux system we can estimate the memory required for
using 1D arrays for msaregnames and mxuregnames to

    64*(8 + 24) + 16*(8 + 24) = 2560 bytes

with 8 byte pointers and 24 byte relocation table entries, the above
does not include the memory for the actual strings. And with the 1D
change we get

    64*7 + 16*5 = 528 bytes

Though I can't speak to the impact on runtime performance.

-- 
Anton Johansson
rev.ng Labs Srl.
Re: [PATCH] target/mips: Fix msaregnames and mxuregnames off-by-one
Posted by Richard Henderson 1 week, 6 days ago 
On 9/12/25 07:29, Anton Johansson via wrote:
> The msaregnames and mxuregnames arrays contains strings of 7 bytes
> ("w10.d0", ...) and 5 bytes ("XR10", ...) in length including the
> NULL byte.
> 
> Signed-off-by: Anton Johansson <anjo@rev.ng>
> ---
>   target/mips/tcg/msa_translate.c | 2 +-
>   target/mips/tcg/mxu_translate.c | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target/mips/tcg/msa_translate.c b/target/mips/tcg/msa_translate.c
> index 82b149922f..0e947125a0 100644
> --- a/target/mips/tcg/msa_translate.c
> +++ b/target/mips/tcg/msa_translate.c
> @@ -32,7 +32,7 @@ static inline int plus_2(DisasContext *s, int x)
>   /* Include the auto-generated decoder.  */
>   #include "decode-msa.c.inc"
>   
> -static const char msaregnames[][6] = {
> +static const char msaregnames[][7] = {
>       "w0.d0",  "w0.d1",  "w1.d0",  "w1.d1",
>       "w2.d0",  "w2.d1",  "w3.d0",  "w3.d1",
>       "w4.d0",  "w4.d1",  "w5.d0",  "w5.d1",
> diff --git a/target/mips/tcg/mxu_translate.c b/target/mips/tcg/mxu_translate.c
> index 35ebb0397d..7e8cc8b06f 100644
> --- a/target/mips/tcg/mxu_translate.c
> +++ b/target/mips/tcg/mxu_translate.c
> @@ -609,7 +609,7 @@ enum {
>   static TCGv mxu_gpr[NUMBER_OF_MXU_REGISTERS - 1];
>   static TCGv mxu_CR;
>   
> -static const char mxuregnames[NUMBER_OF_MXU_REGISTERS][4] = {
> +static const char mxuregnames[NUMBER_OF_MXU_REGISTERS][5] = {
>       "XR1",  "XR2",  "XR3",  "XR4",  "XR5",  "XR6",  "XR7",  "XR8",
>       "XR9",  "XR10", "XR11", "XR12", "XR13", "XR14", "XR15", "XCR",
>   };

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.