RE: [PATCH v3 01/12] btrace: Introduce auxiliary instructions.

["Metzger, Markus T" <markus.t.metzger@intel.com>]

Thanks, Felix,

The patch looks good with two small nits.

> /* Instruction flags.  */
>@@ -68,8 +71,19 @@ DEF_ENUM_FLAGS_TYPE (enum btrace_insn_flag,
>btrace_insn_flags);
>    This represents a single instruction in a branch trace.  */
> struct btrace_insn
> {
>-  /* The address of this instruction.  */
>-  CORE_ADDR pc;
>+  union
>+  {
>+    /* The address of this instruction.  Applies to btrace_insn with
>+       iclass == BTRACE_INSN_OTHER or
>+       iclass == BTRACE_INSN_CALL or
>+       iclass == BTRACE_INSN_RETURN or
>+       iclass == BTRACE_INSN_JUMP.  */
>+    CORE_ADDR pc;
>+
>+    /* Index into btrace_info::aux_data.  Applies to btrace_insn with
>+       iclass == BTRACE_INSN_AUX.  */
>+    uint64_t aux_data_index;
>+  };

Should we add size to the union, too, or is this needed for AUX instructions?
How about a name for the union?


>   /* The size of this instruction in bytes.  */
>   gdb_byte size;
>@@ -330,6 +344,10 @@ struct btrace_thread_info
>      function segment i will be at index (i - 1).  */
>   std::vector<btrace_function> functions;
>
>+  /* Optional auxiliary data that is printed in all commands displaying or
>+     stepping through the execution history.  */
>+  std::vector<std::string> aux_data;

This is not the data itself but a human-readable representation, correct?
Should we say 'information' instead of 'data'?

Regards,
Markus.
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