Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RISC-V: Better support for long instructions (64 < x <= 176 [bits]) #91

Merged
merged 3 commits into from
Nov 28, 2022

Conversation

a4lg
Copy link
Owner

@a4lg a4lg commented Nov 19, 2022

@a4lg a4lg force-pushed the riscv-long-insn-1 branch from ed49fad to 5f4d7bd Compare November 19, 2022 11:57
@a4lg a4lg added the bug Something isn't working label Nov 19, 2022
@a4lg a4lg force-pushed the riscv-long-insn-1 branch 13 times, most recently from 523d124 to 54c5f7e Compare November 26, 2022 00:28
@a4lg a4lg force-pushed the riscv-long-insn-1 branch 2 times, most recently from 9c5d74d to 33d15d8 Compare November 27, 2022 09:42
a4lg added 3 commits November 28, 2022 01:03
Commit bb99669 ("RISC-V/gas: allow generating up to 176-bit
instructions with .insn") tried to start supporting long instructions but
it was insufficient.

On the disassembler, correct ".byte" output was limited to the first 64-bits
of an instruction.  After that, zeroes are incorrectly printed.

Note that, it only happens on ".byte" output (instruction part) and not on
hexdump (data) part.  For example, before this commit, hexdump and ".byte"
produces different values:

Assembly:
  .insn 22, 0xfedcba98765432100123456789abcdef55aa33cc607f
objdump output example (before the fix):
  10:   607f 33cc 55aa cdef     .byte   0x7f, 0x60, 0xcc, 0x33, 0xaa, 0x55, 0xef, 0xcd, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00
  18:   89ab 4567 0123 3210
  20:   7654 ba98 fedc

Note that, after 0xcd (after first 64-bits of the target instruction), all
".byte" values are incorrectly printed as zero while hexdump prints correct
instruction bits.

To resolve this, this commit adds "packet" argument to support dumping
instructions longer than 64-bits (to print correct instruction bits on
".byte").  This commit will be tested on the separate commit.

Assembly:
  .insn 22, 0xfedcba98765432100123456789abcdef55aa33cc607f
objdump output example (after the fix):
  10:   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
  18:   89ab 4567 0123 3210
  20:   7654 ba98 fedc

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.
Commit bb99669 ("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 and
3.  Because long opcode was handled separately (from struct riscv_cl_insn),
    some information like DWARF line number correspondence was missing.

To resolve these problems, this commit:

1.  Handles bignum (and its encodings) precisely and
2.  Incorporates long opcode handling into regular instruction handling.

This commit will be tested on the separate commit.

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.
This commit tests both (assembler and disassembler) fixes of "Better support
for long instructions".

gas/ChangeLog:

	* testsuite/gas/riscv/insn.s: Add testcases such that big number
	handling is required and should be disassembled as long ".byte"
	sequence with correct instruction bits.
	* testsuite/gas/riscv/insn.d: Likewise.
	* testsuite/gas/riscv/insn-na.d: Likewise.
	* testsuite/gas/riscv/insn-dwarf.d: Likewise.
@a4lg a4lg force-pushed the riscv-long-insn-1 branch from 33d15d8 to 405e9d1 Compare November 28, 2022 01:04
@a4lg a4lg merged commit 405e9d1 into master Nov 28, 2022
@a4lg a4lg deleted the riscv-long-insn-1 branch November 28, 2022 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant