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

[Andrew Burgess <aburgess@redhat.com>]

Nick Clifton via Binutils <binutils@sourceware.org> writes:

> Hi Andrew,
>
>> I do still have a few questions about how some elements should be
>> styled, consider this instruction:
>> 
>>      add     x1, x2, x3, lsr #1
>>         ~~~~~  ~~  ~~  ~~   ~      Plain text.
>>      ~~~                           Mnemonic.
>>              ~~  ~~  ~~            Register.
>> 	                    ~~    Immediate.
>>                          ???       What to use here?
>> 
>> The current patch formats the 'lsr' as text, but I wonder if this
>> would be better formatted as mnemonic?  Or maybe it should be
>> considered part of the immediate? 
>
> My $0.02 worth: It is not an immediate - in fact that instruction does
> not have any immediates in it - nor is it just plain text.  I suppose
> that you might consider it as being an extension of the mnemonic, but
> that also feels wrong to me.  Could you create a new class for this
> part of the instruction ?  eg 'shifter' or 'sub-mnemonic'.  If not then
> I would go with mnemonic as that is the closest approximation.  IMHO...

We can add more styles.  There's a small bit of work needed when we pass
16 styles, but that really is trivial.

My reluctance is to adding new styles for every single architectural
feature, ideally I'd like to map everything to a very small number of
styles, enough to highlight different parts of the instruction, but not
so many that the output looks like a crazy rainbow of colour.

I initially went with mnemonic, but wasn't sure what people would think
of having the mnemonic split into multiple parts like this.

Adding a sub-mnemonic could be a good solution, it keeps the number of
styles low, but allows us to (potentially) style these parts
differently in the future.

>
>
>> I have a similar question for how to format 'ge' in:
>> 
>>      ccmp    x1, x2, #0xa, ge
>
> The same reasoning applies here I feel.  This is "ccmp-ge" instruction
> with the condition expressed as a separate field in the disassembled
> text.  Ideally a "condition-code" class could be used to express its
> style, but if that is not possible then mnemonic is the next best
> thing.
>
>
>> And how to format 'sxtb' in:
>>  >      adds    x0, sp, w0, sxtb
>
> Ditto.  Maybe an "extender" class could be used here ?
>
>
> The patch itself looks good to me, but I would like to wait to see if
> anyone else has any comments on the code before approving it.

Thanks, I'll merge the first patch from this series, but leave this
second one to see what opinions others have.

Thanks,
Andrew