[PATCH] D84732: [RISCV] Support vmsge.vx and vmsgeu.vx pseudo instructions in RVV.

[Roger Ferrer Ibanez via Phabricator via llvm-commits]

rogfer01 added inline comments.


================
Comment at: llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp:233
+    //  expansion: vmslt{u}.vx vd, va, x, v0.t; vmxor.mm vd, vd, v0
+    assert(MI.getOperand(0).getReg() != RISCV::V0 &&
+           "The destination register should not be V0.");
----------------
This assertion can be triggered by assembling the following instruction using `llvm-mc --triple riscv64 -mattr +experimental-v --filetype=obj`

```
vmsge.vx v0, v1, x10, v0.t
```

perhaps we could consider having a specific operand `VRegOpNoV0` (or similar) which rejects `v0` here?




================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:558
+
+def : InstAlias<"vmsgeu.vx $vd, $va, $rs1",
+                (PseudoVMSGEU_VX VRegOp:$vd, VRegOp:$va, GPR:$rs1), 0>;
----------------
I was confused by the fact that this seems an unnecessary inst alias. However if we remove it, then `vmsgeu.vx $vd, $va, $rs1` gets parsed as `PseudoVMSGEU_VX_M` but without a mask (according to `llvm-mc --show-inst`)

```
# <MCInst #253 PseudoVMSGEU_VX_M
#  <MCOperand Reg:5>
#  <MCOperand Reg:6>
#  <MCOperand Reg:45>
#  <MCOperand Reg:0>>
```

however the pseudo `PseudoVMSGEU_VX` is explicitly unmasked.

I suggest to add a comment like this

```
// This apparently unnecessary alias prevents matching `vmsgeu.vx vd, vsrc2, rsrc1`
// as if it were an umasked  (i.e. $vm = RISCV::NoRegister) PseudoVMSGEU_VX_M.
```


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoV.td:560
+                (PseudoVMSGEU_VX VRegOp:$vd, VRegOp:$va, GPR:$rs1), 0>;
+def : InstAlias<"vmsge.vx $vd, $va, $rs1",
+                (PseudoVMSGE_VX VRegOp:$vd, VRegOp:$va, GPR:$rs1), 0>;
----------------
And a comment here, something like

```
See comment above. This avoids matching an unmasked `PseudoVMSGE_VX_M`
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84732