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

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

fpallares added a comment.

I've added a couple more inline comments.

I've also noticed that we aren't emitting the aliases for unmasked instructions, for instance:

  (input → output)
  
  vnot.v v8, v4, v0.t → vnot.v v8, v4, v0.t
  ​vnot.v v8, v4       → vxor.vi v8, v4, -1

AFAICT RISC-V doesn't define a preferred way to disassemble, so I'd say this is something minor.
After looking a bit into it I believe this is caused by `MCInstPrinter` not expecting the `VMaskOp` operand to be set to `NoRegister`, and failing to match the alias pattern. We have a downstream patch that makes this case work, though it's a bit hackish.



================
Comment at: llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp:1078
   case Match_InvalidUImm5:
     return generateImmOutOfRangeError(Operands, ErrorInfo, 0, (1 << 5) - 1);
   case Match_InvalidSImm6:
----------------
We are missing a case for the newly added `InvalidSImm5Plus1` here.

Perhaps something in the lines of:

```
    case Match_InvalidSImm5Plus1:                                                 
      return generateImmOutOfRangeError(                                          
          Operands, ErrorInfo, -(1 << 4) + 1, (1 << 4),                           
          "immediate must be in the range");                                      
```

This can be tested using the following input:
```
vmslt.vi v1, v2, -16                                                            
vmslt.vi v1, v2, 17                                                             
```

(we would expect the diagnostic `error: immediate must be in the range [-15, 16]`)


================
Comment at: llvm/lib/Target/RISCV/RISCVRegisterInfo.td:296
+
+def VDataVT : RegisterTypes<[nxv8i8, nxv16i8, nxv32i8,
+                            nxv4i16, nxv8i16, nxv16i16, nxv32i16,
----------------
I think this definition is no longer used in the latest version of the patch.


================
Comment at: llvm/test/MC/RISCV/rvv/add.s:324
+vwcvt.x.x.v v8, v4
+# CHECK-INST: vwadd.vx v8, v4
+# CHECK-ENCODING: [0x57,0x64,0x40,0xc6]
----------------
Should this be `# CHECK-INST:  vwadd.vx v8, v4, zero`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69987