diff mbox series

[v3,1/2] RISC-V: Better support for long instructions (disassembler)

Message ID 66ca80358f78d66d66bbf390fc0be8bec8183e93.1669342633.git.research_trasio@irq.a4lg.com
State Accepted
Headers
Series RISC-V: Better support for long instructions (64 < x <= 176 [bits]) |

Checks

Context Check Description
snail/binutils-gdb-check success Github commit url

Commit Message

Tsukasa OI Nov. 25, 2022, 2:17 a.m. UTC 
  From: Tsukasa OI <research_trasio@irq.a4lg.com>

Commit bb996692bd96 ("RISC-V/gas: allow generating up to 176-bit
instructions with .insn") tried to start supporting long instructions but
it was insufficient.

1.  On the disassembler, disassembler dump was limited up to 64-bit.
    For long (unknown) instructions, instruction bits are incorrectly
    zeroed out.

To solve these problems, this commit adds packet argument to support dumping
instructions longer than 64-bits.  This commit will be tested on the next
commit "RISC-V: Better support for long instructions (assembler)".

opcodes/ChangeLog:

	* riscv-dis.c (riscv_disassemble_insn): Print unknown instruction
	using the new argument packet.
	(riscv_disassemble_data): Add unused argument packet.
	(print_insn_riscv): Pass packet to the disassemble function.
---
 opcodes/riscv-dis.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

Jan Beulich Nov. 25, 2022, 8:03 a.m. UTC | #1 
On 25.11.2022 03:17, Tsukasa OI wrote:
> From: Tsukasa OI <research_trasio@irq.a4lg.com>
> 
> Commit bb996692bd96 ("RISC-V/gas: allow generating up to 176-bit
> instructions with .insn") tried to start supporting long instructions but
> it was insufficient.
> 
> 1.  On the disassembler, disassembler dump was limited up to 64-bit.
>     For long (unknown) instructions, instruction bits are incorrectly
>     zeroed out.
> 
> To solve these problems, this commit adds packet argument to support dumping
> instructions longer than 64-bits.  This commit will be tested on the next
> commit "RISC-V: Better support for long instructions (assembler)".
> 
> opcodes/ChangeLog:
> 
> 	* riscv-dis.c (riscv_disassemble_insn): Print unknown instruction
> 	using the new argument packet.
> 	(riscv_disassemble_data): Add unused argument packet.
> 	(print_insn_riscv): Pass packet to the disassemble function.

Looks okay to me; just one style nit:

> --- a/opcodes/riscv-dis.c
> +++ b/opcodes/riscv-dis.c
> @@ -641,7 +641,10 @@ print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
>     this is little-endian code.  */
>  
>  static int
> -riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
> +riscv_disassemble_insn (bfd_vma memaddr,
> +			insn_t word,
> +			const bfd_byte *packet,

Just like you have it here, please add ...

> @@ -1037,7 +1040,7 @@ print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
>    bfd_vma dump_size;
>    int status;
>    enum riscv_seg_mstate mstate;
> -  int (*riscv_disassembler) (bfd_vma, insn_t, struct disassemble_info *);
> +  int (*riscv_disassembler) (bfd_vma, insn_t, const bfd_byte*, struct disassemble_info *);

... a blank before * here. I guess you will also want to wrap this
(now long) line.

No need to submit a v4 just for that, i.e. feel free to simply commit
with the adjustment (after giving Nelson a little bit of time, just
in case).

Jan
diff mbox series

Patch

diff --git a/opcodes/riscv-dis.c b/opcodes/riscv-dis.c
index 3a31647a2f80..59ebbaf13417 100644
--- a/opcodes/riscv-dis.c
+++ b/opcodes/riscv-dis.c
@@ -641,7 +641,10 @@  print_insn_args (const char *oparg, insn_t l, bfd_vma pc, disassemble_info *info
    this is little-endian code.  */
 
 static int
-riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
+riscv_disassemble_insn (bfd_vma memaddr,
+			insn_t word,
+			const bfd_byte *packet,
+			disassemble_info *info)
 {
   const struct riscv_opcode *op;
   static bool init = false;
@@ -806,8 +809,7 @@  riscv_disassemble_insn (bfd_vma memaddr, insn_t word, disassemble_info *info)
 					    ", ");
 	    (*info->fprintf_styled_func) (info->stream, dis_style_immediate,
 					  "0x%02x",
-					  (unsigned int) (word & 0xff));
-            word >>= 8;
+					  (unsigned int) (*packet++));
           }
       }
       break;
@@ -983,6 +985,7 @@  riscv_data_length (bfd_vma memaddr,
 static int
 riscv_disassemble_data (bfd_vma memaddr ATTRIBUTE_UNUSED,
 			insn_t data,
+			const bfd_byte *packet ATTRIBUTE_UNUSED,
 			disassemble_info *info)
 {
   info->display_endian = info->endian;
@@ -1037,7 +1040,7 @@  print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
   bfd_vma dump_size;
   int status;
   enum riscv_seg_mstate mstate;
-  int (*riscv_disassembler) (bfd_vma, insn_t, struct disassemble_info *);
+  int (*riscv_disassembler) (bfd_vma, insn_t, const bfd_byte*, struct disassemble_info *);
 
   if (info->disassembler_options != NULL)
     {
@@ -1081,7 +1084,7 @@  print_insn_riscv (bfd_vma memaddr, struct disassemble_info *info)
     }
   insn = (insn_t) bfd_get_bits (packet, dump_size * 8, false);
 
-  return (*riscv_disassembler) (memaddr, insn, info);
+  return (*riscv_disassembler) (memaddr, insn, packet, info);
 }
 
 disassembler_ftype