[PATCH] D116215: [RISCV] Refactor immediate comparison instructions patterns

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td:4527
+defm : VPatCompare_VI<"int_riscv_vmsltu", "PseudoVMSNE">;
+// Special cases to avoid matching vmsltu.vi 0 (always false) to
+// vmsleu.vi -1 (always true). Instead match to vmsne.vv.
----------------
jrtc27 wrote:
> Isn't it a bit dodgy for this to be done in the first place; shouldn't we be stopping the normal int_riscv_vmsltu pattern from matching that at all? Currently this relies in SelectionDAG picking the more specific pattern, but that should solely be as a heuristic for code quality, not correctness.
I think we did this to match the assembler where we alias vmsltu.vi and vmsgeu.vi and with 0 immediate to vmsne and vmseq.

For vmsleu.vi I suppose we could block the 0 immediate from matching in isel and let vmsleu.vx with X0 match.

That wouldn't work for vmsgeu.vi though because there is no vmsgeu.vx instruction. We use a multiple instruction sequence. But since we know this is always true we wouldn't want to use a multiple instructions. Using vmset.m(for unmasked) and vmand.mm(for masked) would be better and avoid a false dependency I guess.

But I think any change here should not be part of this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116215