* [PATCH] RISC-V: fix printf types on riscv-dis.c
@ 2022-07-13 13:40 Tsukasa OI
2022-08-03 4:27 ` [PATCH 0/4] RISC-V: Fix disassembler types and styles Tsukasa OI
0 siblings, 1 reply; 34+ messages in thread
From: Tsukasa OI @ 2022-07-13 13:40 UTC (permalink / raw)
To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils
%x requires `unsigned' type, not `int'. This commit fixes that.
opcodes/ChangeLog:
* riscv-dis.c (print_insn_args): Fix printf types from `int'
to `unsigned'.
---
opcodes/riscv-dis.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 164fd209dbd..c031ce9ae8f 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -283,15 +283,15 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
break;
case 'u':
print (info->stream, dis_style_immediate, "0x%x",
- (int)(EXTRACT_CITYPE_IMM (l) & (RISCV_BIGIMM_REACH-1)));
+ (unsigned)(EXTRACT_CITYPE_IMM (l) & (RISCV_BIGIMM_REACH-1)));
break;
case '>':
print (info->stream, dis_style_immediate, "0x%x",
- (int)EXTRACT_CITYPE_IMM (l) & 0x3f);
+ (unsigned)EXTRACT_CITYPE_IMM (l) & 0x3f);
break;
case '<':
print (info->stream, dis_style_immediate, "0x%x",
- (int)EXTRACT_CITYPE_IMM (l) & 0x1f);
+ (unsigned)EXTRACT_CITYPE_IMM (l) & 0x1f);
break;
case 'T': /* Floating-point RS2. */
print (info->stream, dis_style_register, "%s",
@@ -473,7 +473,7 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
case 'y':
print (info->stream, dis_style_text, "0x%x",
- (int)EXTRACT_OPERAND (BS, l));
+ (unsigned)EXTRACT_OPERAND (BS, l));
break;
case 'z':
@@ -482,12 +482,12 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
case '>':
print (info->stream, dis_style_immediate, "0x%x",
- (int)EXTRACT_OPERAND (SHAMT, l));
+ (unsigned)EXTRACT_OPERAND (SHAMT, l));
break;
case '<':
print (info->stream, dis_style_immediate, "0x%x",
- (int)EXTRACT_OPERAND (SHAMTW, l));
+ (unsigned)EXTRACT_OPERAND (SHAMTW, l));
break;
case 'S':
@@ -547,7 +547,7 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
case 'Y':
print (info->stream, dis_style_text, "0x%x",
- (int) EXTRACT_OPERAND (RNUM, l));
+ (unsigned) EXTRACT_OPERAND (RNUM, l));
break;
case 'Z':
base-commit: dd4c046506cd4da46b439a2b4f8b6d933ecbb961
--
2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 0/4] RISC-V: Fix disassembler types and styles
2022-07-13 13:40 [PATCH] RISC-V: fix printf types on riscv-dis.c Tsukasa OI
@ 2022-08-03 4:27 ` Tsukasa OI
2022-08-03 4:27 ` [PATCH 1/4] RISC-V: Fix immediates to have `immediate' style Tsukasa OI
` (4 more replies)
0 siblings, 5 replies; 34+ messages in thread
From: Tsukasa OI @ 2022-08-03 4:27 UTC (permalink / raw)
To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils
Hello,
This patchset fixes various typing and styling errors on the RISC-V
disassembler. Patches 1 and 2 are considered releatively important
(compared to patches 3 and 4).
Previous Patchset:
<https://sourceware.org/pipermail/binutils/2022-July/121790.html> (2022-07-13)
Tracker on GitHub:
<https://github.com/a4lg/binutils-gdb/wiki/riscv_dis_printf_styles_and_types_1>
[Styling]
- Print real immediates with the `immediate' style (PATCH 1/4)
- Print comma and tabs with the `text' style (PATCH 4/4)
[Typing on printf]
- Fix wrong type for "%x" format specifier (PATCH 2/4)
[Small Optimization]
- Use smallest portable types possible to print (PATCH 3/4)
Thanks,
Tsukasa
Tsukasa OI (4):
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
opcodes/riscv-dis.c | 61 +++++++++++++++++++++++++--------------------
1 file changed, 34 insertions(+), 27 deletions(-)
base-commit: 8aaafe957ceee581e691d2f7f984cb40c58f2b5e
--
2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 1/4] RISC-V: Fix immediates to have `immediate' style
2022-08-03 4:27 ` [PATCH 0/4] RISC-V: Fix disassembler types and styles Tsukasa OI
@ 2022-08-03 4:27 ` Tsukasa OI
2022-08-10 11:16 ` Andrew Burgess
2022-08-03 4:27 ` [PATCH 2/4] RISC-V: Fix printf argument types corresponding %x Tsukasa OI
` (3 subsequent siblings)
4 siblings, 1 reply; 34+ messages in thread
From: Tsukasa OI @ 2022-08-03 4:27 UTC (permalink / raw)
To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils
This commit fixes certain print calls on immediate operands to have
`dis_style_immediate'.
opcodes/ChangeLog:
* riscv-dis.c (print_insn_args): Fix immediates to have
`immediate' style.
(riscv_disassemble_data): Likewise.
---
opcodes/riscv-dis.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 164fd209dbd..fe091f294e7 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -472,7 +472,7 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
break;
case 'y':
- print (info->stream, dis_style_text, "0x%x",
+ print (info->stream, dis_style_immediate, "0x%x",
(int)EXTRACT_OPERAND (BS, l));
break;
@@ -541,17 +541,17 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
if (riscv_csr_hash[csr] != NULL)
print (info->stream, dis_style_text, "%s", riscv_csr_hash[csr]);
else
- print (info->stream, dis_style_text, "0x%x", csr);
+ print (info->stream, dis_style_immediate, "0x%x", csr);
break;
}
case 'Y':
- print (info->stream, dis_style_text, "0x%x",
+ print (info->stream, dis_style_immediate, "0x%x",
(int) EXTRACT_OPERAND (RNUM, l));
break;
case 'Z':
- print (info->stream, dis_style_text, "%d", rs1);
+ print (info->stream, dis_style_immediate, "%d", rs1);
break;
default:
@@ -907,7 +907,7 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
(*info->fprintf_styled_func)
(info->stream, dis_style_assembler_directive, ".byte\t");
(*info->fprintf_styled_func)
- (info->stream, dis_style_assembler_directive, "0x%02llx",
+ (info->stream, dis_style_immediate, "0x%02llx",
(unsigned long long) data);
break;
case 2:
--
2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 2/4] RISC-V: Fix printf argument types corresponding %x
2022-08-03 4:27 ` [PATCH 0/4] RISC-V: Fix disassembler types and styles Tsukasa OI
2022-08-03 4:27 ` [PATCH 1/4] RISC-V: Fix immediates to have `immediate' style Tsukasa OI
@ 2022-08-03 4:27 ` Tsukasa OI
2022-08-03 4:27 ` [PATCH 3/4] RISC-V: Optimize riscv_disassemble_data printf Tsukasa OI
` (2 subsequent siblings)
4 siblings, 0 replies; 34+ messages in thread
From: Tsukasa OI @ 2022-08-03 4:27 UTC (permalink / raw)
To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils
"%x" format specifier requires `unsigned' type, not `int'. This commit
fixes this issue on the RISC-V disassembler.
opcodes/ChangeLog:
* riscv-dis.c (print_insn_args): Fix printf argument types where
the format specifier is "%x".
---
opcodes/riscv-dis.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index fe091f294e7..220a16e294f 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -283,15 +283,15 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
break;
case 'u':
print (info->stream, dis_style_immediate, "0x%x",
- (int)(EXTRACT_CITYPE_IMM (l) & (RISCV_BIGIMM_REACH-1)));
+ (unsigned)(EXTRACT_CITYPE_IMM (l) & (RISCV_BIGIMM_REACH-1)));
break;
case '>':
print (info->stream, dis_style_immediate, "0x%x",
- (int)EXTRACT_CITYPE_IMM (l) & 0x3f);
+ (unsigned)EXTRACT_CITYPE_IMM (l) & 0x3f);
break;
case '<':
print (info->stream, dis_style_immediate, "0x%x",
- (int)EXTRACT_CITYPE_IMM (l) & 0x1f);
+ (unsigned)EXTRACT_CITYPE_IMM (l) & 0x1f);
break;
case 'T': /* Floating-point RS2. */
print (info->stream, dis_style_register, "%s",
@@ -473,7 +473,7 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
case 'y':
print (info->stream, dis_style_immediate, "0x%x",
- (int)EXTRACT_OPERAND (BS, l));
+ (unsigned)EXTRACT_OPERAND (BS, l));
break;
case 'z':
@@ -482,12 +482,12 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
case '>':
print (info->stream, dis_style_immediate, "0x%x",
- (int)EXTRACT_OPERAND (SHAMT, l));
+ (unsigned)EXTRACT_OPERAND (SHAMT, l));
break;
case '<':
print (info->stream, dis_style_immediate, "0x%x",
- (int)EXTRACT_OPERAND (SHAMTW, l));
+ (unsigned)EXTRACT_OPERAND (SHAMTW, l));
break;
case 'S':
@@ -547,7 +547,7 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
case 'Y':
print (info->stream, dis_style_immediate, "0x%x",
- (int) EXTRACT_OPERAND (RNUM, l));
+ (unsigned) EXTRACT_OPERAND (RNUM, l));
break;
case 'Z':
--
2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 3/4] RISC-V: Optimize riscv_disassemble_data printf
2022-08-03 4:27 ` [PATCH 0/4] RISC-V: Fix disassembler types and styles Tsukasa OI
2022-08-03 4:27 ` [PATCH 1/4] RISC-V: Fix immediates to have `immediate' style Tsukasa OI
2022-08-03 4:27 ` [PATCH 2/4] RISC-V: Fix printf argument types corresponding %x Tsukasa OI
@ 2022-08-03 4:27 ` Tsukasa OI
2022-08-03 4:27 ` [PATCH 4/4] RISC-V: Print comma and tabs as the `text' style Tsukasa OI
2022-09-26 12:26 ` [PATCH v2 0/6] RISC-V: Fix disassembler types and styles Tsukasa OI
4 siblings, 0 replies; 34+ messages in thread
From: Tsukasa OI @ 2022-08-03 4:27 UTC (permalink / raw)
To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils
This commit makes types of printf arguments on `riscv_disassemble_data'
as small as possible (as long as we can preserve the portability) to reduce
the cost of printf (especially on 32-bit host).
opcodes/ChangeLog:
* riscv-dis.c (riscv_disassemble_data): Use smallest possible type
to printing data.
---
opcodes/riscv-dis.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 220a16e294f..fe18c18642e 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -907,24 +907,22 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
(*info->fprintf_styled_func)
(info->stream, dis_style_assembler_directive, ".byte\t");
(*info->fprintf_styled_func)
- (info->stream, dis_style_immediate, "0x%02llx",
- (unsigned long long) data);
+ (info->stream, dis_style_immediate, "0x%02x", (unsigned) data);
break;
case 2:
info->bytes_per_line = 8;
(*info->fprintf_styled_func)
(info->stream, dis_style_assembler_directive, ".short\t");
(*info->fprintf_styled_func)
- (info->stream, dis_style_immediate, "0x%04llx",
- (unsigned long long) data);
+ (info->stream, dis_style_immediate, "0x%04x", (unsigned) data);
break;
case 4:
info->bytes_per_line = 8;
(*info->fprintf_styled_func)
(info->stream, dis_style_assembler_directive, ".word\t");
(*info->fprintf_styled_func)
- (info->stream, dis_style_immediate, "0x%08llx",
- (unsigned long long) data);
+ (info->stream, dis_style_immediate, "0x%08lx",
+ (unsigned long) data);
break;
case 8:
info->bytes_per_line = 8;
--
2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 4/4] RISC-V: Print comma and tabs as the `text' style
2022-08-03 4:27 ` [PATCH 0/4] RISC-V: Fix disassembler types and styles Tsukasa OI
` (2 preceding siblings ...)
2022-08-03 4:27 ` [PATCH 3/4] RISC-V: Optimize riscv_disassemble_data printf Tsukasa OI
@ 2022-08-03 4:27 ` Tsukasa OI
2022-08-10 11:20 ` Andrew Burgess
2022-09-26 12:26 ` [PATCH v2 0/6] RISC-V: Fix disassembler types and styles Tsukasa OI
4 siblings, 1 reply; 34+ messages in thread
From: Tsukasa OI @ 2022-08-03 4:27 UTC (permalink / raw)
To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils
On the RISC-V disassembler, some separators have non-`text' style when
printed with another word with another style.
This commit splits those, making sure that those comma and tabs are printed
with the `text' style.
opcodes/ChangeLog:
* riscv-dis.c (print_insn_args): Split and print the comma as text.
(riscv_disassemble_insn): Split and print tabs as text.
(riscv_disassemble_data): Likewise.
---
opcodes/riscv-dis.c | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index fe18c18642e..460f77676d2 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -373,9 +373,12 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
(int)EXTRACT_RVV_OFFSET (l));
break;
case 'm':
- if (! EXTRACT_OPERAND (VMASK, l))
- print (info->stream, dis_style_register, ",%s",
- riscv_vecm_names_numeric[0]);
+ if (!EXTRACT_OPERAND (VMASK, l))
+ {
+ print (info->stream, dis_style_text, ",");
+ print (info->stream, dis_style_register, "%s",
+ riscv_vecm_names_numeric[0]);
+ }
break;
}
break;
@@ -709,7 +712,8 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
case 4:
case 8:
(*info->fprintf_styled_func)
- (info->stream, dis_style_assembler_directive, ".%dbyte\t", insnlen);
+ (info->stream, dis_style_assembler_directive, ".%dbyte", insnlen);
+ (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
(*info->fprintf_styled_func) (info->stream, dis_style_immediate,
"0x%llx", (unsigned long long) word);
break;
@@ -717,7 +721,8 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
{
int i;
(*info->fprintf_styled_func)
- (info->stream, dis_style_assembler_directive, ".byte\t");
+ (info->stream, dis_style_assembler_directive, ".byte");
+ (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
for (i = 0; i < insnlen; ++i)
{
if (i > 0)
@@ -905,21 +910,24 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
case 1:
info->bytes_per_line = 6;
(*info->fprintf_styled_func)
- (info->stream, dis_style_assembler_directive, ".byte\t");
- (*info->fprintf_styled_func)
- (info->stream, dis_style_immediate, "0x%02x", (unsigned) data);
+ (info->stream, dis_style_assembler_directive, ".byte");
+ (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
+ (*info->fprintf_styled_func) (info->stream, dis_style_immediate,
+ "0x%02x", (unsigned)data);
break;
case 2:
info->bytes_per_line = 8;
(*info->fprintf_styled_func)
- (info->stream, dis_style_assembler_directive, ".short\t");
+ (info->stream, dis_style_assembler_directive, ".short");
+ (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
(*info->fprintf_styled_func)
(info->stream, dis_style_immediate, "0x%04x", (unsigned) data);
break;
case 4:
info->bytes_per_line = 8;
(*info->fprintf_styled_func)
- (info->stream, dis_style_assembler_directive, ".word\t");
+ (info->stream, dis_style_assembler_directive, ".word");
+ (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
(*info->fprintf_styled_func)
(info->stream, dis_style_immediate, "0x%08lx",
(unsigned long) data);
@@ -927,7 +935,8 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
case 8:
info->bytes_per_line = 8;
(*info->fprintf_styled_func)
- (info->stream, dis_style_assembler_directive, ".dword\t");
+ (info->stream, dis_style_assembler_directive, ".dword");
+ (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
(*info->fprintf_styled_func)
(info->stream, dis_style_immediate, "0x%016llx",
(unsigned long long) data);
--
2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 1/4] RISC-V: Fix immediates to have `immediate' style
2022-08-03 4:27 ` [PATCH 1/4] RISC-V: Fix immediates to have `immediate' style Tsukasa OI
@ 2022-08-10 11:16 ` Andrew Burgess
2022-08-10 13:48 ` Tsukasa OI
0 siblings, 1 reply; 34+ messages in thread
From: Andrew Burgess @ 2022-08-10 11:16 UTC (permalink / raw)
To: Tsukasa OI via Binutils, Tsukasa OI, Nelson Chu, Kito Cheng,
Palmer Dabbelt
Cc: binutils
Tsukasa OI via Binutils <binutils@sourceware.org> writes:
> This commit fixes certain print calls on immediate operands to have
> `dis_style_immediate'.
>
> opcodes/ChangeLog:
>
> * riscv-dis.c (print_insn_args): Fix immediates to have
> `immediate' style.
> (riscv_disassemble_data): Likewise.
I'm not a binutils maintainer, so can't approve this patch. But...
... this looks good to me, with just one minor comment, see below.
> ---
> opcodes/riscv-dis.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> index 164fd209dbd..fe091f294e7 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -472,7 +472,7 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
> break;
>
> case 'y':
> - print (info->stream, dis_style_text, "0x%x",
> + print (info->stream, dis_style_immediate, "0x%x",
> (int)EXTRACT_OPERAND (BS, l));
> break;
>
> @@ -541,17 +541,17 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
> if (riscv_csr_hash[csr] != NULL)
> print (info->stream, dis_style_text, "%s", riscv_csr_hash[csr]);
You haven't updated this line, but, I wonder if this should be
dis_style_register?
Otherwise, LGTM.
Thanks,
Andrew
> else
> - print (info->stream, dis_style_text, "0x%x", csr);
> + print (info->stream, dis_style_immediate, "0x%x", csr);
> break;
> }
>
> case 'Y':
> - print (info->stream, dis_style_text, "0x%x",
> + print (info->stream, dis_style_immediate, "0x%x",
> (int) EXTRACT_OPERAND (RNUM, l));
> break;
>
> case 'Z':
> - print (info->stream, dis_style_text, "%d", rs1);
> + print (info->stream, dis_style_immediate, "%d", rs1);
> break;
>
> default:
> @@ -907,7 +907,7 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
> (*info->fprintf_styled_func)
> (info->stream, dis_style_assembler_directive, ".byte\t");
> (*info->fprintf_styled_func)
> - (info->stream, dis_style_assembler_directive, "0x%02llx",
> + (info->stream, dis_style_immediate, "0x%02llx",
> (unsigned long long) data);
> break;
> case 2:
> --
> 2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 4/4] RISC-V: Print comma and tabs as the `text' style
2022-08-03 4:27 ` [PATCH 4/4] RISC-V: Print comma and tabs as the `text' style Tsukasa OI
@ 2022-08-10 11:20 ` Andrew Burgess
2022-08-10 13:54 ` Tsukasa OI
0 siblings, 1 reply; 34+ messages in thread
From: Andrew Burgess @ 2022-08-10 11:20 UTC (permalink / raw)
To: Tsukasa OI via Binutils, Tsukasa OI, Nelson Chu, Kito Cheng,
Palmer Dabbelt
Cc: binutils
Tsukasa OI via Binutils <binutils@sourceware.org> writes:
> On the RISC-V disassembler, some separators have non-`text' style when
> printed with another word with another style.
>
> This commit splits those, making sure that those comma and tabs are printed
> with the `text' style.
>
> opcodes/ChangeLog:
>
> * riscv-dis.c (print_insn_args): Split and print the comma as text.
> (riscv_disassemble_insn): Split and print tabs as text.
> (riscv_disassemble_data): Likewise.
I'm not a binutils maintainer, so can't approve this patch. However,
this looks good to me with one minor nit, see below...
> ---
> opcodes/riscv-dis.c | 31 ++++++++++++++++++++-----------
> 1 file changed, 20 insertions(+), 11 deletions(-)
>
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> index fe18c18642e..460f77676d2 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -373,9 +373,12 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
> (int)EXTRACT_RVV_OFFSET (l));
> break;
> case 'm':
> - if (! EXTRACT_OPERAND (VMASK, l))
> - print (info->stream, dis_style_register, ",%s",
> - riscv_vecm_names_numeric[0]);
> + if (!EXTRACT_OPERAND (VMASK, l))
> + {
> + print (info->stream, dis_style_text, ",");
> + print (info->stream, dis_style_register, "%s",
> + riscv_vecm_names_numeric[0]);
> + }
> break;
> }
> break;
> @@ -709,7 +712,8 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
> case 4:
> case 8:
> (*info->fprintf_styled_func)
> - (info->stream, dis_style_assembler_directive, ".%dbyte\t", insnlen);
> + (info->stream, dis_style_assembler_directive, ".%dbyte", insnlen);
> + (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
> (*info->fprintf_styled_func) (info->stream, dis_style_immediate,
> "0x%llx", (unsigned long long) word);
> break;
> @@ -717,7 +721,8 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
> {
> int i;
> (*info->fprintf_styled_func)
> - (info->stream, dis_style_assembler_directive, ".byte\t");
> + (info->stream, dis_style_assembler_directive, ".byte");
> + (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
> for (i = 0; i < insnlen; ++i)
> {
> if (i > 0)
> @@ -905,21 +910,24 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
> case 1:
> info->bytes_per_line = 6;
> (*info->fprintf_styled_func)
> - (info->stream, dis_style_assembler_directive, ".byte\t");
> - (*info->fprintf_styled_func)
> - (info->stream, dis_style_immediate, "0x%02x", (unsigned) data);
> + (info->stream, dis_style_assembler_directive, ".byte");
> + (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
> + (*info->fprintf_styled_func) (info->stream, dis_style_immediate,
> + "0x%02x", (unsigned)data);
You lost the space in '(unsigned) data'.
Thanks,
Andrew
> break;
> case 2:
> info->bytes_per_line = 8;
> (*info->fprintf_styled_func)
> - (info->stream, dis_style_assembler_directive, ".short\t");
> + (info->stream, dis_style_assembler_directive, ".short");
> + (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
> (*info->fprintf_styled_func)
> (info->stream, dis_style_immediate, "0x%04x", (unsigned) data);
> break;
> case 4:
> info->bytes_per_line = 8;
> (*info->fprintf_styled_func)
> - (info->stream, dis_style_assembler_directive, ".word\t");
> + (info->stream, dis_style_assembler_directive, ".word");
> + (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
> (*info->fprintf_styled_func)
> (info->stream, dis_style_immediate, "0x%08lx",
> (unsigned long) data);
> @@ -927,7 +935,8 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
> case 8:
> info->bytes_per_line = 8;
> (*info->fprintf_styled_func)
> - (info->stream, dis_style_assembler_directive, ".dword\t");
> + (info->stream, dis_style_assembler_directive, ".dword");
> + (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
> (*info->fprintf_styled_func)
> (info->stream, dis_style_immediate, "0x%016llx",
> (unsigned long long) data);
> --
> 2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 1/4] RISC-V: Fix immediates to have `immediate' style
2022-08-10 11:16 ` Andrew Burgess
@ 2022-08-10 13:48 ` Tsukasa OI
0 siblings, 0 replies; 34+ messages in thread
From: Tsukasa OI @ 2022-08-10 13:48 UTC (permalink / raw)
To: Andrew Burgess; +Cc: Binutils
On 2022/08/10 20:16, Andrew Burgess wrote:
> Tsukasa OI via Binutils <binutils@sourceware.org> writes:
>
>> This commit fixes certain print calls on immediate operands to have
>> `dis_style_immediate'.
>>
>> opcodes/ChangeLog:
>>
>> * riscv-dis.c (print_insn_args): Fix immediates to have
>> `immediate' style.
>> (riscv_disassemble_data): Likewise.
>
> I'm not a binutils maintainer, so can't approve this patch. But...
>
> ... this looks good to me, with just one minor comment, see below.
>
>> ---
>> opcodes/riscv-dis.c | 10 +++++-----
>> 1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
>> index 164fd209dbd..fe091f294e7 100644
>> --- a/opcodes/riscv-dis.c
>> +++ b/opcodes/riscv-dis.c
>> @@ -472,7 +472,7 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
>> break;
>>
>> case 'y':
>> - print (info->stream, dis_style_text, "0x%x",
>> + print (info->stream, dis_style_immediate, "0x%x",
>> (int)EXTRACT_OPERAND (BS, l));
>> break;
>>
>> @@ -541,17 +541,17 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
>> if (riscv_csr_hash[csr] != NULL)
>> print (info->stream, dis_style_text, "%s", riscv_csr_hash[csr]);
>
> You haven't updated this line, but, I wonder if this should be
> dis_style_register?
That might be an option but I wasn't confident enough.
Maybe RISC-V related developers can talk about it.
For now, I prefer to keep this line.
>
> Otherwise, LGTM.
>
> Thanks,
> Andrew
>
>
>> else
>> - print (info->stream, dis_style_text, "0x%x", csr);
>> + print (info->stream, dis_style_immediate, "0x%x", csr);
>> break;
>> }
>>
>> case 'Y':
>> - print (info->stream, dis_style_text, "0x%x",
>> + print (info->stream, dis_style_immediate, "0x%x",
>> (int) EXTRACT_OPERAND (RNUM, l));
>> break;
>>
>> case 'Z':
>> - print (info->stream, dis_style_text, "%d", rs1);
>> + print (info->stream, dis_style_immediate, "%d", rs1);
>> break;
>>
>> default:
>> @@ -907,7 +907,7 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
>> (*info->fprintf_styled_func)
>> (info->stream, dis_style_assembler_directive, ".byte\t");
>> (*info->fprintf_styled_func)
>> - (info->stream, dis_style_assembler_directive, "0x%02llx",
>> + (info->stream, dis_style_immediate, "0x%02llx",
>> (unsigned long long) data);
>> break;
>> case 2:
>> --
>> 2.34.1
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 4/4] RISC-V: Print comma and tabs as the `text' style
2022-08-10 11:20 ` Andrew Burgess
@ 2022-08-10 13:54 ` Tsukasa OI
0 siblings, 0 replies; 34+ messages in thread
From: Tsukasa OI @ 2022-08-10 13:54 UTC (permalink / raw)
To: Andrew Burgess; +Cc: Binutils
On 2022/08/10 20:20, Andrew Burgess wrote:
> Tsukasa OI via Binutils <binutils@sourceware.org> writes:
>
>> On the RISC-V disassembler, some separators have non-`text' style when
>> printed with another word with another style.
>>
>> This commit splits those, making sure that those comma and tabs are printed
>> with the `text' style.
>>
>> opcodes/ChangeLog:
>>
>> * riscv-dis.c (print_insn_args): Split and print the comma as text.
>> (riscv_disassemble_insn): Split and print tabs as text.
>> (riscv_disassemble_data): Likewise.
>
> I'm not a binutils maintainer, so can't approve this patch. However,
> this looks good to me with one minor nit, see below...
>
>> ---
>> opcodes/riscv-dis.c | 31 ++++++++++++++++++++-----------
>> 1 file changed, 20 insertions(+), 11 deletions(-)
>>
>> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
>> index fe18c18642e..460f77676d2 100644
>> --- a/opcodes/riscv-dis.c
>> +++ b/opcodes/riscv-dis.c
>> @@ -373,9 +373,12 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
>> (int)EXTRACT_RVV_OFFSET (l));
>> break;
>> case 'm':
>> - if (! EXTRACT_OPERAND (VMASK, l))
>> - print (info->stream, dis_style_register, ",%s",
>> - riscv_vecm_names_numeric[0]);
>> + if (!EXTRACT_OPERAND (VMASK, l))
>> + {
>> + print (info->stream, dis_style_text, ",");
>> + print (info->stream, dis_style_register, "%s",
>> + riscv_vecm_names_numeric[0]);
>> + }
>> break;
>> }
>> break;
>> @@ -709,7 +712,8 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
>> case 4:
>> case 8:
>> (*info->fprintf_styled_func)
>> - (info->stream, dis_style_assembler_directive, ".%dbyte\t", insnlen);
>> + (info->stream, dis_style_assembler_directive, ".%dbyte", insnlen);
>> + (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
>> (*info->fprintf_styled_func) (info->stream, dis_style_immediate,
>> "0x%llx", (unsigned long long) word);
>> break;
>> @@ -717,7 +721,8 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
>> {
>> int i;
>> (*info->fprintf_styled_func)
>> - (info->stream, dis_style_assembler_directive, ".byte\t");
>> + (info->stream, dis_style_assembler_directive, ".byte");
>> + (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
>> for (i = 0; i < insnlen; ++i)
>> {
>> if (i > 0)
>> @@ -905,21 +910,24 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
>> case 1:
>> info->bytes_per_line = 6;
>> (*info->fprintf_styled_func)
>> - (info->stream, dis_style_assembler_directive, ".byte\t");
>> - (*info->fprintf_styled_func)
>> - (info->stream, dis_style_immediate, "0x%02x", (unsigned) data);
>> + (info->stream, dis_style_assembler_directive, ".byte");
>> + (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
>> + (*info->fprintf_styled_func) (info->stream, dis_style_immediate,
>> + "0x%02x", (unsigned)data);
>
> You lost the space in '(unsigned) data'.
>
> Thanks,
> Andrew
Interesting.
I formatted this line using Visual Studio Code (which uses clang-format)
and I think this is a minor difference between clang-format's "GNU"
indent and real GNU Indent's defaults.
Changing clang-format's settings just like "SpaceAfterCStyleCast: true"
will mimic the default GNU Indent's behavior (better). It's too minor
(this source file has other (type)(value) without any spaces) and will
not republish the patchset unless there's some other reasons.
But thank you for letting me know that there's something clang-format
misses and I will follow "SpaceAfterCStyleCast: true" in the future.
Thanks,
Tsukasa
>
>> break;
>> case 2:
>> info->bytes_per_line = 8;
>> (*info->fprintf_styled_func)
>> - (info->stream, dis_style_assembler_directive, ".short\t");
>> + (info->stream, dis_style_assembler_directive, ".short");
>> + (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
>> (*info->fprintf_styled_func)
>> (info->stream, dis_style_immediate, "0x%04x", (unsigned) data);
>> break;
>> case 4:
>> info->bytes_per_line = 8;
>> (*info->fprintf_styled_func)
>> - (info->stream, dis_style_assembler_directive, ".word\t");
>> + (info->stream, dis_style_assembler_directive, ".word");
>> + (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
>> (*info->fprintf_styled_func)
>> (info->stream, dis_style_immediate, "0x%08lx",
>> (unsigned long) data);
>> @@ -927,7 +935,8 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
>> case 8:
>> info->bytes_per_line = 8;
>> (*info->fprintf_styled_func)
>> - (info->stream, dis_style_assembler_directive, ".dword\t");
>> + (info->stream, dis_style_assembler_directive, ".dword");
>> + (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
>> (*info->fprintf_styled_func)
>> (info->stream, dis_style_immediate, "0x%016llx",
>> (unsigned long long) data);
>> --
>> 2.34.1
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 0/6] RISC-V: Fix disassembler types and styles
2022-08-03 4:27 ` [PATCH 0/4] RISC-V: Fix disassembler types and styles Tsukasa OI
` (3 preceding siblings ...)
2022-08-03 4:27 ` [PATCH 4/4] RISC-V: Print comma and tabs as the `text' style Tsukasa OI
@ 2022-09-26 12:26 ` Tsukasa OI
2022-09-26 12:26 ` [PATCH v2 1/6] RISC-V: Fix immediates to have "immediate" style Tsukasa OI
` (7 more replies)
4 siblings, 8 replies; 34+ messages in thread
From: Tsukasa OI @ 2022-09-26 12:26 UTC (permalink / raw)
To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils
Hello,
This patchset fixes various typing and styling errors on the RISC-V
disassembler.
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
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 1/6] RISC-V: Fix immediates to have "immediate" style
2022-09-26 12:26 ` [PATCH v2 0/6] RISC-V: Fix disassembler types and styles Tsukasa OI
@ 2022-09-26 12:26 ` Tsukasa OI
2022-09-26 12:26 ` [PATCH v2 2/6] RISC-V: Fix printf argument types corresponding %x Tsukasa OI
` (6 subsequent siblings)
7 siblings, 0 replies; 34+ messages in thread
From: Tsukasa OI @ 2022-09-26 12:26 UTC (permalink / raw)
To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils
This commit fixes certain print calls on immediate operands to have
dis_style_immediate.
opcodes/ChangeLog:
* riscv-dis.c (print_insn_args): Fix immediates to have
"immediate" style. (riscv_disassemble_data): Likewise.
---
opcodes/riscv-dis.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index f2d399260c1..a944e7865fe 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -480,7 +480,7 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
break;
case 'y':
- print (info->stream, dis_style_text, "0x%x",
+ print (info->stream, dis_style_immediate, "0x%x",
(int)EXTRACT_OPERAND (BS, l));
break;
@@ -549,17 +549,17 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
if (riscv_csr_hash[csr] != NULL)
print (info->stream, dis_style_text, "%s", riscv_csr_hash[csr]);
else
- print (info->stream, dis_style_text, "0x%x", csr);
+ print (info->stream, dis_style_immediate, "0x%x", csr);
break;
}
case 'Y':
- print (info->stream, dis_style_text, "0x%x",
+ print (info->stream, dis_style_immediate, "0x%x",
(int) EXTRACT_OPERAND (RNUM, l));
break;
case 'Z':
- print (info->stream, dis_style_text, "%d", rs1);
+ print (info->stream, dis_style_immediate, "%d", rs1);
break;
case 'X': /* Integer immediate. */
@@ -963,7 +963,7 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
(*info->fprintf_styled_func)
(info->stream, dis_style_assembler_directive, ".byte\t");
(*info->fprintf_styled_func)
- (info->stream, dis_style_assembler_directive, "0x%02llx",
+ (info->stream, dis_style_immediate, "0x%02llx",
(unsigned long long) data);
break;
case 2:
--
2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 2/6] RISC-V: Fix printf argument types corresponding %x
2022-09-26 12:26 ` [PATCH v2 0/6] RISC-V: Fix disassembler types and styles Tsukasa OI
2022-09-26 12:26 ` [PATCH v2 1/6] RISC-V: Fix immediates to have "immediate" style Tsukasa OI
@ 2022-09-26 12:26 ` Tsukasa OI
2022-09-26 12:26 ` [PATCH v2 3/6] RISC-V: Optimize riscv_disassemble_data printf Tsukasa OI
` (5 subsequent siblings)
7 siblings, 0 replies; 34+ messages in thread
From: Tsukasa OI @ 2022-09-26 12:26 UTC (permalink / raw)
To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils
"%x" format specifier requires unsigned type, not int. This commit
fixes this issue on the RISC-V disassembler.
opcodes/ChangeLog:
* riscv-dis.c (print_insn_args): Fix printf argument types where
the format specifier is "%x".
---
opcodes/riscv-dis.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index a944e7865fe..7adae3a1cd7 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -292,15 +292,15 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
break;
case 'u':
print (info->stream, dis_style_immediate, "0x%x",
- (int)(EXTRACT_CITYPE_IMM (l) & (RISCV_BIGIMM_REACH-1)));
+ (unsigned)(EXTRACT_CITYPE_IMM (l) & (RISCV_BIGIMM_REACH-1)));
break;
case '>':
print (info->stream, dis_style_immediate, "0x%x",
- (int)EXTRACT_CITYPE_IMM (l) & 0x3f);
+ (unsigned)EXTRACT_CITYPE_IMM (l) & 0x3f);
break;
case '<':
print (info->stream, dis_style_immediate, "0x%x",
- (int)EXTRACT_CITYPE_IMM (l) & 0x1f);
+ (unsigned)EXTRACT_CITYPE_IMM (l) & 0x1f);
break;
case 'T': /* Floating-point RS2. */
print (info->stream, dis_style_register, "%s",
@@ -481,7 +481,7 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
case 'y':
print (info->stream, dis_style_immediate, "0x%x",
- (int)EXTRACT_OPERAND (BS, l));
+ (unsigned)EXTRACT_OPERAND (BS, l));
break;
case 'z':
@@ -490,12 +490,12 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
case '>':
print (info->stream, dis_style_immediate, "0x%x",
- (int)EXTRACT_OPERAND (SHAMT, l));
+ (unsigned)EXTRACT_OPERAND (SHAMT, l));
break;
case '<':
print (info->stream, dis_style_immediate, "0x%x",
- (int)EXTRACT_OPERAND (SHAMTW, l));
+ (unsigned)EXTRACT_OPERAND (SHAMTW, l));
break;
case 'S':
@@ -555,7 +555,7 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
case 'Y':
print (info->stream, dis_style_immediate, "0x%x",
- (int) EXTRACT_OPERAND (RNUM, l));
+ (unsigned) EXTRACT_OPERAND (RNUM, l));
break;
case 'Z':
--
2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 3/6] RISC-V: Optimize riscv_disassemble_data printf
2022-09-26 12:26 ` [PATCH v2 0/6] RISC-V: Fix disassembler types and styles Tsukasa OI
2022-09-26 12:26 ` [PATCH v2 1/6] RISC-V: Fix immediates to have "immediate" style Tsukasa OI
2022-09-26 12:26 ` [PATCH v2 2/6] RISC-V: Fix printf argument types corresponding %x Tsukasa OI
@ 2022-09-26 12:26 ` Tsukasa OI
2022-09-26 12:26 ` [PATCH v2 4/6] RISC-V: Print comma and tabs as the "text" style Tsukasa OI
` (4 subsequent siblings)
7 siblings, 0 replies; 34+ messages in thread
From: Tsukasa OI @ 2022-09-26 12:26 UTC (permalink / raw)
To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils
This commit makes types of printf arguments on riscv_disassemble_data
as small as possible (as long as we can preserve the portability) to reduce
the cost of printf (especially on 32-bit host).
opcodes/ChangeLog:
* riscv-dis.c (riscv_disassemble_data): Use smallest possible type
to printing data.
---
opcodes/riscv-dis.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 7adae3a1cd7..30cd90548a0 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -963,24 +963,22 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
(*info->fprintf_styled_func)
(info->stream, dis_style_assembler_directive, ".byte\t");
(*info->fprintf_styled_func)
- (info->stream, dis_style_immediate, "0x%02llx",
- (unsigned long long) data);
+ (info->stream, dis_style_immediate, "0x%02x", (unsigned) data);
break;
case 2:
info->bytes_per_line = 8;
(*info->fprintf_styled_func)
(info->stream, dis_style_assembler_directive, ".short\t");
(*info->fprintf_styled_func)
- (info->stream, dis_style_immediate, "0x%04llx",
- (unsigned long long) data);
+ (info->stream, dis_style_immediate, "0x%04x", (unsigned) data);
break;
case 4:
info->bytes_per_line = 8;
(*info->fprintf_styled_func)
(info->stream, dis_style_assembler_directive, ".word\t");
(*info->fprintf_styled_func)
- (info->stream, dis_style_immediate, "0x%08llx",
- (unsigned long long) data);
+ (info->stream, dis_style_immediate, "0x%08lx",
+ (unsigned long) data);
break;
case 8:
info->bytes_per_line = 8;
--
2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 4/6] RISC-V: Print comma and tabs as the "text" style
2022-09-26 12:26 ` [PATCH v2 0/6] RISC-V: Fix disassembler types and styles Tsukasa OI
` (2 preceding siblings ...)
2022-09-26 12:26 ` [PATCH v2 3/6] RISC-V: Optimize riscv_disassemble_data printf Tsukasa OI
@ 2022-09-26 12:26 ` Tsukasa OI
2022-09-26 12:26 ` [PATCH v2 5/6] RISC-V: Fix T-Head immediate types on printing Tsukasa OI
` (3 subsequent siblings)
7 siblings, 0 replies; 34+ messages in thread
From: Tsukasa OI @ 2022-09-26 12:26 UTC (permalink / raw)
To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils
On the RISC-V disassembler, some separators have non-text style when
printed with another word with another style.
This commit splits those, making sure that those comma and tabs are printed
with the "text" style.
opcodes/ChangeLog:
* riscv-dis.c (print_insn_args): Split and print the comma as
text. (riscv_disassemble_insn): Split and print tabs as text.
(riscv_disassemble_data): Likewise.
---
opcodes/riscv-dis.c | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 30cd90548a0..1c75317fad1 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -382,9 +382,12 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
(int)EXTRACT_RVV_OFFSET (l));
break;
case 'm':
- if (! EXTRACT_OPERAND (VMASK, l))
- print (info->stream, dis_style_register, ",%s",
- riscv_vecm_names_numeric[0]);
+ if (!EXTRACT_OPERAND (VMASK, l))
+ {
+ print (info->stream, dis_style_text, ",");
+ print (info->stream, dis_style_register, "%s",
+ riscv_vecm_names_numeric[0]);
+ }
break;
}
break;
@@ -765,7 +768,8 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
case 4:
case 8:
(*info->fprintf_styled_func)
- (info->stream, dis_style_assembler_directive, ".%dbyte\t", insnlen);
+ (info->stream, dis_style_assembler_directive, ".%dbyte", insnlen);
+ (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
(*info->fprintf_styled_func) (info->stream, dis_style_immediate,
"0x%llx", (unsigned long long) word);
break;
@@ -773,7 +777,8 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
{
int i;
(*info->fprintf_styled_func)
- (info->stream, dis_style_assembler_directive, ".byte\t");
+ (info->stream, dis_style_assembler_directive, ".byte");
+ (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
for (i = 0; i < insnlen; ++i)
{
if (i > 0)
@@ -961,21 +966,24 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
case 1:
info->bytes_per_line = 6;
(*info->fprintf_styled_func)
- (info->stream, dis_style_assembler_directive, ".byte\t");
- (*info->fprintf_styled_func)
- (info->stream, dis_style_immediate, "0x%02x", (unsigned) data);
+ (info->stream, dis_style_assembler_directive, ".byte");
+ (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
+ (*info->fprintf_styled_func) (info->stream, dis_style_immediate,
+ "0x%02x", (unsigned)data);
break;
case 2:
info->bytes_per_line = 8;
(*info->fprintf_styled_func)
- (info->stream, dis_style_assembler_directive, ".short\t");
+ (info->stream, dis_style_assembler_directive, ".short");
+ (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
(*info->fprintf_styled_func)
(info->stream, dis_style_immediate, "0x%04x", (unsigned) data);
break;
case 4:
info->bytes_per_line = 8;
(*info->fprintf_styled_func)
- (info->stream, dis_style_assembler_directive, ".word\t");
+ (info->stream, dis_style_assembler_directive, ".word");
+ (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
(*info->fprintf_styled_func)
(info->stream, dis_style_immediate, "0x%08lx",
(unsigned long) data);
@@ -983,7 +991,8 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
case 8:
info->bytes_per_line = 8;
(*info->fprintf_styled_func)
- (info->stream, dis_style_assembler_directive, ".dword\t");
+ (info->stream, dis_style_assembler_directive, ".dword");
+ (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
(*info->fprintf_styled_func)
(info->stream, dis_style_immediate, "0x%016llx",
(unsigned long long) data);
--
2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 5/6] RISC-V: Fix T-Head immediate types on printing
2022-09-26 12:26 ` [PATCH v2 0/6] RISC-V: Fix disassembler types and styles Tsukasa OI
` (3 preceding siblings ...)
2022-09-26 12:26 ` [PATCH v2 4/6] RISC-V: Print comma and tabs as the "text" style Tsukasa OI
@ 2022-09-26 12:26 ` Tsukasa OI
2022-10-03 9:57 ` Andrew Burgess
2022-10-03 11:06 ` Christoph Müllner
2022-09-26 12:26 ` [PATCH v2 6/6] RISC-V: Print XTheadMemPair literal as "immediate" Tsukasa OI
` (2 subsequent siblings)
7 siblings, 2 replies; 34+ messages in thread
From: Tsukasa OI @ 2022-09-26 12:26 UTC (permalink / raw)
To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils
This commit fixes three minor typing-related issues for
T-Head immediate values.
1. The format string %i is renamed to %d (which is more common).
2. Signed type must be specified when printing with %d.
3. unsigned/signed int it not portable enough for max 32-bit immediates.
Instead, we should use unsigned/signed long.
The format string is changed accordingly.
opcodes/ChangeLog:
* riscv-dis.c (print_insn_args): Fix T-Head immediate types on
printing.
---
opcodes/riscv-dis.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 1c75317fad1..df2a6f4e131 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -596,11 +596,11 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
oparg--;
if (!sign)
- print (info->stream, dis_style_immediate, "%u",
- (unsigned)EXTRACT_U_IMM (n, s, l));
+ print (info->stream, dis_style_immediate, "%lu",
+ (unsigned long)EXTRACT_U_IMM (n, s, l));
else
- print (info->stream, dis_style_immediate, "%i",
- (unsigned)EXTRACT_S_IMM (n, s, l));
+ print (info->stream, dis_style_immediate, "%ld",
+ (signed long)EXTRACT_S_IMM (n, s, l));
break;
default:
goto undefined_modifier;
--
2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v2 6/6] RISC-V: Print XTheadMemPair literal as "immediate"
2022-09-26 12:26 ` [PATCH v2 0/6] RISC-V: Fix disassembler types and styles Tsukasa OI
` (4 preceding siblings ...)
2022-09-26 12:26 ` [PATCH v2 5/6] RISC-V: Fix T-Head immediate types on printing Tsukasa OI
@ 2022-09-26 12:26 ` Tsukasa OI
2022-10-03 11:06 ` Christoph Müllner
2022-10-03 9:59 ` [PATCH v2 0/6] RISC-V: Fix disassembler types and styles Andrew Burgess
2022-10-03 10:43 ` [PATCH v3 " Tsukasa OI
7 siblings, 1 reply; 34+ messages in thread
From: Tsukasa OI @ 2022-09-26 12:26 UTC (permalink / raw)
To: Tsukasa OI, Nelson Chu, Kito Cheng, Palmer Dabbelt; +Cc: binutils
The operand type "Xl(...)" denotes that (...) is a literal. Specifically,
they are intended to be a constant immediate value.
This commit prints "Xl(...)" operand with dis_style_immediate style,
not dis_style_text.
opcodes/ChangeLog:
* riscv-dis.c (print_insn_args): Use dis_style_immediate on
the constant literal of the "Xl..." operand.
---
opcodes/riscv-dis.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index df2a6f4e131..7aa45db75f2 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -577,7 +577,7 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
oparg++;
while (*oparg && *oparg != ',')
{
- print (info->stream, dis_style_text, "%c", *oparg);
+ print (info->stream, dis_style_immediate, "%c", *oparg);
oparg++;
}
oparg--;
--
2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 5/6] RISC-V: Fix T-Head immediate types on printing
2022-09-26 12:26 ` [PATCH v2 5/6] RISC-V: Fix T-Head immediate types on printing Tsukasa OI
@ 2022-10-03 9:57 ` Andrew Burgess
2022-10-03 11:06 ` Christoph Müllner
1 sibling, 0 replies; 34+ messages in thread
From: Andrew Burgess @ 2022-10-03 9:57 UTC (permalink / raw)
To: Tsukasa OI via Binutils, Tsukasa OI, Nelson Chu, Kito Cheng,
Palmer Dabbelt
Cc: binutils
Tsukasa OI via Binutils <binutils@sourceware.org> writes:
> This commit fixes three minor typing-related issues for
> T-Head immediate values.
>
> 1. The format string %i is renamed to %d (which is more common).
> 2. Signed type must be specified when printing with %d.
> 3. unsigned/signed int it not portable enough for max 32-bit immediates.
typo: `it` should be `is`.
Thanks,
Andrew
> Instead, we should use unsigned/signed long.
> The format string is changed accordingly.
>
> opcodes/ChangeLog:
>
> * riscv-dis.c (print_insn_args): Fix T-Head immediate types on
> printing.
> ---
> opcodes/riscv-dis.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> index 1c75317fad1..df2a6f4e131 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -596,11 +596,11 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
> oparg--;
>
> if (!sign)
> - print (info->stream, dis_style_immediate, "%u",
> - (unsigned)EXTRACT_U_IMM (n, s, l));
> + print (info->stream, dis_style_immediate, "%lu",
> + (unsigned long)EXTRACT_U_IMM (n, s, l));
> else
> - print (info->stream, dis_style_immediate, "%i",
> - (unsigned)EXTRACT_S_IMM (n, s, l));
> + print (info->stream, dis_style_immediate, "%ld",
> + (signed long)EXTRACT_S_IMM (n, s, l));
> break;
> default:
> goto undefined_modifier;
> --
> 2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 0/6] RISC-V: Fix disassembler types and styles
2022-09-26 12:26 ` [PATCH v2 0/6] RISC-V: Fix disassembler types and styles Tsukasa OI
` (5 preceding siblings ...)
2022-09-26 12:26 ` [PATCH v2 6/6] RISC-V: Print XTheadMemPair literal as "immediate" Tsukasa OI
@ 2022-10-03 9:59 ` Andrew Burgess
2022-10-03 17:40 ` Palmer Dabbelt
2022-10-03 10:43 ` [PATCH v3 " Tsukasa OI
7 siblings, 1 reply; 34+ messages in thread
From: Andrew Burgess @ 2022-10-03 9:59 UTC (permalink / raw)
To: Tsukasa OI via Binutils, Tsukasa OI, Nelson Chu, Kito Cheng,
Palmer Dabbelt
Cc: binutils
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.
Thanks for doing this.
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
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 0/6] RISC-V: Fix disassembler types and styles
2022-09-26 12:26 ` [PATCH v2 0/6] RISC-V: Fix disassembler types and styles Tsukasa OI
` (6 preceding siblings ...)
2022-10-03 9:59 ` [PATCH v2 0/6] RISC-V: Fix disassembler types and styles Andrew Burgess
@ 2022-10-03 10:43 ` Tsukasa OI
2022-10-03 10:43 ` [PATCH v3 1/6] RISC-V: Fix immediates to have "immediate" style Tsukasa OI
` (5 more replies)
7 siblings, 6 replies; 34+ messages in thread
From: Tsukasa OI @ 2022-10-03 10:43 UTC (permalink / raw)
To: Tsukasa OI, Christoph Müllner, Nelson Chu, Kito Cheng,
Palmer Dabbelt, Andrew Burgess
Cc: binutils
Hello,
This patchset fixes various typing and styling errors on the RISC-V
disassembler.
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)
<https://sourceware.org/pipermail/binutils/2022-September/123090.html> (2022-09-26)
Tracker on GitHub:
<https://github.com/a4lg/binutils-gdb/wiki/riscv_dis_printf_styles_and_types_1>
[Changes: v2 -> v3]
- PATCH 5/6: Changed to %li instead of %ld I proposed (before patch: %i)
because changing to %d-based felt too intrusive.
- PATCH 5/6: Fixed a typo
Thanks to Andrew Burgess for feedback!
[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: 2820f08f2331820808ac2f54d8e3619f36ea258b
--
2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 1/6] RISC-V: Fix immediates to have "immediate" style
2022-10-03 10:43 ` [PATCH v3 " Tsukasa OI
@ 2022-10-03 10:43 ` Tsukasa OI
2022-10-03 10:44 ` [PATCH v3 2/6] RISC-V: Fix printf argument types corresponding %x Tsukasa OI
` (4 subsequent siblings)
5 siblings, 0 replies; 34+ messages in thread
From: Tsukasa OI @ 2022-10-03 10:43 UTC (permalink / raw)
To: Tsukasa OI, Christoph Müllner, Nelson Chu, Kito Cheng,
Palmer Dabbelt, Andrew Burgess
Cc: binutils
This commit fixes certain print calls on immediate operands to have
dis_style_immediate.
opcodes/ChangeLog:
* riscv-dis.c (print_insn_args): Fix immediates to have
"immediate" style. (riscv_disassemble_data): Likewise.
---
opcodes/riscv-dis.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 6ac69490b78..a5bfa97058a 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -480,7 +480,7 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
break;
case 'y':
- print (info->stream, dis_style_text, "0x%x",
+ print (info->stream, dis_style_immediate, "0x%x",
(int)EXTRACT_OPERAND (BS, l));
break;
@@ -549,17 +549,17 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
if (riscv_csr_hash[csr] != NULL)
print (info->stream, dis_style_text, "%s", riscv_csr_hash[csr]);
else
- print (info->stream, dis_style_text, "0x%x", csr);
+ print (info->stream, dis_style_immediate, "0x%x", csr);
break;
}
case 'Y':
- print (info->stream, dis_style_text, "0x%x",
+ print (info->stream, dis_style_immediate, "0x%x",
(int) EXTRACT_OPERAND (RNUM, l));
break;
case 'Z':
- print (info->stream, dis_style_text, "%d", rs1);
+ print (info->stream, dis_style_immediate, "%d", rs1);
break;
case 'X': /* Integer immediate. */
@@ -963,7 +963,7 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
(*info->fprintf_styled_func)
(info->stream, dis_style_assembler_directive, ".byte\t");
(*info->fprintf_styled_func)
- (info->stream, dis_style_assembler_directive, "0x%02llx",
+ (info->stream, dis_style_immediate, "0x%02llx",
(unsigned long long) data);
break;
case 2:
--
2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 2/6] RISC-V: Fix printf argument types corresponding %x
2022-10-03 10:43 ` [PATCH v3 " Tsukasa OI
2022-10-03 10:43 ` [PATCH v3 1/6] RISC-V: Fix immediates to have "immediate" style Tsukasa OI
@ 2022-10-03 10:44 ` Tsukasa OI
2022-10-03 10:44 ` [PATCH v3 3/6] RISC-V: Optimize riscv_disassemble_data printf Tsukasa OI
` (3 subsequent siblings)
5 siblings, 0 replies; 34+ messages in thread
From: Tsukasa OI @ 2022-10-03 10:44 UTC (permalink / raw)
To: Tsukasa OI, Christoph Müllner, Nelson Chu, Kito Cheng,
Palmer Dabbelt, Andrew Burgess
Cc: binutils
"%x" format specifier requires unsigned type, not int. This commit
fixes this issue on the RISC-V disassembler.
opcodes/ChangeLog:
* riscv-dis.c (print_insn_args): Fix printf argument types where
the format specifier is "%x".
---
opcodes/riscv-dis.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index a5bfa97058a..503b839f19e 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -292,15 +292,15 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
break;
case 'u':
print (info->stream, dis_style_immediate, "0x%x",
- (int)(EXTRACT_CITYPE_IMM (l) & (RISCV_BIGIMM_REACH-1)));
+ (unsigned)(EXTRACT_CITYPE_IMM (l) & (RISCV_BIGIMM_REACH-1)));
break;
case '>':
print (info->stream, dis_style_immediate, "0x%x",
- (int)EXTRACT_CITYPE_IMM (l) & 0x3f);
+ (unsigned)EXTRACT_CITYPE_IMM (l) & 0x3f);
break;
case '<':
print (info->stream, dis_style_immediate, "0x%x",
- (int)EXTRACT_CITYPE_IMM (l) & 0x1f);
+ (unsigned)EXTRACT_CITYPE_IMM (l) & 0x1f);
break;
case 'T': /* Floating-point RS2. */
print (info->stream, dis_style_register, "%s",
@@ -481,7 +481,7 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
case 'y':
print (info->stream, dis_style_immediate, "0x%x",
- (int)EXTRACT_OPERAND (BS, l));
+ (unsigned)EXTRACT_OPERAND (BS, l));
break;
case 'z':
@@ -490,12 +490,12 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
case '>':
print (info->stream, dis_style_immediate, "0x%x",
- (int)EXTRACT_OPERAND (SHAMT, l));
+ (unsigned)EXTRACT_OPERAND (SHAMT, l));
break;
case '<':
print (info->stream, dis_style_immediate, "0x%x",
- (int)EXTRACT_OPERAND (SHAMTW, l));
+ (unsigned)EXTRACT_OPERAND (SHAMTW, l));
break;
case 'S':
@@ -555,7 +555,7 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
case 'Y':
print (info->stream, dis_style_immediate, "0x%x",
- (int) EXTRACT_OPERAND (RNUM, l));
+ (unsigned) EXTRACT_OPERAND (RNUM, l));
break;
case 'Z':
--
2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 3/6] RISC-V: Optimize riscv_disassemble_data printf
2022-10-03 10:43 ` [PATCH v3 " Tsukasa OI
2022-10-03 10:43 ` [PATCH v3 1/6] RISC-V: Fix immediates to have "immediate" style Tsukasa OI
2022-10-03 10:44 ` [PATCH v3 2/6] RISC-V: Fix printf argument types corresponding %x Tsukasa OI
@ 2022-10-03 10:44 ` Tsukasa OI
2022-10-03 10:44 ` [PATCH v3 4/6] RISC-V: Print comma and tabs as the "text" style Tsukasa OI
` (2 subsequent siblings)
5 siblings, 0 replies; 34+ messages in thread
From: Tsukasa OI @ 2022-10-03 10:44 UTC (permalink / raw)
To: Tsukasa OI, Christoph Müllner, Nelson Chu, Kito Cheng,
Palmer Dabbelt, Andrew Burgess
Cc: binutils
This commit makes types of printf arguments on riscv_disassemble_data
as small as possible (as long as we can preserve the portability) to reduce
the cost of printf (especially on 32-bit host).
opcodes/ChangeLog:
* riscv-dis.c (riscv_disassemble_data): Use smallest possible type
to printing data.
---
opcodes/riscv-dis.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 503b839f19e..70f484189f5 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -963,24 +963,22 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
(*info->fprintf_styled_func)
(info->stream, dis_style_assembler_directive, ".byte\t");
(*info->fprintf_styled_func)
- (info->stream, dis_style_immediate, "0x%02llx",
- (unsigned long long) data);
+ (info->stream, dis_style_immediate, "0x%02x", (unsigned) data);
break;
case 2:
info->bytes_per_line = 8;
(*info->fprintf_styled_func)
(info->stream, dis_style_assembler_directive, ".short\t");
(*info->fprintf_styled_func)
- (info->stream, dis_style_immediate, "0x%04llx",
- (unsigned long long) data);
+ (info->stream, dis_style_immediate, "0x%04x", (unsigned) data);
break;
case 4:
info->bytes_per_line = 8;
(*info->fprintf_styled_func)
(info->stream, dis_style_assembler_directive, ".word\t");
(*info->fprintf_styled_func)
- (info->stream, dis_style_immediate, "0x%08llx",
- (unsigned long long) data);
+ (info->stream, dis_style_immediate, "0x%08lx",
+ (unsigned long) data);
break;
case 8:
info->bytes_per_line = 8;
--
2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 4/6] RISC-V: Print comma and tabs as the "text" style
2022-10-03 10:43 ` [PATCH v3 " Tsukasa OI
` (2 preceding siblings ...)
2022-10-03 10:44 ` [PATCH v3 3/6] RISC-V: Optimize riscv_disassemble_data printf Tsukasa OI
@ 2022-10-03 10:44 ` Tsukasa OI
2022-10-03 10:44 ` [PATCH v3 5/6] RISC-V: Fix T-Head immediate types on printing Tsukasa OI
2022-10-03 10:44 ` [PATCH v3 6/6] RISC-V: Print XTheadMemPair literal as "immediate" Tsukasa OI
5 siblings, 0 replies; 34+ messages in thread
From: Tsukasa OI @ 2022-10-03 10:44 UTC (permalink / raw)
To: Tsukasa OI, Christoph Müllner, Nelson Chu, Kito Cheng,
Palmer Dabbelt, Andrew Burgess
Cc: binutils
On the RISC-V disassembler, some separators have non-text style when
printed with another word with another style.
This commit splits those, making sure that those comma and tabs are printed
with the "text" style.
opcodes/ChangeLog:
* riscv-dis.c (print_insn_args): Split and print the comma as
text. (riscv_disassemble_insn): Split and print tabs as text.
(riscv_disassemble_data): Likewise.
---
opcodes/riscv-dis.c | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 70f484189f5..7f5ca4dcb85 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -382,9 +382,12 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
(int)EXTRACT_RVV_OFFSET (l));
break;
case 'm':
- if (! EXTRACT_OPERAND (VMASK, l))
- print (info->stream, dis_style_register, ",%s",
- riscv_vecm_names_numeric[0]);
+ if (!EXTRACT_OPERAND (VMASK, l))
+ {
+ print (info->stream, dis_style_text, ",");
+ print (info->stream, dis_style_register, "%s",
+ riscv_vecm_names_numeric[0]);
+ }
break;
}
break;
@@ -765,7 +768,8 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
case 4:
case 8:
(*info->fprintf_styled_func)
- (info->stream, dis_style_assembler_directive, ".%dbyte\t", insnlen);
+ (info->stream, dis_style_assembler_directive, ".%dbyte", insnlen);
+ (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
(*info->fprintf_styled_func) (info->stream, dis_style_immediate,
"0x%llx", (unsigned long long) word);
break;
@@ -773,7 +777,8 @@ riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
{
int i;
(*info->fprintf_styled_func)
- (info->stream, dis_style_assembler_directive, ".byte\t");
+ (info->stream, dis_style_assembler_directive, ".byte");
+ (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
for (i = 0; i < insnlen; ++i)
{
if (i > 0)
@@ -961,21 +966,24 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
case 1:
info->bytes_per_line = 6;
(*info->fprintf_styled_func)
- (info->stream, dis_style_assembler_directive, ".byte\t");
- (*info->fprintf_styled_func)
- (info->stream, dis_style_immediate, "0x%02x", (unsigned) data);
+ (info->stream, dis_style_assembler_directive, ".byte");
+ (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
+ (*info->fprintf_styled_func) (info->stream, dis_style_immediate,
+ "0x%02x", (unsigned)data);
break;
case 2:
info->bytes_per_line = 8;
(*info->fprintf_styled_func)
- (info->stream, dis_style_assembler_directive, ".short\t");
+ (info->stream, dis_style_assembler_directive, ".short");
+ (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
(*info->fprintf_styled_func)
(info->stream, dis_style_immediate, "0x%04x", (unsigned) data);
break;
case 4:
info->bytes_per_line = 8;
(*info->fprintf_styled_func)
- (info->stream, dis_style_assembler_directive, ".word\t");
+ (info->stream, dis_style_assembler_directive, ".word");
+ (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
(*info->fprintf_styled_func)
(info->stream, dis_style_immediate, "0x%08lx",
(unsigned long) data);
@@ -983,7 +991,8 @@ riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
case 8:
info->bytes_per_line = 8;
(*info->fprintf_styled_func)
- (info->stream, dis_style_assembler_directive, ".dword\t");
+ (info->stream, dis_style_assembler_directive, ".dword");
+ (*info->fprintf_styled_func) (info->stream, dis_style_text, "\t");
(*info->fprintf_styled_func)
(info->stream, dis_style_immediate, "0x%016llx",
(unsigned long long) data);
--
2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 5/6] RISC-V: Fix T-Head immediate types on printing
2022-10-03 10:43 ` [PATCH v3 " Tsukasa OI
` (3 preceding siblings ...)
2022-10-03 10:44 ` [PATCH v3 4/6] RISC-V: Print comma and tabs as the "text" style Tsukasa OI
@ 2022-10-03 10:44 ` Tsukasa OI
2022-10-03 10:44 ` [PATCH v3 6/6] RISC-V: Print XTheadMemPair literal as "immediate" Tsukasa OI
5 siblings, 0 replies; 34+ messages in thread
From: Tsukasa OI @ 2022-10-03 10:44 UTC (permalink / raw)
To: Tsukasa OI, Christoph Müllner, Nelson Chu, Kito Cheng,
Palmer Dabbelt, Andrew Burgess
Cc: binutils
This commit fixes two minor typing-related issues for
T-Head immediate operands.
1. A signed type must be specified when printing with %i.
2. unsigned/signed int is not portable enough for max 32-bit immediates.
Instead, we should use unsigned/signed long.
The format string is changed accordingly.
opcodes/ChangeLog:
* riscv-dis.c (print_insn_args): Fix T-Head immediate types on
printing.
---
opcodes/riscv-dis.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 7f5ca4dcb85..c0edc4c7d6b 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -596,11 +596,11 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
oparg--;
if (!sign)
- print (info->stream, dis_style_immediate, "%u",
- (unsigned)EXTRACT_U_IMM (n, s, l));
+ print (info->stream, dis_style_immediate, "%lu",
+ (unsigned long)EXTRACT_U_IMM (n, s, l));
else
- print (info->stream, dis_style_immediate, "%i",
- (unsigned)EXTRACT_S_IMM (n, s, l));
+ print (info->stream, dis_style_immediate, "%li",
+ (signed long)EXTRACT_S_IMM (n, s, l));
break;
default:
goto undefined_modifier;
--
2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH v3 6/6] RISC-V: Print XTheadMemPair literal as "immediate"
2022-10-03 10:43 ` [PATCH v3 " Tsukasa OI
` (4 preceding siblings ...)
2022-10-03 10:44 ` [PATCH v3 5/6] RISC-V: Fix T-Head immediate types on printing Tsukasa OI
@ 2022-10-03 10:44 ` Tsukasa OI
5 siblings, 0 replies; 34+ messages in thread
From: Tsukasa OI @ 2022-10-03 10:44 UTC (permalink / raw)
To: Tsukasa OI, Christoph Müllner, Nelson Chu, Kito Cheng,
Palmer Dabbelt, Andrew Burgess
Cc: binutils
The operand type "Xl(...)" denotes that (...) is a literal. Specifically,
they are intended to be a constant immediate value.
This commit prints "Xl(...)" operand with dis_style_immediate style,
not dis_style_text.
opcodes/ChangeLog:
* riscv-dis.c (print_insn_args): Use dis_style_immediate on
the constant literal of the "Xl..." operand.
---
opcodes/riscv-dis.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index c0edc4c7d6b..7da96e15b65 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -577,7 +577,7 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
oparg++;
while (*oparg && *oparg != ',')
{
- print (info->stream, dis_style_text, "%c", *oparg);
+ print (info->stream, dis_style_immediate, "%c", *oparg);
oparg++;
}
oparg--;
--
2.34.1
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 5/6] RISC-V: Fix T-Head immediate types on printing
2022-09-26 12:26 ` [PATCH v2 5/6] RISC-V: Fix T-Head immediate types on printing Tsukasa OI
2022-10-03 9:57 ` Andrew Burgess
@ 2022-10-03 11:06 ` Christoph Müllner
1 sibling, 0 replies; 34+ messages in thread
From: Christoph Müllner @ 2022-10-03 11:06 UTC (permalink / raw)
To: Tsukasa OI; +Cc: Nelson Chu, Kito Cheng, Palmer Dabbelt, binutils
[-- Attachment #1: Type: text/plain, Size: 1822 bytes --]
On Mon, Sep 26, 2022 at 2:30 PM Tsukasa OI via Binutils <
binutils@sourceware.org> wrote:
> This commit fixes three minor typing-related issues for
> T-Head immediate values.
>
> 1. The format string %i is renamed to %d (which is more common).
> 2. Signed type must be specified when printing with %d.
> 3. unsigned/signed int it not portable enough for max 32-bit immediates.
> Instead, we should use unsigned/signed long.
> The format string is changed accordingly.
>
Tested-by: Christoph Müllner <christoph.muellner@vrull.eu>
>
> opcodes/ChangeLog:
>
> * riscv-dis.c (print_insn_args): Fix T-Head immediate types on
> printing.
> ---
> opcodes/riscv-dis.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> index 1c75317fad1..df2a6f4e131 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -596,11 +596,11 @@ print_insn_args (const char *oparg, insn_t l,
> bfd_vma pc, disassemble_info *info
> oparg--;
>
> if (!sign)
> - print (info->stream, dis_style_immediate, "%u",
> - (unsigned)EXTRACT_U_IMM (n, s, l));
> + print (info->stream, dis_style_immediate, "%lu",
> + (unsigned long)EXTRACT_U_IMM (n, s, l));
> else
> - print (info->stream, dis_style_immediate, "%i",
> - (unsigned)EXTRACT_S_IMM (n, s, l));
> + print (info->stream, dis_style_immediate, "%ld",
> + (signed long)EXTRACT_S_IMM (n, s, l));
> break;
> default:
> goto undefined_modifier;
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 6/6] RISC-V: Print XTheadMemPair literal as "immediate"
2022-09-26 12:26 ` [PATCH v2 6/6] RISC-V: Print XTheadMemPair literal as "immediate" Tsukasa OI
@ 2022-10-03 11:06 ` Christoph Müllner
0 siblings, 0 replies; 34+ messages in thread
From: Christoph Müllner @ 2022-10-03 11:06 UTC (permalink / raw)
To: Tsukasa OI; +Cc: Nelson Chu, Kito Cheng, Palmer Dabbelt, binutils
[-- Attachment #1: Type: text/plain, Size: 1286 bytes --]
On Mon, Sep 26, 2022 at 2:28 PM Tsukasa OI via Binutils <
binutils@sourceware.org> wrote:
> The operand type "Xl(...)" denotes that (...) is a literal. Specifically,
> they are intended to be a constant immediate value.
>
> This commit prints "Xl(...)" operand with dis_style_immediate style,
> not dis_style_text.
>
Tested-by: Christoph Müllner <christoph.muellner@vrull.eu>
>
> opcodes/ChangeLog:
>
> * riscv-dis.c (print_insn_args): Use dis_style_immediate on
> the constant literal of the "Xl..." operand.
> ---
> opcodes/riscv-dis.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
> index df2a6f4e131..7aa45db75f2 100644
> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -577,7 +577,7 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma
> pc, disassemble_info *info
> oparg++;
> while (*oparg && *oparg != ',')
> {
> - print (info->stream, dis_style_text, "%c", *oparg);
> + print (info->stream, dis_style_immediate, "%c",
> *oparg);
> oparg++;
> }
> oparg--;
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 0/6] RISC-V: Fix disassembler types and styles
2022-10-03 9:59 ` [PATCH v2 0/6] RISC-V: Fix disassembler types and styles Andrew Burgess
@ 2022-10-03 17:40 ` Palmer Dabbelt
2022-10-04 1:34 ` Tsukasa OI
` (3 more replies)
0 siblings, 4 replies; 34+ messages in thread
From: Palmer Dabbelt @ 2022-10-03 17:40 UTC (permalink / raw)
To: aburgess; +Cc: binutils, research_trasio, nelson, kito.cheng, binutils
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
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 0/6] RISC-V: Fix disassembler types and styles
2022-10-03 17:40 ` Palmer Dabbelt
@ 2022-10-04 1:34 ` Tsukasa OI
2022-10-04 1:41 ` Nelson Chu
` (2 subsequent siblings)
3 siblings, 0 replies; 34+ messages in thread
From: Tsukasa OI @ 2022-10-04 1:34 UTC (permalink / raw)
To: Palmer Dabbelt, Binutils
On 2022/10/04 2:40, Palmer Dabbelt wrote:
> 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!
Understood. I hope you get better soon.
It seems it looks good to you but no approval yet (I think Nelson will
approve it because he approved related patch). For this particular
patchset, that's better than just "approval of v2" because "PATCH v3"
has minor changes (although no functional changes).
PATCH v3:
<https://sourceware.org/pipermail/binutils/2022-October/123274.html>
CSR style change by Andrew Burgess (I also spotted it but didn't
included in this patchset because it seemed non-obvious as the rest):
<https://sourceware.org/pipermail/binutils/2022-October/123272.html>
Nelson's Approval of the patch by Andrew:
<https://sourceware.org/pipermail/binutils/2022-October/123306.html>
Thanks,
Tsukasa
>
>>
>> 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
>
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 0/6] RISC-V: Fix disassembler types and styles
2022-10-03 17:40 ` Palmer Dabbelt
2022-10-04 1:34 ` Tsukasa OI
@ 2022-10-04 1:41 ` Nelson Chu
2022-10-04 8:46 ` Andrew Burgess
2022-10-05 21:52 ` Jeff Law
3 siblings, 0 replies; 34+ messages in thread
From: Nelson Chu @ 2022-10-04 1:41 UTC (permalink / raw)
To: Palmer Dabbelt; +Cc: aburgess, binutils, research_trasio, kito.cheng
On Tue, Oct 4, 2022 at 1:40 AM Palmer Dabbelt <palmer@dabbelt.com> wrote:
>
> 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.
I have the same worry, but we probably won't hear someone really yell
until the next release. However I am not the maintainer and user of
these analysis tools either, so I cannot say such dis-assembler
patches, including the patch to sort the opcode table, will definitely
break something. But being too conservative does make development
slower, even if stable things should still have a chance to be
improved, so I struggle every time when I see these kinds of patches.
What I can only say is that - if you think these kinds of things may
be helpful to someone, and have not only one maintainer or experts say
these are probably fine, then we can try to commit the patches after a
while, if there are not any objections.
Nelson
> 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 ;)
>
> 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.
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 0/6] RISC-V: Fix disassembler types and styles
2022-10-03 17:40 ` Palmer Dabbelt
2022-10-04 1:34 ` Tsukasa OI
2022-10-04 1:41 ` Nelson Chu
@ 2022-10-04 8:46 ` Andrew Burgess
2022-10-05 22:37 ` Palmer Dabbelt
2022-10-05 21:52 ` Jeff Law
3 siblings, 1 reply; 34+ messages in thread
From: Andrew Burgess @ 2022-10-04 8:46 UTC (permalink / raw)
To: Palmer Dabbelt; +Cc: binutils, research_trasio, nelson, kito.cheng, binutils
Palmer Dabbelt <palmer@dabbelt.com> writes:
> 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.
I guess we'd want to consider these things on a case-by-case basis. I'm
struggling to think of a situation where we wouldn't want to fix
something in the disassembler output if it was wrong over leaving it
incorrect because someone, somewhere, might have a script that depends
on the broken output.
But unless I've missed something, I don't think this series makes any
changes to the output other than styling, and I don't think a script
that's trying to parse the disassembler output should be running with
styling switched on....
...or if they are, maybe because they want to figure out which bits of
the text are which, then surely they'd welcome these fixes?
> 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.
And in objdump, which also styles its output by default now when the
output is going to a tty. Any scripts that capture objdump output
should see no changes with this series.
The styling is pretty new, and I knew when I added it there were going
to be mistakes with things styled incorrectly. I was basically relying
on getting something in that was not terrible, and then relying on folk
like Tsukasa to come along and fix my mistakes.
IMHO, any script that relies on a fixed styling is a bad script.
I'd probably go further, and say that any script that can't handle minor
changes in the disassembly output at all is a bad script, but without
seeing every case that might be a little harsh.
Anyway, FWIW, I think we should for sure approve this series.
Thanks,
Andrew
> 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
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 0/6] RISC-V: Fix disassembler types and styles
2022-10-03 17:40 ` Palmer Dabbelt
` (2 preceding siblings ...)
2022-10-04 8:46 ` Andrew Burgess
@ 2022-10-05 21:52 ` Jeff Law
3 siblings, 0 replies; 34+ messages in thread
From: Jeff Law @ 2022-10-05 21:52 UTC (permalink / raw)
To: binutils
On 10/3/22 11:40, Palmer Dabbelt wrote:
> 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 ;)
I don't think the precise for of disassembly output needs to stay
stable; it's certainly not something we've ever promised or encouraged
folks to rely on. Obviously we shouldn't change it just for the sake of
changing it, but if we need to make a fix, we should just make it.
jeff
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH v2 0/6] RISC-V: Fix disassembler types and styles
2022-10-04 8:46 ` Andrew Burgess
@ 2022-10-05 22:37 ` Palmer Dabbelt
0 siblings, 0 replies; 34+ messages in thread
From: Palmer Dabbelt @ 2022-10-05 22:37 UTC (permalink / raw)
To: aburgess; +Cc: binutils, research_trasio, nelson, kito.cheng, binutils
On Tue, 04 Oct 2022 01:46:01 PDT (-0700), aburgess@redhat.com wrote:
> Palmer Dabbelt <palmer@dabbelt.com> writes:
>> 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.
>
> I guess we'd want to consider these things on a case-by-case basis. I'm
> struggling to think of a situation where we wouldn't want to fix
> something in the disassembler output if it was wrong over leaving it
> incorrect because someone, somewhere, might have a script that depends
> on the broken output.
>
> But unless I've missed something, I don't think this series makes any
> changes to the output other than styling, and I don't think a script
> that's trying to parse the disassembler output should be running with
> styling switched on....
>
> ...or if they are, maybe because they want to figure out which bits of
> the text are which, then surely they'd welcome these fixes?
>
>> 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.
>
> And in objdump, which also styles its output by default now when the
> output is going to a tty. Any scripts that capture objdump output
> should see no changes with this series.
Ah, cool, I hadn't seen that when poking around. I don't think it
changes the reasoning, though.
> The styling is pretty new, and I knew when I added it there were going
> to be mistakes with things styled incorrectly. I was basically relying
> on getting something in that was not terrible, and then relying on folk
> like Tsukasa to come along and fix my mistakes.
>
> IMHO, any script that relies on a fixed styling is a bad script.
Ya, that seems like an easy one -- doubly so if it's relying on styling
that's wrong it a handful of cases. Like you said, if anything this
would fix scripts rather than break them.
> I'd probably go further, and say that any script that can't handle minor
> changes in the disassembly output at all is a bad script, but without
> seeing every case that might be a little harsh.
I've definitely written scripts like that, and it felt pretty bad at the
time. It'd still be a small headache if they broke: less actually
fixing the script, but more that some test would break on a binutils
upgrade and that tends to spook the HW folks into just never upgrading
the toolchain again (even if it's just a test checking script that broken).
That kind of stuff is someone else's problem these days, though, so I'm
fine just pretending none of it ever happened ;)
> Anyway, FWIW, I think we should for sure approve this series.
Works for me. Sounds like Jeff was saying about the same thing in his
reply, so approved from me.
Tsukasa: I haven't built/tested these and my box is kind of tied up with
Linux right now, but feel free to commit them assuming someone's tested
them.
Thanks!
>
> Thanks,
> Andrew
>
>
>> 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
^ permalink raw reply [flat|nested] 34+ messages in thread
end of thread, other threads:[~2022-10-05 22:37 UTC | newest]
Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-13 13:40 [PATCH] RISC-V: fix printf types on riscv-dis.c Tsukasa OI
2022-08-03 4:27 ` [PATCH 0/4] RISC-V: Fix disassembler types and styles Tsukasa OI
2022-08-03 4:27 ` [PATCH 1/4] RISC-V: Fix immediates to have `immediate' style Tsukasa OI
2022-08-10 11:16 ` Andrew Burgess
2022-08-10 13:48 ` Tsukasa OI
2022-08-03 4:27 ` [PATCH 2/4] RISC-V: Fix printf argument types corresponding %x Tsukasa OI
2022-08-03 4:27 ` [PATCH 3/4] RISC-V: Optimize riscv_disassemble_data printf Tsukasa OI
2022-08-03 4:27 ` [PATCH 4/4] RISC-V: Print comma and tabs as the `text' style Tsukasa OI
2022-08-10 11:20 ` Andrew Burgess
2022-08-10 13:54 ` Tsukasa OI
2022-09-26 12:26 ` [PATCH v2 0/6] RISC-V: Fix disassembler types and styles Tsukasa OI
2022-09-26 12:26 ` [PATCH v2 1/6] RISC-V: Fix immediates to have "immediate" style Tsukasa OI
2022-09-26 12:26 ` [PATCH v2 2/6] RISC-V: Fix printf argument types corresponding %x Tsukasa OI
2022-09-26 12:26 ` [PATCH v2 3/6] RISC-V: Optimize riscv_disassemble_data printf Tsukasa OI
2022-09-26 12:26 ` [PATCH v2 4/6] RISC-V: Print comma and tabs as the "text" style Tsukasa OI
2022-09-26 12:26 ` [PATCH v2 5/6] RISC-V: Fix T-Head immediate types on printing Tsukasa OI
2022-10-03 9:57 ` Andrew Burgess
2022-10-03 11:06 ` Christoph Müllner
2022-09-26 12:26 ` [PATCH v2 6/6] RISC-V: Print XTheadMemPair literal as "immediate" Tsukasa OI
2022-10-03 11:06 ` Christoph Müllner
2022-10-03 9:59 ` [PATCH v2 0/6] RISC-V: Fix disassembler types and styles Andrew Burgess
2022-10-03 17:40 ` Palmer Dabbelt
2022-10-04 1:34 ` Tsukasa OI
2022-10-04 1:41 ` Nelson Chu
2022-10-04 8:46 ` Andrew Burgess
2022-10-05 22:37 ` Palmer Dabbelt
2022-10-05 21:52 ` Jeff Law
2022-10-03 10:43 ` [PATCH v3 " Tsukasa OI
2022-10-03 10:43 ` [PATCH v3 1/6] RISC-V: Fix immediates to have "immediate" style Tsukasa OI
2022-10-03 10:44 ` [PATCH v3 2/6] RISC-V: Fix printf argument types corresponding %x Tsukasa OI
2022-10-03 10:44 ` [PATCH v3 3/6] RISC-V: Optimize riscv_disassemble_data printf Tsukasa OI
2022-10-03 10:44 ` [PATCH v3 4/6] RISC-V: Print comma and tabs as the "text" style Tsukasa OI
2022-10-03 10:44 ` [PATCH v3 5/6] RISC-V: Fix T-Head immediate types on printing Tsukasa OI
2022-10-03 10:44 ` [PATCH v3 6/6] RISC-V: Print XTheadMemPair literal as "immediate" Tsukasa OI
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).