Re: [PATCH] libopcodes/ppc: add support for disassembler styling

[Alan Modra <amodra@gmail.com>]

On Tue, Jul 12, 2022 at 02:36:09PM +0100, Andrew Burgess via Binutils wrote:
> This commit adds disassembler styling to the libopcodes ppc
> disassembler.  This conversion was pretty straight forward, I just
> converted the fprintf_func calls to fprintf_styled_func calls and
> added an appropriate style.

Thanks for doing this!  The patch is OK as-is, but see comments
below if you'd like to make some improvements.

> For testing the new styling I just assembled then disassembled the
> source files in gas/testsuite/gas/ppc and manually checked that the
> styling looked reasonable.
> 
> I think the only slightly weird case was how things like '4*cr1+eq'
> are styled.  As best I can tell, this construct, used for example in
> this instruction:
> 
>   crand   4*cr1+lt,4*cr1+gt,4*cr1+eq
> 
> is used to access a field of a control register (NOTE: I know very
> little about the PPC ISA, so please correct me if I'm wrong).  As
> such, I have styled the entire construct as a register.  In some cases
> constructs similar to the above can be simplified to just 'eq', as
> this is still referencing a register field, this is still styled as a
> register.

This field is also present in conditional branches, and looking at
those is perhaps is the best way to understand what ought to be done
here.

eg. this instruction
lab: beq 7,lab
disassembles with "objdump -d" to
	beq cr7,0 <lab>
and with "objdump -d -Mraw" to see the underlying hardware insn
	bc 12,4*cr7+eq,0 <lab>

So the field probably ought to disassemble the "4*" and "+" as text,
the "eq" the same as the opcode mnemonic, and the "cr7" as a
register.  At least, that seems a reasonable way to highlight the
"interesting bits" to me.

>  	  else if ((operand->flags & PPC_OPERAND_UDI) != 0)
> -	    (*info->fprintf_func) (info->stream, "%" PRId64, value);
> +	    (*info->fprintf_styled_func) (info->stream, dis_style_text,
> +					  "%" PRId64, value);

I think these are likely best disassembled as registers.  I really
don't know that much about the Xilinx embedded cpu, but I believe it
has these "user defined instructions" where the field is configurable
as a register or immediate.

-- 
Alan Modra
Australia Development Lab, IBM