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

[Hsiangkai Wang via Phabricator via llvm-commits]

HsiangKai 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.");
----------------
rogfer01 wrote:
> 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?
> 
> 
I added a new register class for it and added test cases.


================
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>;
----------------
rogfer01 wrote:
> 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.
> ```
If we have no such alias. How do we match `vmsge{v}.vx v0, v1, x10`? It may match to `PseudoVMSGE_VX_M` with NoRegister mask. However, the register class for the destination operand is VRNoV0 now. There is no way to match it under the VRNoV0 register class restriction.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84732