[PATCH] D69987: [RISCV] Assemble/Disassemble v-ext instructions.

[Roger Ferrer Ibanez via Phabricator via llvm-commits]

rogfer01 added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:459
+defm VNSRL_W : VALU_IV_V_X_I<"vnsrl", 0b101100, uimm5, "w">;
+defm VNSRA_W : VALU_IV_V_X_I<"vnsra", 0b101101, uimm5, "w">;
+
----------------
HsiangKai wrote:
> fpallares wrote:
> > In section [[ https://github.com/riscv/riscv-v-spec/blob/master/v-spec.adoc#113-narrowing-vector-arithmetic-instructions | 11.3. Narrowing Vector Arithmetic Instructions ]] we find the following paragraph:
> > 
> > > The destination vector register group cannot overlap the first source vector register group (specified by vs2). The destination vector register group cannot overlap the mask register if used, unless LMUL=1. If either constraint is violated, an illegal instruction exception is raised.
> > 
> > From my understanding, this applies to `VNSRL` and `VNSRA`. In that case we need an `@earlyclobber` constraint here.
> In current implementation for MC layer, there is no LMUL information in these instructions. So, if LMUL=1, there is no such restriction for them. I have a downstream patch to extend these instruction definitions for LMUL > 1. Maybe we could keep this MC implementation for LMUL=1 and extend it later. I will add comments for it.
Perhaps the wording in the spec is unclear here?

Looks like the following sentence does apply regardless of LMUL.

>> The destination vector register group cannot overlap the first source vector register group (specified by vs2).  

While the sentence that follows it is clear that applies only to LMUL>1

>> The destination vector register group cannot overlap the mask register if used, unless LMUL=1

You also said that

> I have a downstream patch to extend these instruction definitions for LMUL > 1

I'm curious here: did you use a Pseudo for that or somehow avoided the issue that they'd be encoded in the same way? 

Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69987/new/

https://reviews.llvm.org/D69987