diff mbox series

[v3,01/13] RISC-V: add dedicated vector arithmetic .insn forms

Message ID 81e3d6d0-39d3-42e3-a8cd-d6b11b8449a4@suse.com
State New
Headers
Series RISC-V: assorted fixes and (hopefully) improvements |

Commit Message

Jan Beulich June 19, 2026, 11:47 a.m. UTC 
  As .insn documentation states, using the available .insn forms can be a
little challenging. For OP_V and OP_VE, help the situation by providing a
few more dedicated forms.
---
No form is provided for OPCFG: Such insns may better use "r" forms, or
whatever is best suited there.

For OPIVI permitting both signed (Vi) and unsigned (Vj) immediates might
be nice, but won't work with the present parsing (needs delaying
as_bad()).

To help encoding V*UNARY* insns, forms with immediates in the respective
positions may be helpful. Thoughts? (Some of the testcase additions are
commented upon accordingly.)
---
v3: Add O7 as first operand.

Comments

Jiawei June 22, 2026, 7:45 a.m. UTC | #1 
> As .insn documentation states, using the available .insn forms can be a
> little challenging. For OP_V and OP_VE, help the situation by providing a
> few more dedicated forms.
> ---
> No form is provided for OPCFG: Such insns may better use "r" forms, or
> whatever is best suited there.
>
> For OPIVI permitting both signed (Vi) and unsigned (Vj) immediates might
> be nice, but won't work with the present parsing (needs delaying
> as_bad()).
>
> To help encoding V*UNARY* insns, forms with immediates in the respective
> positions may be helpful. Thoughts? (Some of the testcase additions are
> commented upon accordingly.)
> ---
> v3: Add O7 as first operand.
>
> --- a/opcodes/riscv-opc.c
> +++ b/opcodes/riscv-opc.c
> @@ -3660,6 +3660,17 @@ const struct riscv_opcode riscv_insn_typ
>   {"j",       0, INSN_CLASS_I,       "O7,d,a",            0, 0, NULL, 0 },
>   {"j",       0, INSN_CLASS_F,       "O7,D,a",            0, 0, NULL, 0 },
>   
> +{"ivv",     0, INSN_CLASS_V,       "O7,F6,Vd,Vt,VsVm", 0x0000, 0x7000, NULL, 0 },
> +{"fvv",     0, INSN_CLASS_ZVEF,    "O7,F6,Vd,Vt,VsVm", 0x1000, 0x7000, NULL, 0 },
> +{"fvv",     0, INSN_CLASS_ZVEF,    "O7,F6,D,Vt,VsVm",  0x1000, 0x7000, NULL, 0 },
> +{"mvv",     0, INSN_CLASS_V,       "O7,F6,Vd,Vt,VsVm", 0x2000, 0x7000, NULL, 0 },
> +{"mvv",     0, INSN_CLASS_V,       "O7,F6,d,Vt,VsVm",  0x2000, 0x7000, NULL, 0 },
> +{"ivi",     0, INSN_CLASS_V,       "O7,F6,Vd,Vt,ViVm", 0x3000, 0x7000, NULL, 0 },
> +{"ivx",     0, INSN_CLASS_V,       "O7,F6,Vd,Vt,sVm",  0x4000, 0x7000, NULL, 0 },
> +{"fvf",     0, INSN_CLASS_ZVEF,    "O7,F6,Vd,Vt,SVm",  0x5000, 0x7000, NULL, 0 },
> +{"mvx",     0, INSN_CLASS_V,       "O7,F6,Vd,Vt,sVm",  0x6000, 0x7000, NULL, 0 },
> +{"mvx",     0, INSN_CLASS_V,       "O7,F6,d,Vt,sVm",   0x6000, 0x7000, NULL, 0 },
> +
>   {"cr",      0, INSN_CLASS_ZCA,       "O2,CF4,d,CV",       0, 0, NULL, 0 },
>   {"cr",      0, INSN_CLASS_ZCF, "O2,CF4,D,CV",       0, 0, NULL, 0 },
>   {"cr",      0, INSN_CLASS_ZCF, "O2,CF4,d,CT",       0, 0, NULL, 0 },

LGTM.

Adding O7 as the first operand looks reasonable to me. It lets the vector
.insn forms cover both OP_V and OP_VE, while F6 still selects the operation
within the selected funct3 class.

Regarding the open questions:

* Not adding a dedicated OPCFG form looks fine to me. These encodings seem
less common, and using the generic r form should be acceptable there.

* Allowing both signed and unsigned immediate operands for OPIVI would 
be nice,
but I agree that this probably needs parser-side changes to avoid diagnosing
too early. That seems better handled as a follow-up.

* Dedicated forms for V*UNARY* encodings could make those cases clearer,
especially where an operand position is effectively used as an opcode or
immediate field. But I do not think that needs to block the current OP_V /
OP_VE arithmetic forms.

Reviewed-by: Jiawei <jiawei@iscas.ac.cn>
Nelson Chu July 19, 2026, 10:33 a.m. UTC | #2 
LGTM +1.  A minor thing (this patch is already good enough to me
without any change) is do you think we need to define extra MASK/MATCH
for funct3 to represent bits 10-12 rather than use 0x7000 directly?
So that might make it immediately clear to understand what these
values represent.

Thanks
Nelson

On Mon, Jun 22, 2026 at 3:46 PM Jiawei <jiawei@iscas.ac.cn> wrote:
>
> > As .insn documentation states, using the available .insn forms can be a
> > little challenging. For OP_V and OP_VE, help the situation by providing a
> > few more dedicated forms.
> > ---
> > No form is provided for OPCFG: Such insns may better use "r" forms, or
> > whatever is best suited there.
> >
> > For OPIVI permitting both signed (Vi) and unsigned (Vj) immediates might
> > be nice, but won't work with the present parsing (needs delaying
> > as_bad()).
> >
> > To help encoding V*UNARY* insns, forms with immediates in the respective
> > positions may be helpful. Thoughts? (Some of the testcase additions are
> > commented upon accordingly.)
> > ---
> > v3: Add O7 as first operand.
> >
> > --- a/opcodes/riscv-opc.c
> > +++ b/opcodes/riscv-opc.c
> > @@ -3660,6 +3660,17 @@ const struct riscv_opcode riscv_insn_typ
> >   {"j",       0, INSN_CLASS_I,       "O7,d,a",            0, 0, NULL, 0 },
> >   {"j",       0, INSN_CLASS_F,       "O7,D,a",            0, 0, NULL, 0 },
> >
> > +{"ivv",     0, INSN_CLASS_V,       "O7,F6,Vd,Vt,VsVm", 0x0000, 0x7000, NULL, 0 },
> > +{"fvv",     0, INSN_CLASS_ZVEF,    "O7,F6,Vd,Vt,VsVm", 0x1000, 0x7000, NULL, 0 },
> > +{"fvv",     0, INSN_CLASS_ZVEF,    "O7,F6,D,Vt,VsVm",  0x1000, 0x7000, NULL, 0 },
> > +{"mvv",     0, INSN_CLASS_V,       "O7,F6,Vd,Vt,VsVm", 0x2000, 0x7000, NULL, 0 },
> > +{"mvv",     0, INSN_CLASS_V,       "O7,F6,d,Vt,VsVm",  0x2000, 0x7000, NULL, 0 },
> > +{"ivi",     0, INSN_CLASS_V,       "O7,F6,Vd,Vt,ViVm", 0x3000, 0x7000, NULL, 0 },
> > +{"ivx",     0, INSN_CLASS_V,       "O7,F6,Vd,Vt,sVm",  0x4000, 0x7000, NULL, 0 },
> > +{"fvf",     0, INSN_CLASS_ZVEF,    "O7,F6,Vd,Vt,SVm",  0x5000, 0x7000, NULL, 0 },
> > +{"mvx",     0, INSN_CLASS_V,       "O7,F6,Vd,Vt,sVm",  0x6000, 0x7000, NULL, 0 },
> > +{"mvx",     0, INSN_CLASS_V,       "O7,F6,d,Vt,sVm",   0x6000, 0x7000, NULL, 0 },
> > +
> >   {"cr",      0, INSN_CLASS_ZCA,       "O2,CF4,d,CV",       0, 0, NULL, 0 },
> >   {"cr",      0, INSN_CLASS_ZCF, "O2,CF4,D,CV",       0, 0, NULL, 0 },
> >   {"cr",      0, INSN_CLASS_ZCF, "O2,CF4,d,CT",       0, 0, NULL, 0 },
>
> LGTM.
>
> Adding O7 as the first operand looks reasonable to me. It lets the vector
> .insn forms cover both OP_V and OP_VE, while F6 still selects the operation
> within the selected funct3 class.
>
> Regarding the open questions:
>
> * Not adding a dedicated OPCFG form looks fine to me. These encodings seem
> less common, and using the generic r form should be acceptable there.
>
> * Allowing both signed and unsigned immediate operands for OPIVI would
> be nice,
> but I agree that this probably needs parser-side changes to avoid diagnosing
> too early. That seems better handled as a follow-up.
>
> * Dedicated forms for V*UNARY* encodings could make those cases clearer,
> especially where an operand position is effectively used as an opcode or
> immediate field. But I do not think that needs to block the current OP_V /
> OP_VE arithmetic forms.
>
> Reviewed-by: Jiawei <jiawei@iscas.ac.cn>
>
Jan Beulich July 20, 2026, 5:59 a.m. UTC | #3 
On 19.07.2026 12:33, Nelson Chu wrote:
> LGTM +1.

Thanks for taking a look.

>  A minor thing (this patch is already good enough to me
> without any change) is do you think we need to define extra MASK/MATCH
> for funct3 to represent bits 10-12 rather than use 0x7000 directly?
> So that might make it immediately clear to understand what these
> values represent.

Personally I'm really of the opposite opinion: The huge amount of MASK /
MATCH pairs is clutter. See how e.g. aarch64-tbl.h gets away (almost?)
completely without. And it's (imo) better readable that way.

Jan
diff mbox series

Patch

--- a/gas/config/tc-riscv.c
+++ b/gas/config/tc-riscv.c
@@ -1631,6 +1631,7 @@  validate_riscv_insn (const struct riscv_
 	  switch (*++oparg)
 	    {
 	    case '7': USE_BITS (OP_MASK_FUNCT7, OP_SH_FUNCT7); break;
+	    case '6': USE_BITS (OP_MASK_FUNCT6, OP_SH_FUNCT6); break;
 	    case '3': USE_BITS (OP_MASK_FUNCT3, OP_SH_FUNCT3); break;
 	    case '2': USE_BITS (OP_MASK_FUNCT2, OP_SH_FUNCT2); break;
 	    default:
@@ -3763,6 +3764,10 @@  riscv_ip (char *str, struct riscv_cl_ins
 		  INSERT_OPERAND (FUNCT7, *ip, imm_expr->X_add_number);
 		  break;
 
+		case 6:
+		  INSERT_OPERAND (FUNCT6, *ip, imm_expr->X_add_number);
+		  break;
+
 		case 3:
 		  INSERT_OPERAND (FUNCT3, *ip, imm_expr->X_add_number);
 		  break;
--- a/gas/testsuite/gas/riscv/insn.d
+++ b/gas/testsuite/gas/riscv/insn.d
@@ -70,6 +70,20 @@  Disassembly of section .text:
 [^:]+:[ 	]+00c58533[ 	]+add[ 	]+a0,a1,a2
 [^:]+:[ 	]+00c58533[ 	]+add[ 	]+a0,a1,a2
 [^:]+:[ 	]+022180d7[ 	]+vadd\.vv[ 	]+v1,v2,v3
+[^:]+:[ 	]+2c2081d7[ 	]+vxor\.vv[ 	]+v3,v2,v1,v0\.t
+[^:]+:[ 	]+182091d7[ 	]+vfmax\.vv[ 	]+v3,v2,v1,v0\.t
+[^:]+:[ 	]+fc1111d7[ 	]+vfwnmsac\.vv[ 	]+v3,v2,v1,v0\.t
+[^:]+:[ 	]+422010d7[ 	]+vfmv\.f\.s[ 	]+ft1,v2
+[^:]+:[ 	]+7a20a1d7[ 	]+vmnor\.mm[ 	]+v3,v2,v1
+[^:]+:[ 	]+422020d7[ 	]+vmv\.x\.s[ 	]+ra,v2
+[^:]+:[ 	]+8220a1f7[ 	]+vsm3me\.vv[ 	]+v3,v2,v1
+[^:]+:[ 	]+2c20b1d7[ 	]+vxor\.vi[ 	]+v3,v2,1,v0\.t
+[^:]+:[ 	]+2c20c1d7[ 	]+vxor\.vx[ 	]+v3,v2,ra,v0\.t
+[^:]+:[ 	]+1820d1d7[ 	]+vfmax\.vf[ 	]+v3,v2,ft1,v0\.t
+[^:]+:[ 	]+fc1151d7[ 	]+vfwnmsac\.vf[ 	]+v3,ft2,v1,v0\.t
+[^:]+:[ 	]+420150d7[ 	]+vfmv\.s\.f[ 	]+v1,ft2
+[^:]+:[ 	]+bc1161d7[ 	]+vnmsac\.vx[ 	]+v3,sp,v1,v0\.t
+[^:]+:[ 	]+420160d7[ 	]+vmv\.s\.x[ 	]+v1,sp
 [^:]+:[ 	]+0001[ 	]+nop
 [^:]+:[ 	]+00000013[ 	]+nop
 [^:]+:[ 	]+001f 0000 0000[ 	].*
--- a/gas/testsuite/gas/riscv/insn.s
+++ b/gas/testsuite/gas/riscv/insn.s
@@ -56,6 +56,24 @@  target:
 
 	.insn r  OP_V, 0, 1, x1, x3, x2
 
+	.option push
+	.option arch, +v,+zvksh			# the latter only for disassembly
+	.insn ivv OP_V, 0x0b, v3, v2, v1, v0.t
+	.insn fvv OP_V, 0x06, v3, v2, v1, v0.t
+	.insn fvv OP_V, 0x3f, v3, v1, v2, v0.t
+	.insn fvv OP_V, 0x10, f1, v2, v0	# last operand is opcode
+	.insn mvv OP_V, 0x1e, v3, v2, v1
+	.insn mvv OP_V, 0x10, x1, v2, v0	# last operand is opcode
+	.insn mvv OP_VE, 0x20, v3, v2, v1
+	.insn ivi OP_V, 0x0b, v3, v2, 1, v0.t
+	.insn ivx OP_V, 0x0b, v3, v2, x1, v0.t
+	.insn fvf OP_V, 0x06, v3, v2, f1, v0.t
+	.insn fvf OP_V, 0x3f, v3, v1, f2, v0.t
+	.insn fvf OP_V, 0x10, v1, v0, f2	# middle operand is opcode
+	.insn mvx OP_V, 0x2f, v3, v1, x2, v0.t
+	.insn mvx OP_V, 0x10, v1, v0, x2	# middle operand is opcode
+	.option pop
+
 	.insn 0x0001
 	.insn 0x00000013
 	.insn 0x0000001f
--- a/gas/testsuite/gas/riscv/insn-dwarf.d
+++ b/gas/testsuite/gas/riscv/insn-dwarf.d
@@ -60,28 +60,42 @@  insn.s +53 +0x9a.*
 insn.s +54 +0x9e.*
 insn.s +55 +0xa2.*
 insn.s +57 +0xa6.*
-insn.s +59 +0xaa.*
-insn.s +60 +0xac.*
-insn.s +61 +0xb0.*
-insn.s +62 +0xb6.*
-insn.s +63 +0xbe.*
-insn.s +64 +0xc8.*
-insn.s +65 +0xd4.*
-insn.s +66 +0xea.*
-insn.s +67 +0xec.*
-insn.s +68 +0xf0.*
-insn.s +69 +0xf6.*
-insn.s +70 +0xfe.*
-insn.s +71 +0x108.*
-insn.s +72 +0x114.*
-insn.s +74 +0x12a.*
-insn.s +75 +0x134.*
-insn.s +76 +0x13e.*
-insn.s +77 +0x154.*
-insn.s +78 +0x16a.*
-insn.s +79 +0x180.*
-insn.s +80 +0x196.*
-insn.s +81 +0x1ac.*
-insn.s +83 +0x1c2.*
-insn.s +- +0x1c6
+insn.s +61 +0xaa.*
+insn.s +62 +0xae.*
+insn.s +63 +0xb2.*
+insn.s +64 +0xb6.*
+insn.s +65 +0xba.*
+insn.s +66 +0xbe.*
+insn.s +67 +0xc2.*
+insn.s +68 +0xc6.*
+insn.s +69 +0xca.*
+insn.s +70 +0xce.*
+insn.s +71 +0xd2.*
+insn.s +72 +0xd6.*
+insn.s +73 +0xda.*
+insn.s +74 +0xde.*
+insn.s +77 +0xe2.*
+insn.s +78 +0xe4.*
+insn.s +79 +0xe8.*
+insn.s +80 +0xee.*
+insn.s +81 +0xf6.*
+insn.s +82 +0x100.*
+insn.s +83 +0x10c.*
+insn.s +84 +0x122.*
+insn.s +85 +0x124.*
+insn.s +86 +0x128.*
+insn.s +87 +0x12e.*
+insn.s +88 +0x136.*
+insn.s +89 +0x140.*
+insn.s +90 +0x14c.*
+insn.s +92 +0x162.*
+insn.s +93 +0x16c.*
+insn.s +94 +0x176.*
+insn.s +95 +0x18c.*
+insn.s +96 +0x1a2.*
+insn.s +97 +0x1b8.*
+insn.s +98 +0x1ce.*
+insn.s +99 +0x1e4.*
+insn.s +101 +0x1fa.*
+insn.s +- +0x1fe
 #pass
--- a/gas/testsuite/gas/riscv/insn-na.d
+++ b/gas/testsuite/gas/riscv/insn-na.d
@@ -59,6 +59,20 @@  Disassembly of section .text:
 [^:]+:[ 	]+00c58533[ 	]+add[ 	]+a0,a1,a2
 [^:]+:[ 	]+00c58533[ 	]+add[ 	]+a0,a1,a2
 [^:]+:[ 	]+022180d7[ 	]+vadd\.vv[ 	]+v1,v2,v3
+[^:]+:[ 	]+2c2081d7[ 	]+vxor\.vv[ 	]+v3,v2,v1,v0\.t
+[^:]+:[ 	]+182091d7[ 	]+vfmax\.vv[ 	]+v3,v2,v1,v0\.t
+[^:]+:[ 	]+fc1111d7[ 	]+vfwnmsac\.vv[ 	]+v3,v2,v1,v0\.t
+[^:]+:[ 	]+422010d7[ 	]+vfmv\.f\.s[ 	]+ft1,v2
+[^:]+:[ 	]+7a20a1d7[ 	]+vmnor\.mm[ 	]+v3,v2,v1
+[^:]+:[ 	]+422020d7[ 	]+vmv\.x\.s[ 	]+ra,v2
+[^:]+:[ 	]+8220a1f7[ 	]+vsm3me\.vv[ 	]+v3,v2,v1
+[^:]+:[ 	]+2c20b1d7[ 	]+vxor\.vi[ 	]+v3,v2,1,v0\.t
+[^:]+:[ 	]+2c20c1d7[ 	]+vxor\.vx[ 	]+v3,v2,ra,v0\.t
+[^:]+:[ 	]+1820d1d7[ 	]+vfmax\.vf[ 	]+v3,v2,ft1,v0\.t
+[^:]+:[ 	]+fc1151d7[ 	]+vfwnmsac\.vf[ 	]+v3,ft2,v1,v0\.t
+[^:]+:[ 	]+420150d7[ 	]+vfmv\.s\.f[ 	]+v1,ft2
+[^:]+:[ 	]+bc1161d7[ 	]+vnmsac\.vx[ 	]+v3,sp,v1,v0\.t
+[^:]+:[ 	]+420160d7[ 	]+vmv\.s\.x[ 	]+v1,sp
 [^:]+:[ 	]+0001[ 	]+c\.addi[ 	]+zero,0
 [^:]+:[ 	]+00000013[ 	]+addi[ 	]+zero,zero,0
 [^:]+:[ 	]+001f 0000 0000[ 	].*
--- a/include/opcode/riscv.h
+++ b/include/opcode/riscv.h
@@ -351,6 +351,9 @@  static inline unsigned int riscv_insn_le
 
 /* RVV fields.  */
 
+#define OP_MASK_FUNCT6		0x3fU
+#define OP_SH_FUNCT6		26
+
 #define OP_MASK_VD		0x1f
 #define OP_SH_VD		7
 #define OP_MASK_VS1		0x1f
--- a/opcodes/riscv-opc.c
+++ b/opcodes/riscv-opc.c
@@ -3660,6 +3660,17 @@  const struct riscv_opcode riscv_insn_typ
 {"j",       0, INSN_CLASS_I,       "O7,d,a",            0, 0, NULL, 0 },
 {"j",       0, INSN_CLASS_F,       "O7,D,a",            0, 0, NULL, 0 },
 
+{"ivv",     0, INSN_CLASS_V,       "O7,F6,Vd,Vt,VsVm", 0x0000, 0x7000, NULL, 0 },
+{"fvv",     0, INSN_CLASS_ZVEF,    "O7,F6,Vd,Vt,VsVm", 0x1000, 0x7000, NULL, 0 },
+{"fvv",     0, INSN_CLASS_ZVEF,    "O7,F6,D,Vt,VsVm",  0x1000, 0x7000, NULL, 0 },
+{"mvv",     0, INSN_CLASS_V,       "O7,F6,Vd,Vt,VsVm", 0x2000, 0x7000, NULL, 0 },
+{"mvv",     0, INSN_CLASS_V,       "O7,F6,d,Vt,VsVm",  0x2000, 0x7000, NULL, 0 },
+{"ivi",     0, INSN_CLASS_V,       "O7,F6,Vd,Vt,ViVm", 0x3000, 0x7000, NULL, 0 },
+{"ivx",     0, INSN_CLASS_V,       "O7,F6,Vd,Vt,sVm",  0x4000, 0x7000, NULL, 0 },
+{"fvf",     0, INSN_CLASS_ZVEF,    "O7,F6,Vd,Vt,SVm",  0x5000, 0x7000, NULL, 0 },
+{"mvx",     0, INSN_CLASS_V,       "O7,F6,Vd,Vt,sVm",  0x6000, 0x7000, NULL, 0 },
+{"mvx",     0, INSN_CLASS_V,       "O7,F6,d,Vt,sVm",   0x6000, 0x7000, NULL, 0 },
+
 {"cr",      0, INSN_CLASS_ZCA,       "O2,CF4,d,CV",       0, 0, NULL, 0 },
 {"cr",      0, INSN_CLASS_ZCF, "O2,CF4,D,CV",       0, 0, NULL, 0 },
 {"cr",      0, INSN_CLASS_ZCF, "O2,CF4,d,CT",       0, 0, NULL, 0 },