On 8/10/2026 4:32 PM, Taylor Simpson wrote:
On Mon, Aug 10, 2026 at 3:12 PM Brian Cain
<[email protected]> wrote:
On 8/10/2026 3:53 PM, Taylor Simpson wrote:
On Mon, Aug 10, 2026 at 1:34 PM Brian Cain
<[email protected]> 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 <[email protected]>
> ---
> 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?
