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

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

fpallares added a comment.

Thanks for updating the patch to the stable v0.8 release (previously version 0.8-draft-20191213). I've added some inline comments.

In addition, we are also missing the whole register move instructions `vmv<nf>r.v` (for `nf` = [1,2,4,8]), introduced in version 0.8-draft-20191117.



================
Comment at: llvm/lib/Target/RISCV/RISCV.td:56
+def HasStdExtV : Predicate<"Subtarget->hasStdExtV()">,
+                           AssemblerPredicate<"FeatureStdExtV">;
+
----------------
Since [[ https://reviews.llvm.org/D69899 | D69899 ]] was commited, we should add the feature name in the `AssemblerPredicate` parameter and update the missing feature error messages in the tests. Like this:
```
def HasStdExtV : Predicate<"Subtarget->hasStdExtV()">,                          
                           AssemblerPredicate<"FeatureStdExtV",                 
                           "'V' (Vector Instructions)">;                        

```


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:419
+  def VS#nf#R_V : VWholeStore<!add(nf, -1), "vs" # nf # "r.v">;
+}
+
----------------
As of the stable v0.8 release, the whole register vector loads and stores have been restricted to a single register only. You probably just forgot to change this `foreach` since `vl1r.v` and `vs1r.v` are the only instructions being tested.

After this change you may want to revisit whether the `VWholeLoad` and `VWholeStore` multiclasses are still needed.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:510
+// Vector Integer Merge Instructions
+defm VMERGE : VALUm_IV_V_X_I<"vmerge", 0b010111>;
+
----------------
I wonder if this should be `VMERGE_V`, so that the defined records are `VMERGE_VVM`, `VMERGE_VXM`, `VMERGE_VIM`.

Currently these records are defined `VMERGE_VM`, `VMERGE_XM` and `VMERGE_IM`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69987