qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] target/mips: Fix msaregnames and mxuregnames off-by-one
@ 2025-09-12 14:29 Anton Johansson via
  2025-09-15 13:31 ` Richard Henderson
  2025-09-15 14:43 ` Alex Bennée
  0 siblings, 2 replies; 4+ messages in thread
From: Anton Johansson via @ 2025-09-12 14:29 UTC (permalink / raw)
  To: qemu-devel; +Cc: philmd

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



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] target/mips: Fix msaregnames and mxuregnames off-by-one
  2025-09-12 14:29 [PATCH] target/mips: Fix msaregnames and mxuregnames off-by-one Anton Johansson via
@ 2025-09-15 13:31 ` Richard Henderson
  2025-09-15 14:43 ` Alex Bennée
  1 sibling, 0 replies; 4+ messages in thread
From: Richard Henderson @ 2025-09-15 13:31 UTC (permalink / raw)
  To: qemu-devel

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~


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] target/mips: Fix msaregnames and mxuregnames off-by-one
  2025-09-12 14:29 [PATCH] target/mips: Fix msaregnames and mxuregnames off-by-one Anton Johansson via
  2025-09-15 13:31 ` Richard Henderson
@ 2025-09-15 14:43 ` Alex Bennée
  2025-09-15 16:02   ` Anton Johansson via
  1 sibling, 1 reply; 4+ messages in thread
From: Alex Bennée @ 2025-09-15 14:43 UTC (permalink / raw)
  To: Anton Johansson via; +Cc: Anton Johansson, philmd

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] target/mips: Fix msaregnames and mxuregnames off-by-one
  2025-09-15 14:43 ` Alex Bennée
@ 2025-09-15 16:02   ` Anton Johansson via
  0 siblings, 0 replies; 4+ messages in thread
From: Anton Johansson via @ 2025-09-15 16:02 UTC (permalink / raw)
  To: Alex Bennée; +Cc: Anton Johansson via, philmd

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.


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-09-15 16:04 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-12 14:29 [PATCH] target/mips: Fix msaregnames and mxuregnames off-by-one Anton Johansson via
2025-09-15 13:31 ` Richard Henderson
2025-09-15 14:43 ` Alex Bennée
2025-09-15 16:02   ` Anton Johansson via

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).