From: Tsukasa OI <research_trasio@irq.a4lg.com>
To: Tsukasa OI <research_trasio@irq.a4lg.com>
Cc: binutils@sourceware.org
Subject: [RFC PATCH 0/2] RISC-V: Zfinx fixes/enhancements: Part 2A
Date: Tue, 1 Feb 2022 22:51:12 +0900 [thread overview]
Message-ID: <cover.1643723432.git.research_trasio@irq.a4lg.com> (raw)
In-Reply-To: <cover.1643723292.git.research_trasio@irq.a4lg.com>
[About this patchset]
This is the Part 2A (part 2 option A) of my Z[fdq]inx fixes and
enhancements. See Part 1 for general background.
<https://sourceware.org/pipermail/binutils/2022-February/119570.html>
There is also Part 2B (part 2 option B) I will submit later to fix the
same bug but I prefer option A. See below for details.
[Bugfix in Part 2]
On RV32_Zdinx and RV[32|64]_Zqinx, there is a bug that allows invalid
Zdinx/Zqinx instructions to be emitted and/or disassembled.
GNU Assembler accepts all register index (as long as encodable) but
on Zdinx extension + RV32, all FP64 register numbers must be even
(encoding with odd register numbers is reserved).
For instance, this is valid on RV64_Zdinx but not on RV32_Zdinx.
fadd.d a1, a3, a5
On the other hand, this is valid both on RV[32|64]_Zdinx.
fadd.d a0, a2, a4
Although not ratified, Zqinx would require similar constraints (register
pairs and quad-register groups) as the ISA Manual draft,
section 24.5 says:
> In the future, an RV64Zqinx quad-precision extension could be defined
> analogously to RV32Zdinx. An RV32Zqinx extension could also be
> defined but would require quad-register groups.
[Option A and B]
To address this issue (at least in the disassembler), we need to modify
instruction match functions to reject invalid encodings. However,
because whether given Zdinx/Zqinx instruction is valid depends on XLEN
and matching extension (D/Q or Zdinx/Zqinx), current `match_func' with
current instruction list cannot handle this situation.
We have two options (as I came up with):
** PART 2A (OPTION A) **
1. Separate D/Q and Zdinx/Zqinx completely and split Zdinx/Zqinx per
XLEN (because whether given Zdinx/Zqinx instruction is valid depends
on the XLEN).
For instance,
{"fadd.d", 0, INSN_CLASS_D_OR_ZDINX, ... , 0 },
in riscv_opcodes may be splitted to:
{"fadd.d", 0, INSN_CLASS_D, ... , 0 },
{"fadd.d", 32, INSN_CLASS_ZDINX, ... , 0 },
{"fadd.d", 64, INSN_CLASS_ZDINX, ... , 0 },
to preserve current `match_func' interface simplicity.
We also need 16 new matching functions:
- match_opcode_zdinx_rtype_g2
- match_opcode_zdinx_rtype_g4
For regular arithmetic instructions (rd, rs1 and rs2 are checked)
- match_rs1_eq_rs2_zdinx_rtype_g2
- match_rs1_eq_rs2_zdinx_rtype_g4
For fmv, fneg and fabs pseudoinstructions
(rd and rs1[==rs2] are checked).
- match_opcode_zdinx_r4type_g2
- match_opcode_zdinx_r4type_g4
For FMA instructions (rd and rs1..3 are checked)
- match_opcode_zdinx_itype_g1_2
- match_opcode_zdinx_itype_g1_4
- match_opcode_zdinx_itype_g2_1
- match_opcode_zdinx_itype_g2_4
- match_opcode_zdinx_itype_g4_1
- match_opcode_zdinx_itype_g4_2
Mainly for fcvt instructions (rd and rs1 are checked).
- match_opcode_zdinx_itype_g2_2
- match_opcode_zdinx_itype_g4_4
For fsqrt instructions (rd and rs1 are checked).
- match_opcode_zdinx_cmp_g2
- match_opcode_zdinx_cmp_g4
For compare instructions (rs1 and rs2 are checked)
Downside of this is:
- It almost triples D/Q riscv_opcodes entries. This is bloat.
- We need separate Zdinx/Zqinx entries per XLEN (new riscv_opcodes
entries will be required for RV128_Z[dq]inx).
- We cannot give proper diagnostics as an assembler error
(other than simple "illegal operands") because we purely handle this
issue with general-purpose matching functions.
** PART 2B (OPTION B) **
2. Pass additional information
If we prefer to give proper diagnostics in the assembler, we definitely
need additional flags holding floating type information on each
instruction and we need to validate all floating point register operands
using those information.
For assembler part, this is done in the Part 2B but... this part doesn't
handle disassembler issue (unlike Part 2A).
To address disassembler issue, we ADDITIONALLY need to add some
arguments to `match_func' (RISC-V subset [riscv_parse_subset_t] and
XLEN) and add some matching functions (like Part 2A but the number of
new matching functions would be less) for Zdinx/Zqinx instructions.
The big problem of this approach is bloat (in an other way than Part 2A)
and redundant design.
Why do we need to pass riscv_rps_dis/riscv_rps_as just for Zdinx/Zqinx
instructions? Why do we need to check xlen twice? That's why I stopped
before tackling with the disassembler issue.
[Opinion (submitter prefers option A)]
So my opinion is..., unless RISC-V maintainers prefer option B, option A
would be better and complete. I would like to hear thoughts of RISC-V
people.
Tsukasa OI (2):
RISC-V: Prepare D/Q and Zdinx/Zqinx separation
RISC-V: Validate Zdinx/Zqinx register pairs
bfd/elfxx-riscv.c | 10 +-
include/opcode/riscv.h | 4 +-
opcodes/riscv-opc.c | 541 +++++++++++++++++++++++++++++++++--------
3 files changed, 441 insertions(+), 114 deletions(-)
base-commit: e327c35ef5768789d3ba41a629f178f5eec32790
prerequisite-patch-id: 9e408f2e6186c8956aae077daf95f38b9ad98675
prerequisite-patch-id: 32ea143f7662a3297a7cf809cec6454e788f2916
prerequisite-patch-id: 25d5aa65f72b1b4f1f52c92aa0f8ac30d218cc9c
prerequisite-patch-id: 6599ccdcc15585db285c30e14528f905327fd638
prerequisite-patch-id: a5f3689afda87a68d4faae698c438aa3211521e0
--
2.32.0
next prev parent reply other threads:[~2022-02-01 13:51 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-01 13:49 [PATCH 0/5] RISC-V: Zfinx fixes/enhancements: Part 1 Tsukasa OI
2022-02-01 13:49 ` [PATCH 1/5] RISC-V: Fix disassembling Zfinx with -M numeric Tsukasa OI
2022-02-08 2:00 ` Palmer Dabbelt
2022-02-01 13:49 ` [PATCH 2/5] RISC-V: Make indentation consistent Tsukasa OI
2022-02-08 2:00 ` Palmer Dabbelt
2022-02-01 13:49 ` [PATCH 3/5] RISC-V: Use different registers for testing Tsukasa OI
2022-02-08 2:00 ` Palmer Dabbelt
2022-02-01 13:49 ` [PATCH 4/5] RISC-V: Relax `fmv.[sdq]' requirements Tsukasa OI
2022-02-08 2:00 ` Palmer Dabbelt
2022-02-08 9:51 ` Tsukasa OI
2022-02-01 13:49 ` [PATCH 5/5] RISC-V: Fix RV64_Zqinx to use register pairs Tsukasa OI
2022-02-08 2:00 ` Palmer Dabbelt
2022-02-01 13:51 ` Tsukasa OI [this message]
2022-02-01 13:51 ` [RFC PATCH 1/2] RISC-V: Prepare D/Q and Zdinx/Zqinx separation Tsukasa OI
2022-02-01 13:51 ` [RFC PATCH 2/2] RISC-V: Validate Zdinx/Zqinx register pairs Tsukasa OI
2022-02-08 2:00 ` [RFC PATCH 0/2] RISC-V: Zfinx fixes/enhancements: Part 2A Palmer Dabbelt
2022-02-01 13:52 ` [RFC PATCH 0/2] RISC-V: Zfinx fixes/enhancements: Part 2B Tsukasa OI
2022-02-01 13:52 ` [RFC PATCH 1/2] RISC-V: Add floating point instruction metadata Tsukasa OI
2022-02-01 13:52 ` [RFC PATCH 2/2] RISC-V: Validate Zdinx/Zqinx register pairs Tsukasa OI
2022-02-01 13:53 ` [PATCH 0/4] RISC-V: Zfinx fixes/enhancements: Part 3 Tsukasa OI
2022-02-01 13:53 ` [PATCH 1/4] RISC-V: Add assembler testcases for Zdinx regs Tsukasa OI
2022-02-08 2:00 ` Palmer Dabbelt
2022-02-01 13:53 ` [PATCH 2/4] RISC-V: Add disassembler tests " Tsukasa OI
2022-02-08 2:00 ` Palmer Dabbelt
2022-02-01 13:53 ` [PATCH 3/4] RISC-V: Add assembler testcases for Zqinx regs Tsukasa OI
2022-02-08 2:01 ` Palmer Dabbelt
2022-02-01 13:53 ` [PATCH 4/4] RISC-V: Add disassembler tests " Tsukasa OI
2022-05-22 5:15 ` [PATCH v2 00/11] RISC-V: Zfinx fixes/enhancements Tsukasa OI
2022-05-22 5:15 ` [PATCH v2 01/11] RISC-V: Fix disassembling Zfinx with -M numeric Tsukasa OI
2022-05-22 5:15 ` [PATCH v2 02/11] RISC-V: Make indentation consistent Tsukasa OI
2022-05-22 5:15 ` [PATCH v2 03/11] RISC-V: Use different registers for testing Tsukasa OI
2022-05-22 5:15 ` [PATCH v2 04/11] RISC-V: Relax `fmv.[sdq]' requirements Tsukasa OI
2022-05-22 5:15 ` [PATCH v2 05/11] RISC-V: Fix RV64_Zqinx to use register pairs Tsukasa OI
2022-05-22 5:15 ` [PATCH v2 06/11] RISC-V: Prepare D/Q and Zdinx/Zqinx separation Tsukasa OI
2022-05-22 5:15 ` [PATCH v2 07/11] RISC-V: Validate Zdinx/Zqinx register pairs Tsukasa OI
2022-05-22 5:15 ` [PATCH v2 08/11] RISC-V: Add assembler testcases for Zdinx regs Tsukasa OI
2022-05-22 5:15 ` [PATCH v2 09/11] RISC-V: Add disassembler tests " Tsukasa OI
2022-05-22 5:15 ` [PATCH v2 10/11] RISC-V: Add assembler testcases for Zqinx regs Tsukasa OI
2022-05-22 5:16 ` [PATCH v2 11/11] RISC-V: Add disassembler tests " Tsukasa OI
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=cover.1643723432.git.research_trasio@irq.a4lg.com \
--to=research_trasio@irq.a4lg.com \
--cc=binutils@sourceware.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).