[PATCH 4/5] RISC-V: Relax `fmv.[sdq]' requirements

Tue Feb 8 02:00:52 GMT 2022

On Tue, 01 Feb 2022 05:49:05 PST (-0800), binutils@sourceware.org wrote:
> This commit relaxes requirements to `fmv.s' instructions from F to (F or
> Zfinx).  The same applies to `fmv.d' and `fmv.q'.
>
> gas/ChangeLog:
>
> 	* testsuite/gas/riscv/zfinx.s: Add `fmv.s' instruction.
> 	* testsuite/gas/riscv/zfinx.d: Likewise.
> 	* testsuite/gas/riscv/zdinx.s: Add `fmv.d' instruction.
> 	* testsuite/gas/riscv/zdinx.d: Likewise.
> 	* testsuite/gas/riscv/zqinx.d: Add `fmv.q' instruction.
> 	* testsuite/gas/riscv/zqinx.s: Likewise.
>
> opcodes/ChangeLog:
>
> 	* riscv-opc.c (riscv_opcodes): Relax requirements to
> 	`fmv.[sdq]' instructions to support those in Zfinx/Zdinx/Zqinx.
> ---
>  gas/testsuite/gas/riscv/zdinx.d | 1 +
>  gas/testsuite/gas/riscv/zdinx.s | 1 +
>  gas/testsuite/gas/riscv/zfinx.d | 1 +
>  gas/testsuite/gas/riscv/zfinx.s | 1 +
>  gas/testsuite/gas/riscv/zqinx.d | 1 +
>  gas/testsuite/gas/riscv/zqinx.s | 1 +
>  opcodes/riscv-opc.c             | 6 +++---
>  7 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/gas/testsuite/gas/riscv/zdinx.d b/gas/testsuite/gas/riscv/zdinx.d
> index cb465bfbef4..3db2cb56f1a 100644
> --- a/gas/testsuite/gas/riscv/zdinx.d
> +++ b/gas/testsuite/gas/riscv/zdinx.d
> @@ -36,6 +36,7 @@ Disassembly of section .text:
>  [ 	]+[0-9a-f]+:[ 	]+a2c58553[ 	]+fle.d[ 	]+a0,a1,a2
>  [ 	]+[0-9a-f]+:[ 	]+a2b61553[ 	]+flt.d[ 	]+a0,a2,a1
>  [ 	]+[0-9a-f]+:[ 	]+a2b60553[ 	]+fle.d[ 	]+a0,a2,a1
> +[ 	]+[0-9a-f]+:[ 	]+22b58553[ 	]+fmv.d[ 	]+a0,a1
>  [ 	]+[0-9a-f]+:[ 	]+22b59553[ 	]+fneg.d[ 	]+a0,a1
>  [ 	]+[0-9a-f]+:[ 	]+22b5a553[ 	]+fabs.d[ 	]+a0,a1
>  [ 	]+[0-9a-f]+:[ 	]+e2059553[ 	]+fclass.d[ 	]+a0,a1
> diff --git a/gas/testsuite/gas/riscv/zdinx.s b/gas/testsuite/gas/riscv/zdinx.s
> index f44358111de..cdf5f3c2e7e 100644
> --- a/gas/testsuite/gas/riscv/zdinx.s
> +++ b/gas/testsuite/gas/riscv/zdinx.s
> @@ -28,6 +28,7 @@ target:
>  	fle.d	a0, a1, a2
>  	fgt.d	a0, a1, a2
>  	fge.d	a0, a1, a2
> +	fmv.d	a0, a1
>  	fneg.d	a0, a1
>  	fabs.d	a0, a1
>  	fclass.d	a0, a1
> diff --git a/gas/testsuite/gas/riscv/zfinx.d b/gas/testsuite/gas/riscv/zfinx.d
> index 6465c08ea9a..6fc4491fbc0 100644
> --- a/gas/testsuite/gas/riscv/zfinx.d
> +++ b/gas/testsuite/gas/riscv/zfinx.d
> @@ -34,6 +34,7 @@ Disassembly of section .text:
>  [ 	]+[0-9a-f]+:[ 	]+a0c58553[ 	]+fle.s[ 	]+a0,a1,a2
>  [ 	]+[0-9a-f]+:[ 	]+a0b61553[ 	]+flt.s[ 	]+a0,a2,a1
>  [ 	]+[0-9a-f]+:[ 	]+a0b60553[ 	]+fle.s[ 	]+a0,a2,a1
> +[ 	]+[0-9a-f]+:[ 	]+20b58553[ 	]+fmv.s[ 	]+a0,a1
>  [ 	]+[0-9a-f]+:[ 	]+20b59553[ 	]+fneg.s[ 	]+a0,a1
>  [ 	]+[0-9a-f]+:[ 	]+20b5a553[ 	]+fabs.s[ 	]+a0,a1
>  [ 	]+[0-9a-f]+:[ 	]+e0059553[ 	]+fclass.s[ 	]+a0,a1
> diff --git a/gas/testsuite/gas/riscv/zfinx.s b/gas/testsuite/gas/riscv/zfinx.s
> index 41ae0e38ad4..d63c0c37570 100644
> --- a/gas/testsuite/gas/riscv/zfinx.s
> +++ b/gas/testsuite/gas/riscv/zfinx.s
> @@ -26,6 +26,7 @@ target:
>  	fle.s	a0, a1, a2
>  	fgt.s	a0, a1, a2
>  	fge.s	a0, a1, a2
> +	fmv.s	a0, a1
>  	fneg.s	a0, a1
>  	fabs.s	a0, a1
>  	fclass.s	a0, a1
> diff --git a/gas/testsuite/gas/riscv/zqinx.d b/gas/testsuite/gas/riscv/zqinx.d
> index e8d2b7ba4c5..c704241bc90 100644
> --- a/gas/testsuite/gas/riscv/zqinx.d
> +++ b/gas/testsuite/gas/riscv/zqinx.d
> @@ -38,6 +38,7 @@ Disassembly of section .text:
>  [ 	]+[0-9a-f]+:[ 	]+a6c58553[ 	]+fle.q[ 	]+a0,a1,a2
>  [ 	]+[0-9a-f]+:[ 	]+a6b61553[ 	]+flt.q[ 	]+a0,a2,a1
>  [ 	]+[0-9a-f]+:[ 	]+a6b60553[ 	]+fle.q[ 	]+a0,a2,a1
> +[ 	]+[0-9a-f]+:[ 	]+26b58553[ 	]+fmv.q[ 	]+a0,a1
>  [ 	]+[0-9a-f]+:[ 	]+26b59553[ 	]+fneg.q[ 	]+a0,a1
>  [ 	]+[0-9a-f]+:[ 	]+26b5a553[ 	]+fabs.q[ 	]+a0,a1
>  [ 	]+[0-9a-f]+:[ 	]+e6059553[ 	]+fclass.q[ 	]+a0,a1
> diff --git a/gas/testsuite/gas/riscv/zqinx.s b/gas/testsuite/gas/riscv/zqinx.s
> index ecfa509b98c..02147b1919c 100644
> --- a/gas/testsuite/gas/riscv/zqinx.s
> +++ b/gas/testsuite/gas/riscv/zqinx.s
> @@ -30,6 +30,7 @@ target:
>  	fle.q	a0, a1, a2
>  	fgt.q	a0, a1, a2
>  	fge.q	a0, a1, a2
> +	fmv.q	a0, a1
>  	fneg.q	a0, a1
>  	fabs.q	a0, a1
>  	fclass.q	a0, a1

Looking at the ISA manual, I'm not actually seeing Zqinx defined.  
Specifically

    \begin{commentary}
    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.
    \end{commentary}

Looks like it was removed from the ISA manual here

    9025a7f Remove Zqinx (for now, at least)

I must have missed that when reviewing the patches last time, but not 
entirely sure what to do as we're about to release it.  None of that 
shouldn't block this patch set, though.

> diff --git a/opcodes/riscv-opc.c b/opcodes/riscv-opc.c
> index 2da0f7cf0a4..991d4d7a0aa 100644
> --- a/opcodes/riscv-opc.c
> +++ b/opcodes/riscv-opc.c
> @@ -598,7 +598,7 @@ const struct riscv_opcode riscv_opcodes[] =
>  {"fmv.w.x",    0, INSN_CLASS_F,   "D,s",       MATCH_FMV_S_X, MASK_FMV_S_X, match_opcode, 0 },
>  {"fmv.x.s",    0, INSN_CLASS_F,   "d,S",       MATCH_FMV_X_S, MASK_FMV_X_S, match_opcode, 0 },
>  {"fmv.s.x",    0, INSN_CLASS_F,   "D,s",       MATCH_FMV_S_X, MASK_FMV_S_X, match_opcode, 0 },
> -{"fmv.s",      0, INSN_CLASS_F,   "D,U",       MATCH_FSGNJ_S, MASK_FSGNJ_S, match_rs1_eq_rs2, INSN_ALIAS },
> +{"fmv.s",      0, INSN_CLASS_F_OR_ZFINX,   "D,U",       MATCH_FSGNJ_S, MASK_FSGNJ_S, match_rs1_eq_rs2, INSN_ALIAS },
>  {"fneg.s",     0, INSN_CLASS_F_OR_ZFINX,   "D,U",       MATCH_FSGNJN_S, MASK_FSGNJN_S, match_rs1_eq_rs2, INSN_ALIAS },
>  {"fabs.s",     0, INSN_CLASS_F_OR_ZFINX,   "D,U",       MATCH_FSGNJX_S, MASK_FSGNJX_S, match_rs1_eq_rs2, INSN_ALIAS },
>  {"fsgnj.s",    0, INSN_CLASS_F_OR_ZFINX,   "D,S,T",     MATCH_FSGNJ_S, MASK_FSGNJ_S, match_opcode, 0 },
> @@ -656,7 +656,7 @@ const struct riscv_opcode riscv_opcodes[] =
>  {"fsd",        0, INSN_CLASS_D_AND_C, "CD,Cl(Cs)", MATCH_C_FSD, MASK_C_FSD, match_opcode, INSN_ALIAS|INSN_DREF|INSN_8_BYTE },
>  {"fsd",        0, INSN_CLASS_D,   "T,q(s)",    MATCH_FSD, MASK_FSD, match_opcode, INSN_DREF|INSN_8_BYTE },
>  {"fsd",        0, INSN_CLASS_D,   "T,A,s",     0, (int) M_FSD, match_never, INSN_MACRO },
> -{"fmv.d",      0, INSN_CLASS_D,   "D,U",       MATCH_FSGNJ_D, MASK_FSGNJ_D, match_rs1_eq_rs2, INSN_ALIAS },
> +{"fmv.d",      0, INSN_CLASS_D_OR_ZDINX,   "D,U",       MATCH_FSGNJ_D, MASK_FSGNJ_D, match_rs1_eq_rs2, INSN_ALIAS },
>  {"fneg.d",     0, INSN_CLASS_D_OR_ZDINX,   "D,U",       MATCH_FSGNJN_D, MASK_FSGNJN_D, match_rs1_eq_rs2, INSN_ALIAS },
>  {"fabs.d",     0, INSN_CLASS_D_OR_ZDINX,   "D,U",       MATCH_FSGNJX_D, MASK_FSGNJX_D, match_rs1_eq_rs2, INSN_ALIAS },
>  {"fsgnj.d",    0, INSN_CLASS_D_OR_ZDINX,   "D,S,T",     MATCH_FSGNJ_D, MASK_FSGNJ_D, match_opcode, 0 },
> @@ -713,7 +713,7 @@ const struct riscv_opcode riscv_opcodes[] =
>  {"flq",        0, INSN_CLASS_Q,   "D,A,s",     0, (int) M_FLQ, match_never, INSN_MACRO },
>  {"fsq",        0, INSN_CLASS_Q,   "T,q(s)",    MATCH_FSQ, MASK_FSQ, match_opcode, INSN_DREF|INSN_16_BYTE },
>  {"fsq",        0, INSN_CLASS_Q,   "T,A,s",     0, (int) M_FSQ, match_never, INSN_MACRO },
> -{"fmv.q",      0, INSN_CLASS_Q,   "D,U",       MATCH_FSGNJ_Q, MASK_FSGNJ_Q, match_rs1_eq_rs2, INSN_ALIAS },
> +{"fmv.q",      0, INSN_CLASS_Q_OR_ZQINX,   "D,U",       MATCH_FSGNJ_Q, MASK_FSGNJ_Q, match_rs1_eq_rs2, INSN_ALIAS },
>  {"fneg.q",     0, INSN_CLASS_Q_OR_ZQINX,   "D,U",       MATCH_FSGNJN_Q, MASK_FSGNJN_Q, match_rs1_eq_rs2, INSN_ALIAS },
>  {"fabs.q",     0, INSN_CLASS_Q_OR_ZQINX,   "D,U",       MATCH_FSGNJX_Q, MASK_FSGNJX_Q, match_rs1_eq_rs2, INSN_ALIAS },
>  {"fsgnj.q",    0, INSN_CLASS_Q_OR_ZQINX,   "D,S,T",     MATCH_FSGNJ_Q, MASK_FSGNJ_Q, match_opcode, 0 },

Presumably whomever wrote this assumed these were flavors of move, but 
the ISA manual is pretty clear that they're not

    Note, FSGNJ.S {\em rx, ry, ry} moves {\em ry} to {\em rx} (assembler 
    pseudoinstruction FMV.S {\em rx, ry});

Thus they're in Zfinx, as they're not explicitly omitted.

Reviewed-by: Palmer Dabbelt <palmer@rivosinc.com>

Thanks!