[PATCH] D142102: [RISCV] Combine FP_TO_INT to vfwcvt/fvncvt

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.cpp:11651
+    // =========================================================================
+
   case RISCV::PseudoVFCVT_RM_X_F_V_M1_MASK:
----------------
luke wrote:
> This is growing really unwieldy as it needs to handle the matrix of `{X_F,XU_F,F_X,F_XU} x {M1,M2,M4,M8,MF2,MF4} x {vfcvt, vfwcvt, vfncvt}`. Could this tablegen'ed away in another patch?
Not through any existing tablegen mechanism. This is running on MachineIR after isel. We could apply some preprocessor macros to unroll the LMUL.


================
Comment at: llvm/lib/Target/RISCV/RISCVISelLowering.h:240
   VFCVT_RTZ_XU_F_VL,
-  VFCVT_X_F_VL,
+  VFCVT_RTZ_X_F_VL,
   VFCVT_XU_F_VL,
----------------
Why changing the order? This puts in a different order than the list in RISCVTargetLowering::getTargetNodeName


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfoVVLPatterns.td:883
+
+multiclass VPatNConvertFP2IVL_W<SDNode vop, string instruction_name> {
+  // Reuse the same list of types used in the widening nodes, but just swap the
----------------
I'd prefer we didn't rename things in this patch. It makes the diff bigger and makes the view in phabricator more confusing. If you want to rename, do it as a separate pre-patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142102