Re: [PATCH v2 0/6] RISC-V: Fix disassembler types and styles

[Palmer Dabbelt <palmer@dabbelt.com>]

On Mon, 03 Oct 2022 02:59:18 PDT (-0700), aburgess@redhat.com wrote:
> Tsukasa OI via Binutils <binutils@sourceware.org> writes:
>
>> Hello,
>>
>> This patchset fixes various typing and styling errors on the RISC-V
>> disassembler.
>
> I can't approve binutils patches.
>
> I've taken a look through this series, and other than the minor typo I
> spotted, this all looks good to me.

My worry with these things is that it's sort of a grey area in terms of 
what our stable user interface is: there's definitely folks who try and 
parse the text output of tools like objdump and changing anything risks 
breaking that.  IMO that's too strict of a stable interface to keep 
around as it pretty much prevents us from fixing any disassembly 
weirdness, but I'm not sure if that's what other ports do and I don't 
want to make a mess by breaking users' expectations here.

That said, as far as I can tell the only user-visible change here is to 
print some bits with the correct syntax coloring in GDB.  Seems pretty 
straight-forward to say anything users can't rely on parsing syntax 
highlighting colors being a stable interface, so maybe we can just punt 
on the grey area for now ;)

> Thanks for doing this.

Yep.  These LGTM, but I'm still kind of buried thanks to the 
a few weeks of conference->COVID.  They're probably fine, but hopefully 
Nelson has the time to chime in -- if not I'll leave them in the queue.

Thanks!

>
> Andrew
>
>
>
>>
>> Previous Patchsets:
>> <https://sourceware.org/pipermail/binutils/2022-July/121790.html> (2022-07-13)
>> <https://sourceware.org/pipermail/binutils/2022-August/122172.html> (2022-08-03)
>> Tracker on GitHub:
>> <https://github.com/a4lg/binutils-gdb/wiki/riscv_dis_printf_styles_and_types_1>
>>
>>
>> [Changes: v1 -> v2]
>>
>> -   Added PATCH 5-6/6 after T-Head extensions are merged.
>> -   Slightly improved the commit messages?
>>
>>
>> [Styling]
>> -   Print real immediates with the `immediate' style         (PATCH 1/6)
>> -   Print comma and tabs with the `text' style               (PATCH 4/6)
>> -   Print T-Head literal operand with `immediate' style      (PATCH 6/6)
>> [Typing on printf]
>> -   Fix wrong type for "%x" format specifier                 (PATCH 2/6)
>> -   Fix minor but various typing issues on T-Head immediates (PATCH 5/6)
>> [Small Optimization]
>> -   Use smallest portable types possible to print            (PATCH 3/6)
>>
>> Thanks,
>> Tsukasa
>>
>>
>>
>>
>> Tsukasa OI (6):
>>   RISC-V: Fix immediates to have "immediate" style
>>   RISC-V: Fix printf argument types corresponding %x
>>   RISC-V: Optimize riscv_disassemble_data printf
>>   RISC-V: Print comma and tabs as the "text" style
>>   RISC-V: Fix T-Head immediate types on printing
>>   RISC-V: Print XTheadMemPair literal as "immediate"
>>
>>  opcodes/riscv-dis.c | 71 +++++++++++++++++++++++++--------------------
>>  1 file changed, 39 insertions(+), 32 deletions(-)
>>
>>
>> base-commit: c21736aed1d4877e090df60362413669dbdc391d
>> --
>> 2.34.1