Re: [PATCH 2/6] disasm: add struct disas_insn to describe to-be-disassembled instruction

[Pedro Alves <palves@redhat.com>]

On 09/21/2015 03:54 PM, Markus Metzger wrote:
> Add a new struct disas_insn to add additional fields describing the
> to-be-disassembled instruction.  The additional fields are:
> 
>   number            an optional instruction number, zero if omitted.
>   is_speculative    a predicate saying whether the instruction was
>                     executed speculatively.
> 
> Replace the PC parameter of dump_insn with a pointer to the above struct.
> 
> If non-zero, the instruction number is printed first.  It will also appear
> as a new optional field "insn-number" in MI.  The field will be present if
> insn_num is non-zero.
> 
> If is_speculative is set, speculative execution will be indicated by a "?"
> following the new instruction number field.  Unless the PC is omitted, it
> will overwrite the first byte of the PC prefix.  It will appear as a new
> optional field "is-speculative" in MI.  The field will contain "?" and will
> be present if is_speculative is set.
> 

I think the log would be much clearer if the rationale was specified in
terms of why this is necessary, and if we saw a before/after example.

Also, being a user/frontend visible change, shouldn't these new
fields be documented and mentioned in NEWS?

> --- a/gdb/disasm.h
> +++ b/gdb/disasm.h
> @@ -27,11 +27,25 @@
>  #define DISASSEMBLY_FILENAME	(0x1 << 3)
>  #define DISASSEMBLY_OMIT_PC	(0x1 << 4)
>  #define DISASSEMBLY_SOURCE	(0x1 << 5)
> +#define DISASSEMBLY_SPECULATIVE	(0x1 << 6)
>  
>  struct gdbarch;
>  struct ui_out;
>  struct ui_file;
>  
> +/* An instruction to be disassembled.  */
> +
> +struct disas_insn {

{ goes on the next line.

Thanks,
Pedro Alves