Re: [PATCH 3/3] opcodes/i386: partially implement disassembler style support

[Andrew Burgess <aburgess@redhat.com>]

Jan Beulich via Binutils <binutils@sourceware.org> writes:

> On 17.02.2022 17:15, Andrew Burgess wrote:
>> Jan Beulich via Binutils <binutils@sourceware.org> writes:
>>> On 16.02.2022 21:53, Andrew Burgess via Binutils wrote:
>>>> @@ -9456,12 +9456,18 @@ print_insn (bfd_vma pc, instr_info *ins)
>>>>  	{
>>>>  	  name = prefix_name (ins, priv.the_buffer[0], priv.orig_sizeflag);
>>>>  	  if (name != NULL)
>>>> -	    (*ins->info->fprintf_func) (ins->info->stream, "%s", name);
>>>> +	    (*ins->info->fprintf_styled_func)
>>>> +	      (ins->info->stream, dis_style_mnemonic, "%s", name);
>>>>  	  else
>>>>  	    {
>>>>  	      /* Just print the first byte as a .byte instruction.  */
>>>> -	      (*ins->info->fprintf_func) (ins->info->stream, ".byte 0x%x",
>>>> -				     (unsigned int) priv.the_buffer[0]);
>>>> +	      (*ins->info->fprintf_styled_func)
>>>> +		(ins->info->stream, dis_style_mnemonic, ".byte");
>>>
>>> Perhaps better have dis_style_directive for this? It's certainly not
>>> an insn mnemonic.
>> 
>> Are you suggesting directive in addition to mnemonic?  Or as a
>> replacement for?
>
> In addition to. They're fundamentally different things at the
> assembler level. But that's just my view ...
>
>> My goal with the style list was to try and keep the number of styles
>> pretty small, an instrution mnemonic like 'add' and a directive like
>> '.byte' seemed to have a similar enough function that styling them
>> identically felt OK.
>> 
>>>
>>>> +	      (*ins->info->fprintf_styled_func)
>>>> +		(ins->info->stream, dis_style_text, " ");
>>>> +	      (*ins->info->fprintf_styled_func)
>>>> +		(ins->info->stream, dis_style_immediate, "0x%x",
>>>> +		 (unsigned int) priv.the_buffer[0]);
>>>
>>> I wonder if the naming (dis_style_immediate) isn't misleading. As per
>>> the comment next to its definition it really appears to mean any kind
>>> of number (like is the case here), not just immediate operands of
>>> instructions. Hence maybe dis_style_number (as replacement for or in
>>> addition to dis_style_immediate)?
>> 
>> You mentioned this before in the previous thread, and I didn't really
>> understand then either.
>> 
>> Can you give an example of something that's a number, but not an
>> immediate?  e.g. I wonder (given the instruction/directive distinction
>> you draw above), I wonder if you're conserned about: '.byte 0x4', maybe
>> you don't like referring to this 0x4 here as an immediate?
>
> Well, an operand to a directive for example is not an immediate imo,
> yes. A "load offset" (as your comment calls it) may also not be an
> immediate. E.g. in x86 memory access instructions:
>
> 	mov	0x10(%rbx), %eax
>
> the 0x10 isn't an immediate, but a displacement. The difference may
> be more relevant in something like
>
> 	mov	$0, 0x10(%rbx)
>
> where the $0 is an immediate operand, but the 0x10 isn't (and you
> wouldn't want to mix the two).
>
> From that comment it's not clear to me where else you would think
> "immediate" applies (or not), but in RISC-V's
>
> 	lw	x0, 0x10(x0)
>
> I wouldn't consider the 0x10 an immediate either, albeit this may
> be a result of my x86 bias.

I wonder if there's a name we could come up with that would allow me to
classify the '$0' and '0x10' (in your example above) as the same style?

I've kind-of lost the thread a bit, but maybe that's what the 'number'
you suggested original was for?  If I replaced dis_style_immediate with
dis_style_number, and just replaced thoughout, would that be less
problematic?

Another possibility would be to have some aliases either in the original
enum, as in:

  dis_style_displacement = dis_style_immediate,

or even at the top of i386-dis.c, as in:

  #define dis_style_displacement dis_style_immediate

I really think we should avoid adding too many distinct styles if we
can.  My concern is less about disassembler users handling the different
styles, and more about consistency between the disassemblers.  I figure
it's easier to be consistent if we only have a small number of styles.
If displacement is a different style to other immediate (yes, I'm still
going to call numbers in instruction immediates!) then we end up with
some architectures going one way, and others another.

Anyway, that's just my feelings, if the consensus is that the more
styles the better, then I'm sure we can manage...

Thanks,
Andrew