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

[Ferran Pallarès Roca via Phabricator via cfe-commits]

fpallares added inline comments.


================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:2283
+      unsigned Src2Reg = Inst.getOperand(1).getReg();
+      if (DestReg == Src2Reg)
+        return Error(Loc, "The destination vector register group cannot overlap"
----------------
HsiangKai wrote:
> fpallares wrote:
> > In this function we are validating the overlap constraints by checking whether `DestReg` matches a vector operand. However, for widenings and narrowings those checks should be extended to validate that the operand doesn't belong to the same register group.
> > 
> > For instance:
> > 
> > `vwadd.vv v4, v4, v7`: The current code would give an error for this instruction.
> > `vwadd.vv v4, v5, v7`: There would be no error for this, even though `v5` will always overlap with any widened group of `v4`.
> > `vwadd.vv v4, v6, v7`: This should not emit any diagnostic because we can't really tell at this point if there will be an overlap.
> > 
> > Perhaps we are already checking this somewhere else and I missed it?
> > 
> > (Note that this will probably change with fractional LMUL in 0.9)
> > 
> Good point!
> 
> Because there is no LMUL information in these instructions, we cannot detect the invalid cases precisely. I only detect destination and source occupy the same register. Maybe I could detect destination and (source + 1). The smallest LMUL is 1 in v0.8.
> 
> After this patch, I am going to prepare another one to upgrade the MC layer to v0.9. I will review this part.
> 
> Do you have any suggestions?
Yes, I was thinking on the (source + 1) case too. I don't think much can be done in v0.9 at the MC layer, since this case would be correct for LMUL < 1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69987