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. 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?
