1. Patchew
  2. QEMU
  3. View series

[PATCH] Hexagon (target/hexagon) Clean up disassembly of control and system regs

Taylor Simpson posted 1 patch 1 month, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260810182804.27310-1-ltaylorsimpson@gmail.com
Maintainers: Brian Cain <brian.cain@oss.qualcomm.com>, Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>
There is a newer version of this series
target/hexagon/printinsn.c      | 22 ++++++++++++++++++----
target/hexagon/gen_printinsn.py | 14 ++++++++------
2 files changed, 26 insertions(+), 10 deletions(-)
[PATCH] Hexagon (target/hexagon) Clean up disassembly of control and system regs
Posted by Taylor Simpson 1 month, 2 weeks ago 
Change disassembly of control regs from C{num}/{name} to {name}
Change disassembly of system regs from S{num}/r{num} to {name}

Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com>
---
 target/hexagon/printinsn.c      | 22 ++++++++++++++++++----
 target/hexagon/gen_printinsn.py | 14 ++++++++------
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/target/hexagon/printinsn.c b/target/hexagon/printinsn.c
index a7e46f4bcd..b55d3b0b35 100644
--- a/target/hexagon/printinsn.c
+++ b/target/hexagon/printinsn.c
@@ -23,18 +23,32 @@
 #include "internal.h"
 #include "decode.h"
 
+static char regstr[10];
+
 static const char *sreg2str(unsigned int reg)
 {
-    if (reg < TOTAL_PER_THREAD_REGS) {
-        return hexagon_regnames[reg];
+#ifndef CONFIG_USER_ONLY
+    if (reg < NUM_SREGS) {
+        return hexagon_sregnames[reg];
     } else {
-        return "???";
+        snprintf(regstr, sizeof(regstr), "S%d", reg);
+        return regstr;
     }
+#else
+    snprintf(regstr, sizeof(regstr), "S%d", reg);
+    return regstr;
+#endif
 }
 
 static const char *creg2str(unsigned int reg)
 {
-    return sreg2str(reg + HEX_REG_SA0);
+    unsigned int gpr = reg + HEX_REG_SA0;
+    if (gpr < TOTAL_PER_THREAD_REGS) {
+        return hexagon_regnames[gpr];
+    } else {
+        snprintf(regstr, sizeof(regstr), "C%d", reg);
+        return regstr;
+    }
 }
 
 static void snprintinsn(GString *buf, Insn *insn)
diff --git a/target/hexagon/gen_printinsn.py b/target/hexagon/gen_printinsn.py
index d5f969960a..cf1a12adbc 100755
--- a/target/hexagon/gen_printinsn.py
+++ b/target/hexagon/gen_printinsn.py
@@ -28,13 +28,14 @@
 ##     Generate data for printing each instruction (format string + operands)
 ##
 def regprinter(m):
-    str = m.group(1)
-    str += ":".join(["%d"] * len(m.group(2)))
-    str += m.group(3)
     if ("S" in m.group(1)) and (len(m.group(2)) == 1):
-        str += "/%s"
+        str = "%s"
     elif ("C" in m.group(1)) and (len(m.group(2)) == 1):
-        str += "/%s"
+        str = "%s"
+    else:
+        str = m.group(1)
+        str += ":".join(["%d"] * len(m.group(2)))
+        str += m.group(3)
     return str
 
 
@@ -142,11 +143,12 @@ def main():
                     else:
                         regno = ri
                     if len(b) == 1:
-                        f.write(f", insn->regno[{regno}]")
                         if "S" in a:
                             f.write(f", sreg2str(insn->regno[{regno}])")
                         elif "C" in a:
                             f.write(f", creg2str(insn->regno[{regno}])")
+                        else:
+                            f.write(f", insn->regno[{regno}]")
                     elif len(b) == 2:
                         f.write(f", insn->regno[{regno}] + 1" f", insn->regno[{regno}]")
                     else:
-- 
2.43.0
Re: [PATCH] Hexagon (target/hexagon) Clean up disassembly of control and system regs
Posted by Brian Cain 1 month, 2 weeks ago 
On 8/10/2026 1:28 PM, Taylor Simpson wrote:
> Change disassembly of control regs from C{num}/{name} to {name}
> Change disassembly of system regs from S{num}/r{num} to {name}
>
> Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com>
> ---
>   target/hexagon/printinsn.c      | 22 ++++++++++++++++++----
>   target/hexagon/gen_printinsn.py | 14 ++++++++------
>   2 files changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/target/hexagon/printinsn.c b/target/hexagon/printinsn.c
> index a7e46f4bcd..b55d3b0b35 100644
> --- a/target/hexagon/printinsn.c
> +++ b/target/hexagon/printinsn.c
> @@ -23,18 +23,32 @@
>   #include "internal.h"
>   #include "decode.h"
>   
> +static char regstr[10];
> +
>   static const char *sreg2str(unsigned int reg)
>   {
> -    if (reg < TOTAL_PER_THREAD_REGS) {
> -        return hexagon_regnames[reg];
> +#ifndef CONFIG_USER_ONLY
> +    if (reg < NUM_SREGS) {
> +        return hexagon_sregnames[reg];
>       } else {
> -        return "???";
> +        snprintf(regstr, sizeof(regstr), "S%d", reg);
> +        return regstr;
>       }
> +#else
> +    snprintf(regstr, sizeof(regstr), "S%d", reg);
> +    return regstr;
> +#endif
>   }
>   
>   static const char *creg2str(unsigned int reg)
>   {
> -    return sreg2str(reg + HEX_REG_SA0);
> +    unsigned int gpr = reg + HEX_REG_SA0;
> +    if (gpr < TOTAL_PER_THREAD_REGS) {
> +        return hexagon_regnames[gpr];
> +    } else {
> +        snprintf(regstr, sizeof(regstr), "C%d", reg);
> +        return regstr;
> +    }
>   }
>   


Seems like this change also depends on some kind of mutex around 
regstr[] so that we know it's not being used concurrently by multiple 
threads.  Translation is single-threaded but if we gained another caller 
we wouldn't remember to come back and remediate this.

I suppose we could take an input buffer/length.  Or maybe we just add 
another static _regnames[] array to solve the lifetime/race problems?



>   static void snprintinsn(GString *buf, Insn *insn)
> diff --git a/target/hexagon/gen_printinsn.py b/target/hexagon/gen_printinsn.py
> index d5f969960a..cf1a12adbc 100755
> --- a/target/hexagon/gen_printinsn.py
> +++ b/target/hexagon/gen_printinsn.py
> @@ -28,13 +28,14 @@
>   ##     Generate data for printing each instruction (format string + operands)
>   ##
>   def regprinter(m):
> -    str = m.group(1)
> -    str += ":".join(["%d"] * len(m.group(2)))
> -    str += m.group(3)
>       if ("S" in m.group(1)) and (len(m.group(2)) == 1):
> -        str += "/%s"
> +        str = "%s"
>       elif ("C" in m.group(1)) and (len(m.group(2)) == 1):
> -        str += "/%s"
> +        str = "%s"
> +    else:
> +        str = m.group(1)
> +        str += ":".join(["%d"] * len(m.group(2)))
> +        str += m.group(3)
>       return str
>   
>   
> @@ -142,11 +143,12 @@ def main():
>                       else:
>                           regno = ri
>                       if len(b) == 1:
> -                        f.write(f", insn->regno[{regno}]")
>                           if "S" in a:
>                               f.write(f", sreg2str(insn->regno[{regno}])")
>                           elif "C" in a:
>                               f.write(f", creg2str(insn->regno[{regno}])")
> +                        else:
> +                            f.write(f", insn->regno[{regno}]")
>                       elif len(b) == 2:
>                           f.write(f", insn->regno[{regno}] + 1" f", insn->regno[{regno}]")
>                       else:
Re: [PATCH] Hexagon (target/hexagon) Clean up disassembly of control and system regs
Posted by Taylor Simpson 1 month, 2 weeks ago 
On Mon, Aug 10, 2026 at 1:34 PM Brian Cain <brian.cain@oss.qualcomm.com>
wrote:

>
> On 8/10/2026 1:28 PM, Taylor Simpson wrote:
> > Change disassembly of control regs from C{num}/{name} to {name}
> > Change disassembly of system regs from S{num}/r{num} to {name}
> >
> > Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com>
> > ---
> >   target/hexagon/printinsn.c      | 22 ++++++++++++++++++----
> >   target/hexagon/gen_printinsn.py | 14 ++++++++------
> >   2 files changed, 26 insertions(+), 10 deletions(-)
> >
> > diff --git a/target/hexagon/printinsn.c b/target/hexagon/printinsn.c
> > index a7e46f4bcd..b55d3b0b35 100644
> > --- a/target/hexagon/printinsn.c
> > +++ b/target/hexagon/printinsn.c
> > @@ -23,18 +23,32 @@
> >   #include "internal.h"
> >   #include "decode.h"
> >
> > +static char regstr[10];
> > +
> >   static const char *sreg2str(unsigned int reg)
> >   {
> > -    if (reg < TOTAL_PER_THREAD_REGS) {
> > -        return hexagon_regnames[reg];
> > +#ifndef CONFIG_USER_ONLY
> > +    if (reg < NUM_SREGS) {
> > +        return hexagon_sregnames[reg];
> >       } else {
> > -        return "???";
> > +        snprintf(regstr, sizeof(regstr), "S%d", reg);
> > +        return regstr;
> >       }
> > +#else
> > +    snprintf(regstr, sizeof(regstr), "S%d", reg);
> > +    return regstr;
> > +#endif
> >   }
> >
> >   static const char *creg2str(unsigned int reg)
> >   {
> > -    return sreg2str(reg + HEX_REG_SA0);
> > +    unsigned int gpr = reg + HEX_REG_SA0;
> > +    if (gpr < TOTAL_PER_THREAD_REGS) {
> > +        return hexagon_regnames[gpr];
> > +    } else {
> > +        snprintf(regstr, sizeof(regstr), "C%d", reg);
> > +        return regstr;
> > +    }
> >   }
> >
>
>
> Seems like this change also depends on some kind of mutex around
> regstr[] so that we know it's not being used concurrently by multiple
> threads.  Translation is single-threaded but if we gained another caller
> we wouldn't remember to come back and remediate this.
>
> I suppose we could take an input buffer/length.  Or maybe we just add
> another static _regnames[] array to solve the lifetime/race problems?
>
>
How about putting a regstr field in the Insn struct (and pass Insn* to
these two functions)?

Thanks,
Taylor
Re: [PATCH] Hexagon (target/hexagon) Clean up disassembly of control and system regs
Posted by Brian Cain 1 month, 2 weeks ago 
On 8/10/2026 3:53 PM, Taylor Simpson wrote:
>
>
> On Mon, Aug 10, 2026 at 1:34 PM Brian Cain 
> <brian.cain@oss.qualcomm.com> wrote:
>
>
>     On 8/10/2026 1:28 PM, Taylor Simpson wrote:
>     > Change disassembly of control regs from C{num}/{name} to {name}
>     > Change disassembly of system regs from S{num}/r{num} to {name}
>     >
>     > Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com>
>     > ---
>     >   target/hexagon/printinsn.c      | 22 ++++++++++++++++++----
>     >   target/hexagon/gen_printinsn.py | 14 ++++++++------
>     >   2 files changed, 26 insertions(+), 10 deletions(-)
>     >
>     > diff --git a/target/hexagon/printinsn.c b/target/hexagon/printinsn.c
>     > index a7e46f4bcd..b55d3b0b35 100644
>     > --- a/target/hexagon/printinsn.c
>     > +++ b/target/hexagon/printinsn.c
>     > @@ -23,18 +23,32 @@
>     >   #include "internal.h"
>     >   #include "decode.h"
>     >
>     > +static char regstr[10];
>     > +
>     >   static const char *sreg2str(unsigned int reg)
>     >   {
>     > -    if (reg < TOTAL_PER_THREAD_REGS) {
>     > -        return hexagon_regnames[reg];
>     > +#ifndef CONFIG_USER_ONLY
>     > +    if (reg < NUM_SREGS) {
>     > +        return hexagon_sregnames[reg];
>     >       } else {
>     > -        return "???";
>     > +        snprintf(regstr, sizeof(regstr), "S%d", reg);
>     > +        return regstr;
>     >       }
>     > +#else
>     > +    snprintf(regstr, sizeof(regstr), "S%d", reg);
>     > +    return regstr;
>     > +#endif
>     >   }
>     >
>     >   static const char *creg2str(unsigned int reg)
>     >   {
>     > -    return sreg2str(reg + HEX_REG_SA0);
>     > +    unsigned int gpr = reg + HEX_REG_SA0;
>     > +    if (gpr < TOTAL_PER_THREAD_REGS) {
>     > +        return hexagon_regnames[gpr];
>     > +    } else {
>     > +        snprintf(regstr, sizeof(regstr), "C%d", reg);
>     > +        return regstr;
>     > +    }
>     >   }
>     >
>
>
>     Seems like this change also depends on some kind of mutex around
>     regstr[] so that we know it's not being used concurrently by multiple
>     threads.  Translation is single-threaded but if we gained another
>     caller
>     we wouldn't remember to come back and remediate this.
>
>     I suppose we could take an input buffer/length.  Or maybe we just add
>     another static _regnames[] array to solve the lifetime/race problems?
>
>
> How about putting a regstr field in the Insn struct (and pass Insn* to 
> these two functions)?
>

A character array?  Sure: seems like that should work.

But if we have a static array for the others why not leverage that here 
too?  We can statically fill these values, right?


> Thanks,
> Taylor
>
Re: [PATCH] Hexagon (target/hexagon) Clean up disassembly of control and system regs
Posted by Taylor Simpson 1 month, 2 weeks ago 
On Mon, Aug 10, 2026 at 3:12 PM Brian Cain <brian.cain@oss.qualcomm.com>
wrote:

>
> On 8/10/2026 3:53 PM, Taylor Simpson wrote:
>
>
>
> On Mon, Aug 10, 2026 at 1:34 PM Brian Cain <brian.cain@oss.qualcomm.com>
> wrote:
>
>>
>> On 8/10/2026 1:28 PM, Taylor Simpson wrote:
>> > Change disassembly of control regs from C{num}/{name} to {name}
>> > Change disassembly of system regs from S{num}/r{num} to {name}
>> >
>> > Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com>
>> > ---
>> >   target/hexagon/printinsn.c      | 22 ++++++++++++++++++----
>> >   target/hexagon/gen_printinsn.py | 14 ++++++++------
>> >   2 files changed, 26 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/target/hexagon/printinsn.c b/target/hexagon/printinsn.c
>> > index a7e46f4bcd..b55d3b0b35 100644
>> > --- a/target/hexagon/printinsn.c
>> > +++ b/target/hexagon/printinsn.c
>> > @@ -23,18 +23,32 @@
>> >   #include "internal.h"
>> >   #include "decode.h"
>> >
>> > +static char regstr[10];
>> > +
>> >   static const char *sreg2str(unsigned int reg)
>> >   {
>> > -    if (reg < TOTAL_PER_THREAD_REGS) {
>> > -        return hexagon_regnames[reg];
>> > +#ifndef CONFIG_USER_ONLY
>> > +    if (reg < NUM_SREGS) {
>> > +        return hexagon_sregnames[reg];
>> >       } else {
>> > -        return "???";
>> > +        snprintf(regstr, sizeof(regstr), "S%d", reg);
>> > +        return regstr;
>> >       }
>> > +#else
>> > +    snprintf(regstr, sizeof(regstr), "S%d", reg);
>> > +    return regstr;
>> > +#endif
>> >   }
>> >
>> >   static const char *creg2str(unsigned int reg)
>> >   {
>> > -    return sreg2str(reg + HEX_REG_SA0);
>> > +    unsigned int gpr = reg + HEX_REG_SA0;
>> > +    if (gpr < TOTAL_PER_THREAD_REGS) {
>> > +        return hexagon_regnames[gpr];
>> > +    } else {
>> > +        snprintf(regstr, sizeof(regstr), "C%d", reg);
>> > +        return regstr;
>> > +    }
>> >   }
>> >
>>
>>
>> Seems like this change also depends on some kind of mutex around
>> regstr[] so that we know it's not being used concurrently by multiple
>> threads.  Translation is single-threaded but if we gained another caller
>> we wouldn't remember to come back and remediate this.
>>
>> I suppose we could take an input buffer/length.  Or maybe we just add
>> another static _regnames[] array to solve the lifetime/race problems?
>>
>>
> How about putting a regstr field in the Insn struct (and pass Insn* to
> these two functions)?
>
>
> A character array?  Sure: seems like that should work.
>
> But if we have a static array for the others why not leverage that here
> too?  We can statically fill these values, right?
>

These are for cases where there is some sort of error and the reg number is
larger than NUM_SREGS/TOTAL_PER_THREAD_REGS.  So, we wouldn't know how to
size these without looking at the number of bits in the encoding of each
instruction that has one of these.  However, that could change over time if
the architecture adds new instructions.

The regno field in the Insn struct is a uint8_t, so we could create static
arrays with 256 entries and

We could revert back to returning "???".  Alternatively, S<unknown> or
C<unknown> would be more readable.

Thoughts?
Re: [PATCH] Hexagon (target/hexagon) Clean up disassembly of control and system regs
Posted by Brian Cain 1 month, 2 weeks ago 
On 8/10/2026 4:32 PM, Taylor Simpson wrote:
>
>
> On Mon, Aug 10, 2026 at 3:12 PM Brian Cain 
> <brian.cain@oss.qualcomm.com> wrote:
>
>
>     On 8/10/2026 3:53 PM, Taylor Simpson wrote:
>>
>>
>>     On Mon, Aug 10, 2026 at 1:34 PM Brian Cain
>>     <brian.cain@oss.qualcomm.com> wrote:
>>
>>
>>         On 8/10/2026 1:28 PM, Taylor Simpson wrote:
>>         > Change disassembly of control regs from C{num}/{name} to {name}
>>         > Change disassembly of system regs from S{num}/r{num} to {name}
>>         >
>>         > Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com>
>>         > ---
>>         >   target/hexagon/printinsn.c      | 22 ++++++++++++++++++----
>>         >   target/hexagon/gen_printinsn.py | 14 ++++++++------
>>         >   2 files changed, 26 insertions(+), 10 deletions(-)
>>         >
>>         > diff --git a/target/hexagon/printinsn.c
>>         b/target/hexagon/printinsn.c
>>         > index a7e46f4bcd..b55d3b0b35 100644
>>         > --- a/target/hexagon/printinsn.c
>>         > +++ b/target/hexagon/printinsn.c
>>         > @@ -23,18 +23,32 @@
>>         >   #include "internal.h"
>>         >   #include "decode.h"
>>         >
>>         > +static char regstr[10];
>>         > +
>>         >   static const char *sreg2str(unsigned int reg)
>>         >   {
>>         > -    if (reg < TOTAL_PER_THREAD_REGS) {
>>         > -        return hexagon_regnames[reg];
>>         > +#ifndef CONFIG_USER_ONLY
>>         > +    if (reg < NUM_SREGS) {
>>         > +        return hexagon_sregnames[reg];
>>         >       } else {
>>         > -        return "???";
>>         > +        snprintf(regstr, sizeof(regstr), "S%d", reg);
>>         > +        return regstr;
>>         >       }
>>         > +#else
>>         > +    snprintf(regstr, sizeof(regstr), "S%d", reg);
>>         > +    return regstr;
>>         > +#endif
>>         >   }
>>         >
>>         >   static const char *creg2str(unsigned int reg)
>>         >   {
>>         > -    return sreg2str(reg + HEX_REG_SA0);
>>         > +    unsigned int gpr = reg + HEX_REG_SA0;
>>         > +    if (gpr < TOTAL_PER_THREAD_REGS) {
>>         > +        return hexagon_regnames[gpr];
>>         > +    } else {
>>         > +        snprintf(regstr, sizeof(regstr), "C%d", reg);
>>         > +        return regstr;
>>         > +    }
>>         >   }
>>         >
>>
>>
>>         Seems like this change also depends on some kind of mutex around
>>         regstr[] so that we know it's not being used concurrently by
>>         multiple
>>         threads.  Translation is single-threaded but if we gained
>>         another caller
>>         we wouldn't remember to come back and remediate this.
>>
>>         I suppose we could take an input buffer/length. Or maybe we
>>         just add
>>         another static _regnames[] array to solve the lifetime/race
>>         problems?
>>
>>
>>     How about putting a regstr field in the Insn struct (and pass
>>     Insn* to these two functions)?
>>
>
>     A character array?  Sure: seems like that should work.
>
>     But if we have a static array for the others why not leverage that
>     here too?  We can statically fill these values, right?
>
>
> These are for cases where there is some sort of error and the reg 
> number is larger than NUM_SREGS/TOTAL_PER_THREAD_REGS.  So, we 
> wouldn't know how to size these without looking at the number of bits 
> in the encoding of each instruction that has one of these. However, 
> that could change over time if the architecture adds new instructions.
>
Ah, okay, right.

Okay - any solution that mitigates the race should suffice, then.


> The regno field in the Insn struct is a uint8_t, so we could create 
> static arrays with 256 entries and
>
> We could revert back to returning "???".  Alternatively, S<unknown> or 
> C<unknown> would be more readable.
>
> Thoughts?
>
>
>
Re: [PATCH] Hexagon (target/hexagon) Clean up disassembly of control and system regs
Posted by Taylor Simpson 1 month, 2 weeks ago 
On Mon, Aug 10, 2026 at 4:54 PM Brian Cain <brian.cain@oss.qualcomm.com>
wrote:

>
> On 8/10/2026 4:32 PM, Taylor Simpson wrote:
>
>
>
> On Mon, Aug 10, 2026 at 3:12 PM Brian Cain <brian.cain@oss.qualcomm.com>
> wrote:
>
>>
>> On 8/10/2026 3:53 PM, Taylor Simpson wrote:
>>
>>
>>
>> On Mon, Aug 10, 2026 at 1:34 PM Brian Cain <brian.cain@oss.qualcomm.com>
>> wrote:
>>
>>>
>>> On 8/10/2026 1:28 PM, Taylor Simpson wrote:
>>> > Change disassembly of control regs from C{num}/{name} to {name}
>>> > Change disassembly of system regs from S{num}/r{num} to {name}
>>> >
>>> > Signed-off-by: Taylor Simpson <ltaylorsimpson@gmail.com>
>>> > ---
>>> >   target/hexagon/printinsn.c      | 22 ++++++++++++++++++----
>>> >   target/hexagon/gen_printinsn.py | 14 ++++++++------
>>> >   2 files changed, 26 insertions(+), 10 deletions(-)
>>> >
>>> > diff --git a/target/hexagon/printinsn.c b/target/hexagon/printinsn.c
>>> > index a7e46f4bcd..b55d3b0b35 100644
>>> > --- a/target/hexagon/printinsn.c
>>> > +++ b/target/hexagon/printinsn.c
>>> > @@ -23,18 +23,32 @@
>>> >   #include "internal.h"
>>> >   #include "decode.h"
>>> >
>>> > +static char regstr[10];
>>> > +
>>> >   static const char *sreg2str(unsigned int reg)
>>> >   {
>>> > -    if (reg < TOTAL_PER_THREAD_REGS) {
>>> > -        return hexagon_regnames[reg];
>>> > +#ifndef CONFIG_USER_ONLY
>>> > +    if (reg < NUM_SREGS) {
>>> > +        return hexagon_sregnames[reg];
>>> >       } else {
>>> > -        return "???";
>>> > +        snprintf(regstr, sizeof(regstr), "S%d", reg);
>>> > +        return regstr;
>>> >       }
>>> > +#else
>>> > +    snprintf(regstr, sizeof(regstr), "S%d", reg);
>>> > +    return regstr;
>>> > +#endif
>>> >   }
>>> >
>>> >   static const char *creg2str(unsigned int reg)
>>> >   {
>>> > -    return sreg2str(reg + HEX_REG_SA0);
>>> > +    unsigned int gpr = reg + HEX_REG_SA0;
>>> > +    if (gpr < TOTAL_PER_THREAD_REGS) {
>>> > +        return hexagon_regnames[gpr];
>>> > +    } else {
>>> > +        snprintf(regstr, sizeof(regstr), "C%d", reg);
>>> > +        return regstr;
>>> > +    }
>>> >   }
>>> >
>>>
>>>
>>> Seems like this change also depends on some kind of mutex around
>>> regstr[] so that we know it's not being used concurrently by multiple
>>> threads.  Translation is single-threaded but if we gained another caller
>>> we wouldn't remember to come back and remediate this.
>>>
>>> I suppose we could take an input buffer/length.  Or maybe we just add
>>> another static _regnames[] array to solve the lifetime/race problems?
>>>
>>>
>> How about putting a regstr field in the Insn struct (and pass Insn* to
>> these two functions)?
>>
>>
>> A character array?  Sure: seems like that should work.
>>
>> But if we have a static array for the others why not leverage that here
>> too?  We can statically fill these values, right?
>>
>
> These are for cases where there is some sort of error and the reg number
> is larger than NUM_SREGS/TOTAL_PER_THREAD_REGS.  So, we wouldn't know how
> to size these without looking at the number of bits in the encoding of each
> instruction that has one of these.  However, that could change over time if
> the architecture adds new instructions.
>
> Ah, okay, right.
>
> Okay - any solution that mitigates the race should suffice, then.
>

I'll go with static arrays with 256 entries.  Stand by for v2 of the patch.
Re: [PATCH] Hexagon (target/hexagon) Clean up disassembly of control and system regs
Posted by Philippe Mathieu-Daudé 1 month, 2 weeks ago 
Hi Taylor,

On 10/8/26 23:32, Taylor Simpson wrote:
> On Mon, Aug 10, 2026 at 3:12 PM Brian Cain <brian.cain@oss.qualcomm.com>
> wrote:


>>> Seems like this change also depends on some kind of mutex around
>>> regstr[] so that we know it's not being used concurrently by multiple
>>> threads.  Translation is single-threaded but if we gained another caller
>>> we wouldn't remember to come back and remediate this.
>>>
>>> I suppose we could take an input buffer/length.  Or maybe we just add
>>> another static _regnames[] array to solve the lifetime/race problems?
>>>
>>>
>> How about putting a regstr field in the Insn struct (and pass Insn* to
>> these two functions)?
>>
>>
>> A character array?  Sure: seems like that should work.
>>
>> But if we have a static array for the others why not leverage that here
>> too?  We can statically fill these values, right?
>>
> 
> These are for cases where there is some sort of error and the reg number is
> larger than NUM_SREGS/TOTAL_PER_THREAD_REGS.  So, we wouldn't know how to
> size these without looking at the number of bits in the encoding of each
> instruction that has one of these.  However, that could change over time if
> the architecture adds new instructions.
> 
> The regno field in the Insn struct is a uint8_t, so we could create static
> arrays with 256 entries and
> 
> We could revert back to returning "???".  Alternatively, S<unknown> or
> C<unknown> would be more readable.

Hexagon frontend already relies a lot on generators, why not also
generate a static array?

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.