On Thu, 2026-09-03 at 10:49 +0100, Andrew Stubbs wrote:
> On 28/07/2026 23:50, David Malcolm wrote:
> > On Thu, 2026-07-02 at 09:57 +0100, Andrew Stubbs wrote:
> > > Ping.
> >
> > Thanks for the patch, sorry about the delay in reviewing this.
> >
> > Testing this is usually a bit of a pain. You don't happen to have
> > a
> > precanned way of doing this by any chance? :)
>
> I do not have any sort of testsuite. I just did some manual checks.
>
> > Given that I rarely touch the world of rtx, you're probably in a
> > better
> > position to assess this patch than me. Mostly looks good...
>
> > > > +def reg_name(regno):
> > > > + val_reg_names = gdb.parse_and_eval('reg_names')
> > > > + try:
> > > > + return val_reg_names[regno].string()
> > > > + except:
> > > > + return "pseudo"
>
> I discovered that "reg_names" is actually defined as a macro, which
> means it only works in GDB in some contexts (assuming that GCC was
> even
> built with -g3, which might be just me). The attached patch fixes
> this
> issue, I hope, by inlining both the internal definitions of the
> macro.
>
> > > > + if name == 'insn' or name == 'call_insn' or name ==
> > > > 'jump_insn':
> > > > + result += ' %d' % self.gdbval['u2']['insn_uid']
> > > > + icode = self.rtx.XINT(5)
> >
> > (might want a trailing comment here: # INSN_CODE)
>
> Good idea; done.
>
> >
> > > > + if icode != -1:
> > > > + result += ' %s' % get_insn_name(icode)
> > > > + else:
> > > > + result += ' <unrecognized>'
> > > > + elif name == 'reg':
> > > > + regno = self.gdbval['u']['reg']['regno']
> > > > + mode = mode_name(self.gdbval['mode'])
> > > > + result += ':%s %d %s' % (mode, regno,
> > > > reg_name(regno))
> > > > + #else:
> > > > + #format_ = GET_RTX_FORMAT(code)
> > > > + #for i in range(GET_RTX_LENGTH(code)):
> > > > + # print(format_[i])
> >
> > ...though I see that you're commenting out my old implementation
> > for
> > this case. How well does it work? Should we just remove it? Is
> > any
> > of it worth saving? IIRC I tried it, had issues with it, and found
> > the
> > kludge easier, but that was 13 years ago.
>
> This code only printed the format letters (direct to the GDB console,
> without any indication of what they signified), and did not
> contribute
> anything to the pretty-print string. It was vaguely useful for
> developing this code, but not useful to the end user. The new patch
> has
> removed it.
>
> > One other nit: please update the multiline string at the top of the
> > file, in particular the "RTL expressions use a kludge" part, which
> > is
> > no longer true with your improvement.
> Now updated with new examples. I've left a reference to the
> print-rtl.cc printer because it is so much more comprehensive than
> these
> basic summaries. I think there's room for both.
>
> Is the new patch OK?
Yes, thanks (with the caveat that at this point you know the RTL part
of it better than me, so please can you debug it if it breaks)
Dave
