[PATCH v5 03/10] btrace: Enable auxiliary instructions in record function-call-history.

[Willgerodt, Felix]

Sorry, I forgot to send these out on Friday.

Felix 

> -----Original Message-----
> From: Metzger, Markus T <markus.t.metzger@intel.com>
> Sent: Dienstag, 28. Juni 2022 11:11
> To: Willgerodt, Felix <felix.willgerodt@intel.com>; gdb-
> patches@sourceware.org
> Subject: RE: [PATCH v5 03/10] btrace: Enable auxiliary instructions in record
> function-call-history.
> 
> Hello Felix,
> 
> >Print the auxiliary data when a btrace_insn of type BTRACE_INSN_AUX
> >is encountered in the function-call-history.  Printing is
> >active by default, it can be silenced with the /a modifier.
> 
> I find it a bit strange that /a disables some output instead of enabling it.
> Should we change the default?  I'd probably always use it, but then, I
> also always supply /cli.
>

See the same comment/discussion in patch 2. We agreed that we leave it
as is, as there are already other flags that disable stuff.

> 
> >This patch is in preparation for the new ptwrite feature, which is based on
> >auxiliary instructions.
> >---
> > gdb/btrace.h        |  6 +++++-
> > gdb/doc/gdb.texinfo |  5 +++--
> > gdb/record-btrace.c | 21 +++++++++++++++++++++
> > gdb/record.c        |  5 +++++
> > gdb/record.h        |  5 ++++-
> > 5 files changed, 38 insertions(+), 4 deletions(-)
> 
> 
> >   /* The 'up' link points to a tail call.  This obviously only makes sense
> >      if bfun_up_links_to_ret is clear.  */
> >-  BFUN_UP_LINKS_TO_TAILCALL = (1 << 1)
> >+  BFUN_UP_LINKS_TO_TAILCALL = (1 << 1),
> >+
> >+  /* Indicates that at least one auxiliary instruction is in the current
> >+     function segment.  */
> >+  BFUN_AUX_DECODED = (1 << 2)
> 
> Should this maybe be called BFUN_CONTAINS_AUX?

Fine with me, I renamed it.

> 
> >+static void
> >+btrace_print_aux_insn (struct ui_out *uiout,
> >+		       const struct btrace_function *bfun,
> >+		       const struct btrace_thread_info *btinfo)
> >+{
> >+  for (const btrace_insn &insn : bfun->insn)
> >+    {
> >+      if (insn.iclass == BTRACE_INSN_AUX)
> >+	{
> >+	  uiout->text ("\t\t[");
> 
> This should probably do
> 
>     uiout->field_skip ("index");
>     uiout->text ("\t[");
> 

I tried it locally. This actually isn't equivalent and only one /t will be
printed. I don't think we want that.

Regards,
Felix
Intel Deutschland GmbH
Registered Address: Am Campeon 10, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de <http://www.intel.de>
Managing Directors: Christin Eisenschmid, Sharon Heck, Tiffany Doon Silva  
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928