Skip to content

riscv_zhinx_quick_fix

Tsukasa OI edited this page Oct 4, 2022 · 2 revisions

QUICK fix on Li's Zhinx implementation

Issues Solved

1. Canonical Ordering of H (removed in v2)

  • PATCH v1: PATCH 1/2
  • PATCH v2/v3: none

This is a duplicate of riscv-h-canonical. See this for details.

This patch is independently applied to master and removed in PATCH v2.

2. Most likely Wrong Feature Gate

  • PATCH v1: PATCH 2/2
  • PATCH v2: PATCH 1/1
  • PATCH v3: PATCH 1/2

Following instructions have INSN_CLASS_D_AND_ZFH_INX:

  • fcvt.h.d
  • fcvt.d.h

Following instructions have INSN_CLASS_Q_AND_ZFH_INX:

  • fcvt.h.q
  • fcvt.q.h

I noticed that feature gates on those instruction classes are most likely incorrect.

Quoting ISA Manual 24.5 "Zhinxmin":

If the Zdinx extension is present, the FCVT.D.H and FCVT.H.D instructions are also included.

Although no such limitation is explicitly specified on Zhinx itself, it's very unlikely that FCVT.D.H and FCVT.H.D can be supported without Zdinx extension.

Quoting Manual 16.3 "Half-Precision Conversion and Move Instructions" from Zfh:

If the D extension is present, FCVT.D.H or FCVT.H.D converts a half-precision floating-point number to a double-precision floating-point number, or vice-versa, respectively. If the Q extension is present, FCVT.Q.H or FCVT.H.Q converts a half-precision floating-point number to a quad-precision floating-point number, or vice-versa, respectively.

And 24.4 "Zhinx":

The Zhinx extension adds all of the instructions that the Zfh extension adds, except for the transfer instructions FLH, FSH, FMV.H.X, and FMV.X.H. The Zhinx variants of these Zfh-extension instructions have the same semantics, except that (cont...)

On INSN_CLASS_D_AND_ZFH_INX:

Implementation Feature Gate
Li's implementation (D && Zfh) or Zhinx
This patch (D && Zfh) or (Zdinx && Zhinx)
My combined float patchset
(PATCH v1/v2)
(D && Zfhmin) or (Zdinx && Zhinxmin)

3. Coding Style (added in v3)

  • PATCH v1/v2: none
  • PATCH v3: PATCH 2/2

On riscv_supported_std_z_ext in bfd/elfxx-riscv.c, extension entries should be ordered by its canonical order (it says must on the comment but not strictly required; Zvl* extensions are not strictly ordered now).

So, Zhinx should be moved after all Zv* extensions.

Clone this wiki locally