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

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 <[email protected]> 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 <[email protected]>
>