[RFC PATCH 0/2] RISC-V: Zfinx fixes/enhancements: Part 2A

[Tsukasa OI]

[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