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

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td:3726
+
+multiclass VPatCompareSpecil<string intrinsic, string inst> {
+  foreach vti = AllIntegerVectors in {
----------------
"Specil"? Assuming that's a typo for "Special" that's hardly a descriptive name


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td:4526
+defm : VPatCompare_VI<"int_riscv_vmslt", "PseudoVMSLE">;
+defm : VPatCompare_VI<"int_riscv_vmsltu", "PseudoVMSNE">;
+// Special cases to avoid matching vmsltu.vi 0 (always false) to
----------------
This should be mapping to PseudoVMSLEU


================
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.
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116215