diff mbox series

[v2,2/2] RISC-V: Better support for long instructions

Message ID cdd1f831660ce24353b47dc4098992f136e45bcc.1669192210.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. 23, 2022, 8:30 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.  It heavily depended on the bignum internals (radix of 2^16),
2.  It generates "value conflicts with instruction length" even if a big
    number instruction encoding does not exceed its expected length,
3.  Because long opcode was handled separately (from struct riscv_cl_insn),
    some information like DWARF line number correspondence was missing and
4.  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:

1.  Handles bignum (and its encodings) precisely,
2.  Incorporates long opcode handling into regular
    struct riscv_cl_insn-handling functions and
3.  Adds packet argument to support dumping instructions
    longer than 64-bits.

gas/ChangeLog:

	* config/tc-riscv.c (struct riscv_cl_insn): Add long opcode field.
	(create_insn) Clear long opcode marker.
	(install_insn) Install longer opcode as well.
	(s_riscv_insn) Likewise.
	(riscv_ip_hardcode): Make big number handling stricter. Length and
	the value conflicts only if the bignum size exceeds the expected
	maximum length.
	* testsuite/gas/riscv/insn.s: Add testcases such that big number
	handling is required.
	* testsuite/gas/riscv/insn.d: Likewise.
	* testsuite/gas/riscv/insn-na.d: Likewise.
	* testsuite/gas/riscv/insn-dwarf.d: Likewise.

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.
---
 gas/config/tc-riscv.c                | 38 ++++++++++++++++++++++------
 gas/testsuite/gas/riscv/insn-dwarf.d | 10 +++++++-
 gas/testsuite/gas/riscv/insn-na.d    |  8 ++++++
 gas/testsuite/gas/riscv/insn.d       | 22 ++++++++++++++++
 gas/testsuite/gas/riscv/insn.s       |  9 +++++++
 opcodes/riscv-dis.c                  | 13 ++++++----
 6 files changed, 86 insertions(+), 14 deletions(-)

Comments

Jan Beulich Nov. 23, 2022, 9:04 a.m. UTC | #1 
On 23.11.2022 09:30, 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.  It heavily depended on the bignum internals (radix of 2^16),
> 2.  It generates "value conflicts with instruction length" even if a big
>     number instruction encoding does not exceed its expected length,
> 3.  Because long opcode was handled separately (from struct riscv_cl_insn),
>     some information like DWARF line number correspondence was missing and
> 4.  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:
> 
> 1.  Handles bignum (and its encodings) precisely,
> 2.  Incorporates long opcode handling into regular
>     struct riscv_cl_insn-handling functions and
> 3.  Adds packet argument to support dumping instructions
>     longer than 64-bits.
> 
> gas/ChangeLog:
> 
> 	* config/tc-riscv.c (struct riscv_cl_insn): Add long opcode field.
> 	(create_insn) Clear long opcode marker.
> 	(install_insn) Install longer opcode as well.
> 	(s_riscv_insn) Likewise.
> 	(riscv_ip_hardcode): Make big number handling stricter. Length and
> 	the value conflicts only if the bignum size exceeds the expected
> 	maximum length.
> 	* testsuite/gas/riscv/insn.s: Add testcases such that big number
> 	handling is required.
> 	* testsuite/gas/riscv/insn.d: Likewise.
> 	* testsuite/gas/riscv/insn-na.d: Likewise.
> 	* testsuite/gas/riscv/insn-dwarf.d: Likewise.
> 
> 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.

The code changes look okay to me. For the testsuite additions I have
voiced my reservations, and I've given further background in an earlier
reply still on the v1 sub-thread. Whatever the resolution there would
imo want to be applied here as well.

As to mixing assembler and disassembler changes in the same patch: Is
this strictly necessary here for some reason? Generally I would suggest
to split such, but once again I wouldn't insist on you doing so ...

Jan
Tsukasa OI Nov. 24, 2022, 2:34 a.m. UTC | #2 
On 2022/11/23 18:04, Jan Beulich wrote:
> On 23.11.2022 09:30, 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.  It heavily depended on the bignum internals (radix of 2^16),
>> 2.  It generates "value conflicts with instruction length" even if a big
>>     number instruction encoding does not exceed its expected length,
>> 3.  Because long opcode was handled separately (from struct riscv_cl_insn),
>>     some information like DWARF line number correspondence was missing and
>> 4.  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:
>>
>> 1.  Handles bignum (and its encodings) precisely,
>> 2.  Incorporates long opcode handling into regular
>>     struct riscv_cl_insn-handling functions and
>> 3.  Adds packet argument to support dumping instructions
>>     longer than 64-bits.
>>
>> gas/ChangeLog:
>>
>> 	* config/tc-riscv.c (struct riscv_cl_insn): Add long opcode field.
>> 	(create_insn) Clear long opcode marker.
>> 	(install_insn) Install longer opcode as well.
>> 	(s_riscv_insn) Likewise.
>> 	(riscv_ip_hardcode): Make big number handling stricter. Length and
>> 	the value conflicts only if the bignum size exceeds the expected
>> 	maximum length.
>> 	* testsuite/gas/riscv/insn.s: Add testcases such that big number
>> 	handling is required.
>> 	* testsuite/gas/riscv/insn.d: Likewise.
>> 	* testsuite/gas/riscv/insn-na.d: Likewise.
>> 	* testsuite/gas/riscv/insn-dwarf.d: Likewise.
>>
>> 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.
> 
> The code changes look okay to me. For the testsuite additions I have
> voiced my reservations, and I've given further background in an earlier
> reply still on the v1 sub-thread. Whatever the resolution there would
> imo want to be applied here as well.

Understood.  My (minimum) opinion is, I want to keep 22-bytes patterns
corresponding PATCH v2 2/2 because that's exactly changed by the
assembler / disassembler fixes.

> 
> As to mixing assembler and disassembler changes in the same patch: Is
> this strictly necessary here for some reason? Generally I would suggest
> to split such, but once again I wouldn't insist on you doing so ...
> 
> Jan
> 
I'm okay to split:
-   Assembler fix + Disassembler fix + Test
to:
1.  Assembler fix
2.  Disassembler fix + Test
but there are a good reason I did like this in this patch.

To test fixed assembler, we need to fix disassembler as well.  Although
they are not exactly the same issue, they are corresponding enough so
that merging changes might be justified.

But since they are not the same issue (as you pointed out), I'm okay to
split to two (three might be too much) separate patches.

Thanks,
Tsukasa
Jan Beulich Nov. 24, 2022, 7:31 a.m. UTC | #3 
On 24.11.2022 03:34, Tsukasa OI wrote:
> On 2022/11/23 18:04, Jan Beulich wrote:
>> On 23.11.2022 09:30, 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.  It heavily depended on the bignum internals (radix of 2^16),
>>> 2.  It generates "value conflicts with instruction length" even if a big
>>>     number instruction encoding does not exceed its expected length,
>>> 3.  Because long opcode was handled separately (from struct riscv_cl_insn),
>>>     some information like DWARF line number correspondence was missing and
>>> 4.  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:
>>>
>>> 1.  Handles bignum (and its encodings) precisely,
>>> 2.  Incorporates long opcode handling into regular
>>>     struct riscv_cl_insn-handling functions and
>>> 3.  Adds packet argument to support dumping instructions
>>>     longer than 64-bits.
>>>
>>> gas/ChangeLog:
>>>
>>> 	* config/tc-riscv.c (struct riscv_cl_insn): Add long opcode field.
>>> 	(create_insn) Clear long opcode marker.
>>> 	(install_insn) Install longer opcode as well.
>>> 	(s_riscv_insn) Likewise.
>>> 	(riscv_ip_hardcode): Make big number handling stricter. Length and
>>> 	the value conflicts only if the bignum size exceeds the expected
>>> 	maximum length.
>>> 	* testsuite/gas/riscv/insn.s: Add testcases such that big number
>>> 	handling is required.
>>> 	* testsuite/gas/riscv/insn.d: Likewise.
>>> 	* testsuite/gas/riscv/insn-na.d: Likewise.
>>> 	* testsuite/gas/riscv/insn-dwarf.d: Likewise.
>>>
>>> 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.
>>
>> The code changes look okay to me. For the testsuite additions I have
>> voiced my reservations, and I've given further background in an earlier
>> reply still on the v1 sub-thread. Whatever the resolution there would
>> imo want to be applied here as well.
> 
> Understood.  My (minimum) opinion is, I want to keep 22-bytes patterns
> corresponding PATCH v2 2/2 because that's exactly changed by the
> assembler / disassembler fixes.

But the assembler was rejecting the input there originally, wasn't it?
At which point _extending_ the testcase is certainly wanted, but do you
really need to check the ".byte ..." output to achieve the goal of the
test?

>> As to mixing assembler and disassembler changes in the same patch: Is
>> this strictly necessary here for some reason? Generally I would suggest
>> to split such, but once again I wouldn't insist on you doing so ...
>>
>> Jan
>>
> I'm okay to split:
> -   Assembler fix + Disassembler fix + Test
> to:
> 1.  Assembler fix
> 2.  Disassembler fix + Test
> but there are a good reason I did like this in this patch.
> 
> To test fixed assembler, we need to fix disassembler as well.  Although
> they are not exactly the same issue, they are corresponding enough so
> that merging changes might be justified.
> 
> But since they are not the same issue (as you pointed out), I'm okay to
> split to two (three might be too much) separate patches.

I agree three would be too much; I wonder whether

1.  Disassembler fix
2.  Assembler fix + Test

wouldn't be the better way to split, if you are going to in the first
place. Aiui the disassembler fix doesn't need any adjustments to
testcases, whereas my view is that the testcase addition is primarily
about the previously wrongly rejected assembler input, and only
secondarily about disassembler output. Hence keeping the testcase
adjustments with the assembler fix would, to me, seem more "natural".

Jan
Tsukasa OI Nov. 24, 2022, 7:35 a.m. UTC | #4 
On 2022/11/24 16:31, Jan Beulich wrote:
> On 24.11.2022 03:34, Tsukasa OI wrote:
>> On 2022/11/23 18:04, Jan Beulich wrote:
>>> On 23.11.2022 09:30, 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.  It heavily depended on the bignum internals (radix of 2^16),
>>>> 2.  It generates "value conflicts with instruction length" even if a big
>>>>     number instruction encoding does not exceed its expected length,
>>>> 3.  Because long opcode was handled separately (from struct riscv_cl_insn),
>>>>     some information like DWARF line number correspondence was missing and
>>>> 4.  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:
>>>>
>>>> 1.  Handles bignum (and its encodings) precisely,
>>>> 2.  Incorporates long opcode handling into regular
>>>>     struct riscv_cl_insn-handling functions and
>>>> 3.  Adds packet argument to support dumping instructions
>>>>     longer than 64-bits.
>>>>
>>>> gas/ChangeLog:
>>>>
>>>> 	* config/tc-riscv.c (struct riscv_cl_insn): Add long opcode field.
>>>> 	(create_insn) Clear long opcode marker.
>>>> 	(install_insn) Install longer opcode as well.
>>>> 	(s_riscv_insn) Likewise.
>>>> 	(riscv_ip_hardcode): Make big number handling stricter. Length and
>>>> 	the value conflicts only if the bignum size exceeds the expected
>>>> 	maximum length.
>>>> 	* testsuite/gas/riscv/insn.s: Add testcases such that big number
>>>> 	handling is required.
>>>> 	* testsuite/gas/riscv/insn.d: Likewise.
>>>> 	* testsuite/gas/riscv/insn-na.d: Likewise.
>>>> 	* testsuite/gas/riscv/insn-dwarf.d: Likewise.
>>>>
>>>> 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.
>>>
>>> The code changes look okay to me. For the testsuite additions I have
>>> voiced my reservations, and I've given further background in an earlier
>>> reply still on the v1 sub-thread. Whatever the resolution there would
>>> imo want to be applied here as well.
>>
>> Understood.  My (minimum) opinion is, I want to keep 22-bytes patterns
>> corresponding PATCH v2 2/2 because that's exactly changed by the
>> assembler / disassembler fixes.
> 
> But the assembler was rejecting the input there originally, wasn't it?
> At which point _extending_ the testcase is certainly wanted, but do you
> really need to check the ".byte ..." output to achieve the goal of the
> test?
> 
>>> As to mixing assembler and disassembler changes in the same patch: Is
>>> this strictly necessary here for some reason? Generally I would suggest
>>> to split such, but once again I wouldn't insist on you doing so ...
>>>
>>> Jan
>>>
>> I'm okay to split:
>> -   Assembler fix + Disassembler fix + Test
>> to:
>> 1.  Assembler fix
>> 2.  Disassembler fix + Test
>> but there are a good reason I did like this in this patch.
>>
>> To test fixed assembler, we need to fix disassembler as well.  Although
>> they are not exactly the same issue, they are corresponding enough so
>> that merging changes might be justified.
>>
>> But since they are not the same issue (as you pointed out), I'm okay to
>> split to two (three might be too much) separate patches.
> 
> I agree three would be too much; I wonder whether
> 
> 1.  Disassembler fix
> 2.  Assembler fix + Test
> 
> wouldn't be the better way to split, if you are going to in the first
> place. Aiui the disassembler fix doesn't need any adjustments to
> testcases, whereas my view is that the testcase addition is primarily
> about the previously wrongly rejected assembler input, and only
> secondarily about disassembler output. Hence keeping the testcase
> adjustments with the assembler fix would, to me, seem more "natural".
> 
> Jan
> 

Ah, I can agree that as well and I feel your option more natural.
I somehow missed that (probably because of my health issues for a weeks).

Anyway, thanks for pointing this out!

Tsukasa
diff mbox series

Patch

diff --git a/gas/config/tc-riscv.c b/gas/config/tc-riscv.c
index 019545171f5e..2237062d8b45 100644
--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -45,6 +45,9 @@  struct riscv_cl_insn
   /* The encoded instruction bits.  */
   insn_t insn_opcode;
 
+  /* The long encoded instruction bits ([0] is non-zero if used).  */
+  char insn_long_opcode[RISCV_MAX_INSN_LEN];
+
   /* The frag that contains the instruction.  */
   struct frag *frag;
 
@@ -714,6 +717,7 @@  create_insn (struct riscv_cl_insn *insn, const struct riscv_opcode *mo)
 {
   insn->insn_mo = mo;
   insn->insn_opcode = mo->match;
+  insn->insn_long_opcode[0] = 0;  /* Long insn has non-zero value.  */
   insn->frag = NULL;
   insn->where = 0;
   insn->fixp = NULL;
@@ -725,7 +729,10 @@  static void
 install_insn (const struct riscv_cl_insn *insn)
 {
   char *f = insn->frag->fr_literal + insn->where;
-  number_to_chars_littleendian (f, insn->insn_opcode, insn_length (insn));
+  if (insn->insn_long_opcode[0] != 0)
+    memcpy (f, insn->insn_long_opcode, insn_length (insn));
+  else
+    number_to_chars_littleendian (f, insn->insn_opcode, insn_length (insn));
 }
 
 /* Move INSN to offset WHERE in FRAG.  Adjust the fixups accordingly
@@ -3481,7 +3488,9 @@  riscv_ip_hardcode (char *str,
 	  values[num++] = (insn_t) imm_expr->X_add_number;
 	  break;
 	case O_big:
-	  values[num++] = generic_bignum[0];
+	  /* Extract lower 32-bits of a big number.
+	     Assume that generic_bignum_to_int32 work on such number.  */
+	  values[num++] = (insn_t) generic_bignum_to_int32 ();
 	  break;
 	default:
 	  /* The first value isn't constant, so it should be
@@ -3508,12 +3517,25 @@  riscv_ip_hardcode (char *str,
 
   if (imm_expr->X_op == O_big)
     {
-      if (bytes != imm_expr->X_add_number * CHARS_PER_LITTLENUM)
+      unsigned int llen = 0;
+      for (LITTLENUM_TYPE lval = generic_bignum[imm_expr->X_add_number - 1];
+	   lval != 0; llen++)
+	lval >>= BITS_PER_CHAR;
+      unsigned int repr_bytes
+	  = (imm_expr->X_add_number - 1) * CHARS_PER_LITTLENUM + llen;
+      if (bytes < repr_bytes)
 	return _("value conflicts with instruction length");
-      char *f = frag_more (bytes);
-      for (num = 0; num < imm_expr->X_add_number; ++num)
-	number_to_chars_littleendian (f + num * CHARS_PER_LITTLENUM,
-	                              generic_bignum[num], CHARS_PER_LITTLENUM);
+      for (num = 0; num < imm_expr->X_add_number - 1; ++num)
+	number_to_chars_littleendian (
+	    ip->insn_long_opcode + num * CHARS_PER_LITTLENUM,
+	    generic_bignum[num],
+	    CHARS_PER_LITTLENUM);
+      if (llen != 0)
+	number_to_chars_littleendian (
+	    ip->insn_long_opcode + num * CHARS_PER_LITTLENUM,
+	    generic_bignum[num],
+	    llen);
+      memset(ip->insn_long_opcode + repr_bytes, 0, bytes - repr_bytes);
       return NULL;
     }
 
@@ -4590,7 +4612,7 @@  s_riscv_insn (int x ATTRIBUTE_UNUSED)
       else
 	as_bad ("%s `%s'", error.msg, error.statement);
     }
-  else if (imm_expr.X_op != O_big)
+  else
     {
       gas_assert (insn.insn_mo->pinfo != INSN_MACRO);
       append_insn (&insn, &imm_expr, imm_reloc);
diff --git a/gas/testsuite/gas/riscv/insn-dwarf.d b/gas/testsuite/gas/riscv/insn-dwarf.d
index 89dc8d58ff09..b8bd42dff18c 100644
--- a/gas/testsuite/gas/riscv/insn-dwarf.d
+++ b/gas/testsuite/gas/riscv/insn-dwarf.d
@@ -74,5 +74,13 @@  insn.s +69 +0xf6.*
 insn.s +70 +0xfe.*
 insn.s +71 +0x108.*
 insn.s +72 +0x114.*
-insn.s +- +0x12a
+insn.s +74 +0x12a.*
+insn.s +75 +0x134.*
+insn.s +76 +0x13e.*
+insn.s +77 +0x154.*
+insn.s +78 +0x16a.*
+insn.s +79 +0x180.*
+insn.s +80 +0x196.*
+insn.s +81 +0x1ac.*
+insn.s +- +0x1c2
 #pass
diff --git a/gas/testsuite/gas/riscv/insn-na.d b/gas/testsuite/gas/riscv/insn-na.d
index be6c9f9dd66a..60024555c71f 100644
--- a/gas/testsuite/gas/riscv/insn-na.d
+++ b/gas/testsuite/gas/riscv/insn-na.d
@@ -73,3 +73,11 @@  Disassembly of section .text:
 [^:]+:[ 	]+007f 0000 0000 0000 0000[ 	]+\.byte[ 	]+0x7f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
 [^:]+:[ 	]+0000107f 00000000 00000000[ 	]+\.byte[ 	]+0x7f, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
 [^:]+:[ 	]+607f 0000 0000 0000 0000 0000 0000 0000 0000 0000 0000[ 	]+\.byte[ 	]+0x7f, 0x60, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+[^:]+:[ 	]+007f 0000 0000 0000 8000[ 	]+\.byte[ 	]+0x7f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x80
+[^:]+:[ 	]+007f 0000 0000 0000 8000[ 	]+\.byte[ 	]+0x7f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x80
+[^:]+:[ 	]+607f 89ab 4567 0123 3210 7654 ba98 fedc 0000 0000 0000[ 	]+\.byte[ 	]+0x7f, 0x60, 0xab, 0x89, 0x67, 0x45, 0x23, 0x01, 0x10, 0x32, 0x54, 0x76, 0x98, 0xba, 0xdc, 0xfe, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+[^:]+:[ 	]+607f 89ab 4567 0123 3210 7654 ba98 fedc 0000 0000 0000[ 	]+\.byte[ 	]+0x7f, 0x60, 0xab, 0x89, 0x67, 0x45, 0x23, 0x01, 0x10, 0x32, 0x54, 0x76, 0x98, 0xba, 0xdc, 0xfe, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+[^:]+:[ 	]+607f 33cc 55aa cdef 89ab 4567 0123 3210 7654 ba98 00dc[ 	]+\.byte[ 	]+0x7f, 0x60, 0xcc, 0x33, 0xaa, 0x55, 0xef, 0xcd, 0xab, 0x89, 0x67, 0x45, 0x23, 0x01, 0x10, 0x32, 0x54, 0x76, 0x98, 0xba, 0xdc, 0x00
+[^:]+:[ 	]+607f 33cc 55aa cdef 89ab 4567 0123 3210 7654 ba98 00dc[ 	]+\.byte[ 	]+0x7f, 0x60, 0xcc, 0x33, 0xaa, 0x55, 0xef, 0xcd, 0xab, 0x89, 0x67, 0x45, 0x23, 0x01, 0x10, 0x32, 0x54, 0x76, 0x98, 0xba, 0xdc, 0x00
+[^:]+:[ 	]+607f 33cc 55aa cdef 89ab 4567 0123 3210 7654 ba98 fedc[ 	]+\.byte[ 	]+0x7f, 0x60, 0xcc, 0x33, 0xaa, 0x55, 0xef, 0xcd, 0xab, 0x89, 0x67, 0x45, 0x23, 0x01, 0x10, 0x32, 0x54, 0x76, 0x98, 0xba, 0xdc, 0xfe
+[^:]+:[ 	]+607f 33cc 55aa cdef 89ab 4567 0123 3210 7654 ba98 fedc[ 	]+\.byte[ 	]+0x7f, 0x60, 0xcc, 0x33, 0xaa, 0x55, 0xef, 0xcd, 0xab, 0x89, 0x67, 0x45, 0x23, 0x01, 0x10, 0x32, 0x54, 0x76, 0x98, 0xba, 0xdc, 0xfe
diff --git a/gas/testsuite/gas/riscv/insn.d b/gas/testsuite/gas/riscv/insn.d
index cf84f177af39..b02d67c5f120 100644
--- a/gas/testsuite/gas/riscv/insn.d
+++ b/gas/testsuite/gas/riscv/insn.d
@@ -92,3 +92,25 @@  Disassembly of section .text:
 [^:]+:[ 	]+607f 0000 0000 0000[ 	]+\.byte[ 	]+0x7f, 0x60, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
 [^:]+:[ 	]+0000 0000 0000 0000 ?
 [^:]+:[ 	]+0000 0000 0000 ?
+[^:]+:[ 	]+007f 0000 0000 0000[ 	]+\.byte[ 	]+0x7f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x80
+[^:]+:[ 	]+8000 ?
+[^:]+:[ 	]+007f 0000 0000 0000[ 	]+\.byte[ 	]+0x7f, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x80
+[^:]+:[ 	]+8000 ?
+[^:]+:[ 	]+607f 89ab 4567 0123[ 	]+\.byte[ 	]+0x7f, 0x60, 0xab, 0x89, 0x67, 0x45, 0x23, 0x01, 0x10, 0x32, 0x54, 0x76, 0x98, 0xba, 0xdc, 0xfe, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+[^:]+:[ 	]+3210 7654 ba98 fedc ?
+[^:]+:[ 	]+0000 0000 0000 ?
+[^:]+:[ 	]+607f 89ab 4567 0123[ 	]+\.byte[ 	]+0x7f, 0x60, 0xab, 0x89, 0x67, 0x45, 0x23, 0x01, 0x10, 0x32, 0x54, 0x76, 0x98, 0xba, 0xdc, 0xfe, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
+[^:]+:[ 	]+3210 7654 ba98 fedc ?
+[^:]+:[ 	]+0000 0000 0000 ?
+[^:]+:[ 	]+607f 33cc 55aa cdef[ 	]+\.byte[ 	]+0x7f, 0x60, 0xcc, 0x33, 0xaa, 0x55, 0xef, 0xcd, 0xab, 0x89, 0x67, 0x45, 0x23, 0x01, 0x10, 0x32, 0x54, 0x76, 0x98, 0xba, 0xdc, 0x00
+[^:]+:[ 	]+89ab 4567 0123 3210 ?
+[^:]+:[ 	]+7654 ba98 00dc ?
+[^:]+:[ 	]+607f 33cc 55aa cdef[ 	]+\.byte[ 	]+0x7f, 0x60, 0xcc, 0x33, 0xaa, 0x55, 0xef, 0xcd, 0xab, 0x89, 0x67, 0x45, 0x23, 0x01, 0x10, 0x32, 0x54, 0x76, 0x98, 0xba, 0xdc, 0x00
+[^:]+:[ 	]+89ab 4567 0123 3210 ?
+[^:]+:[ 	]+7654 ba98 00dc ?
+[^:]+:[ 	]+607f 33cc 55aa cdef[ 	]+\.byte[ 	]+0x7f, 0x60, 0xcc, 0x33, 0xaa, 0x55, 0xef, 0xcd, 0xab, 0x89, 0x67, 0x45, 0x23, 0x01, 0x10, 0x32, 0x54, 0x76, 0x98, 0xba, 0xdc, 0xfe
+[^:]+:[ 	]+89ab 4567 0123 3210 ?
+[^:]+:[ 	]+7654 ba98 fedc ?
+[^:]+:[ 	]+607f 33cc 55aa cdef[ 	]+\.byte[ 	]+0x7f, 0x60, 0xcc, 0x33, 0xaa, 0x55, 0xef, 0xcd, 0xab, 0x89, 0x67, 0x45, 0x23, 0x01, 0x10, 0x32, 0x54, 0x76, 0x98, 0xba, 0xdc, 0xfe
+[^:]+:[ 	]+89ab 4567 0123 3210 ?
+[^:]+:[ 	]+7654 ba98 fedc ?
diff --git a/gas/testsuite/gas/riscv/insn.s b/gas/testsuite/gas/riscv/insn.s
index 94f62fe5672e..48db59b14e88 100644
--- a/gas/testsuite/gas/riscv/insn.s
+++ b/gas/testsuite/gas/riscv/insn.s
@@ -70,3 +70,12 @@  target:
 	.insn 10, 0x007f
 	.insn 12, 0x107f
 	.insn 22, 0x607f
+
+	.insn 0x8000000000000000007f
+	.insn 10, 0x8000000000000000007f
+	.insn 0x000000000000fedcba98765432100123456789ab607f
+	.insn 22, 0x000000000000fedcba98765432100123456789ab607f
+	.insn 0x00dcba98765432100123456789abcdef55aa33cc607f
+	.insn 22, 0x00dcba98765432100123456789abcdef55aa33cc607f
+	.insn 0xfedcba98765432100123456789abcdef55aa33cc607f
+	.insn 22, 0xfedcba98765432100123456789abcdef55aa33cc607f
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