[RISCV] Merge FMV_H_X_RV32/FMV_H_X_RV64 into a single opcode. Same with FMV_X_ANYEXTH_RV32/RV64

[craig.topper]

Summary

Rather than having a different opcode for RV32 and RV64. Let's just say the integer type is XLenVT and use a single opcode for both modes.

[frasercrmck]

Makes sense to me otherwise but I'll defer my LGTM to someone else.

llvm/lib/Target/RISCV/RISCVISelLowering.h

56–57

nit: the comments for these notes are still third-person plural despite each being a single node: match and are; I think matches and is is better.

[craig.topper]

Update comments.

[jrtc27]

llvm/lib/Target/RISCV/RISCVInstrInfoZfh.td

344

Hm, looking at the other files this really should have had a blank line above it, which is more important now you're adding a new set of patterns to the conversion operations "section".

348

This could do with a comment; F has // Moves (no conversion) for its bitcasts.

[craig.topper]

Address @jrtc27 's comments

[jrtc27]

Ah, I hadn't realised the F comment got lost in 741b04b0b7912611a8a5b7e74462e87b8930a116, I was looking at our fork which is currently still based on 11. This looks good to me now.

[craig.topper]

In D92538#2431700, @jrtc27 wrote:

Ah, I hadn't realised the F comment got lost in 741b04b0b7912611a8a5b7e74462e87b8930a116, I was looking at our fork which is currently still based on 11. This looks good to me now.

I'm not sure why I dropped it unless I was basing it on the lack of comment for the RV64 patterns.

llvm/lib/Target/RISCV/RISCVISelLowering.h

enum NodeType : unsigned {
   RORW,
   // RV64IB funnel shifts, with the semantics of the named RISC-V instructions,
   // but the same operand order as fshl/fshr intrinsics.
   FSRW,
   FSLW,
   // FPR<->GPR transfer operations when the FPR is smaller than XLEN, needed as
   // XLEN is the only legal integer width.
   //
-  // FMV_H_X_RV32/RV64 match the semantics of the FMV.H.X.
-  // FMV_X_ANYEXTH_RV32/RV64 are similar to FMV.X.H but has an any-extended
-  // result.
+  // FMV_H_X matches the semantics of the FMV.H.X.
+  // FMV_X_ANYEXTH is similar to FMV.X.H but has an any-extended result.
   // FMV_W_X_RV64 matches the semantics of the FMV.W.X.
   // FMV_X_ANYEXTW_RV64 is similar to FMV.X.W but has an any-extended result.
   //
   // This is a more convenient semantic for producing dagcombines that remove
   // unnecessary GPR->FPR->GPR moves.
-  FMV_H_X_RV32,
-  FMV_H_X_RV64,
-  FMV_X_ANYEXTH_RV32,
-  FMV_X_ANYEXTH_RV64,
+  FMV_H_X,
+  FMV_X_ANYEXTH,
   FMV_W_X_RV64,
   FMV_X_ANYEXTW_RV64,
   // READ_CYCLE_WIDE - A read of the 64-bit cycle CSR on a 32-bit target
   // (returns (Lo, Hi)). It takes a chain operand.
   READ_CYCLE_WIDE,
   // Generalized Reverse and Generalized Or-Combine - directly matching the
   // semantics of the named RISC-V instructions. Lowered as custom nodes as
   // TableGen chokes when faced with commutative permutations in deeply-nested

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

case ISD::BITCAST: {
     assert(((Subtarget.is64Bit() && Subtarget.hasStdExtF()) ||
             Subtarget.hasStdExtZfh()) &&
            "Unexpected custom legalisation");
     SDLoc DL(Op);
     SDValue Op0 = Op.getOperand(0);
     if (Op.getValueType() == MVT::f16 && Subtarget.hasStdExtZfh()) {
       if (Op0.getValueType() != MVT::i16)
         return SDValue();
-      unsigned Opcode = RISCVISD::FMV_H_X_RV32;
-      EVT ExtType = MVT::i32;
-      if (Subtarget.is64Bit()) {
-        Opcode = RISCVISD::FMV_H_X_RV64;
-        ExtType = MVT::i64;
-      }
-      SDValue NewOp0 = DAG.getNode(ISD::ANY_EXTEND, DL, ExtType, Op0);
-      SDValue FPConv = DAG.getNode(Opcode, DL, MVT::f16, NewOp0);
+      SDValue NewOp0 =
+          DAG.getNode(ISD::ANY_EXTEND, DL, Subtarget.getXLenVT(), Op0);
+      SDValue FPConv = DAG.getNode(RISCVISD::FMV_H_X, DL, MVT::f16, NewOp0);
       return FPConv;
     } else if (Op.getValueType() == MVT::f32 && Subtarget.is64Bit() &&
                Subtarget.hasStdExtF()) {
       if (Op0.getValueType() != MVT::i32)
         return SDValue();
       SDValue NewOp0 = DAG.getNode(ISD::ANY_EXTEND, DL, MVT::i64, Op0);
       SDValue FPConv =
           DAG.getNode(RISCVISD::FMV_W_X_RV64, DL, MVT::f32, NewOp0);
case ISD::BITCAST: {
     assert(((N->getValueType(0) == MVT::i32 && Subtarget.is64Bit() &&
              Subtarget.hasStdExtF()) ||
             (N->getValueType(0) == MVT::i16 && Subtarget.hasStdExtZfh())) &&
            "Unexpected custom legalisation");
     SDValue Op0 = N->getOperand(0);
     if (N->getValueType(0) == MVT::i16 && Subtarget.hasStdExtZfh()) {
       if (Op0.getValueType() != MVT::f16)
         return;
-      unsigned Opcode = RISCVISD::FMV_X_ANYEXTH_RV32;
-      EVT ExtType = MVT::i32;
-      if (Subtarget.is64Bit()) {
-        Opcode = RISCVISD::FMV_X_ANYEXTH_RV64;
-        ExtType = MVT::i64;
-      }
-      SDValue FPConv = DAG.getNode(Opcode, DL, ExtType, Op0);
+      SDValue FPConv =
+          DAG.getNode(RISCVISD::FMV_X_ANYEXTH, DL, Subtarget.getXLenVT(), Op0);
       Results.push_back(DAG.getNode(ISD::TRUNCATE, DL, MVT::i16, FPConv));
     } else if (N->getValueType(0) == MVT::i32 && Subtarget.is64Bit() &&
                Subtarget.hasStdExtF()) {
       if (Op0.getValueType() != MVT::f32)
         return;
       SDValue FPConv =
           DAG.getNode(RISCVISD::FMV_X_ANYEXTW_RV64, DL, MVT::i64, Op0);
       Results.push_back(DAG.getNode(ISD::TRUNCATE, DL, MVT::i32, FPConv));

 static SDValue convertLocVTToValVT(SelectionDAG &DAG, SDValue Val,
                                    const CCValAssign &VA, const SDLoc &DL) {
   switch (VA.getLocInfo()) {
   default:
     llvm_unreachable("Unexpected CCValAssign::LocInfo");
   case CCValAssign::Full:
     break;
   case CCValAssign::BCvt:
-    if (VA.getLocVT() == MVT::i32 && VA.getValVT() == MVT::f16)
-      Val = DAG.getNode(RISCVISD::FMV_H_X_RV32, DL, MVT::f16, Val);
-    else if (VA.getLocVT() == MVT::i64 && VA.getValVT() == MVT::f16)
-      Val = DAG.getNode(RISCVISD::FMV_H_X_RV64, DL, MVT::f16, Val);
+    if (VA.getLocVT().isInteger() && VA.getValVT() == MVT::f16)
+      Val = DAG.getNode(RISCVISD::FMV_H_X, DL, MVT::f16, Val);
     else if (VA.getLocVT() == MVT::i64 && VA.getValVT() == MVT::f32)
       Val = DAG.getNode(RISCVISD::FMV_W_X_RV64, DL, MVT::f32, Val);
     else
       Val = DAG.getNode(ISD::BITCAST, DL, VA.getValVT(), Val);
     break;
   }
   return Val;
 }
static SDValue convertValVTToLocVT(SelectionDAG &DAG, SDValue Val,
   EVT LocVT = VA.getLocVT();
 
   switch (VA.getLocInfo()) {
   default:
     llvm_unreachable("Unexpected CCValAssign::LocInfo");
   case CCValAssign::Full:
     break;
   case CCValAssign::BCvt:
-    if (VA.getLocVT() == MVT::i32 && VA.getValVT() == MVT::f16)
-      Val = DAG.getNode(RISCVISD::FMV_X_ANYEXTH_RV32, DL, MVT::i32, Val);
-    else if (VA.getLocVT() == MVT::i64 && VA.getValVT() == MVT::f16)
-      Val = DAG.getNode(RISCVISD::FMV_X_ANYEXTH_RV64, DL, MVT::i64, Val);
+    if (VA.getLocVT().isInteger() && VA.getValVT() == MVT::f16)
+      Val = DAG.getNode(RISCVISD::FMV_X_ANYEXTH, DL, VA.getLocVT(), Val);
     else if (VA.getLocVT() == MVT::i64 && VA.getValVT() == MVT::f32)
       Val = DAG.getNode(RISCVISD::FMV_X_ANYEXTW_RV64, DL, MVT::i64, Val);
     else
       Val = DAG.getNode(ISD::BITCAST, DL, LocVT, Val);
     break;
   }
   return Val;
 }
#define NODE_NAME_CASE(NODE)                                                   \
   NODE_NAME_CASE(SRLW)
   NODE_NAME_CASE(DIVW)
   NODE_NAME_CASE(DIVUW)
   NODE_NAME_CASE(REMUW)
   NODE_NAME_CASE(ROLW)
   NODE_NAME_CASE(RORW)
   NODE_NAME_CASE(FSLW)
   NODE_NAME_CASE(FSRW)
-  NODE_NAME_CASE(FMV_H_X_RV32)
-  NODE_NAME_CASE(FMV_H_X_RV64)
-  NODE_NAME_CASE(FMV_X_ANYEXTH_RV32)
-  NODE_NAME_CASE(FMV_X_ANYEXTH_RV64)
+  NODE_NAME_CASE(FMV_H_X)
+  NODE_NAME_CASE(FMV_X_ANYEXTH)
   NODE_NAME_CASE(FMV_W_X_RV64)
   NODE_NAME_CASE(FMV_X_ANYEXTW_RV64)
   NODE_NAME_CASE(READ_CYCLE_WIDE)
   NODE_NAME_CASE(GREVI)
   NODE_NAME_CASE(GREVIW)
   NODE_NAME_CASE(GORCI)
   NODE_NAME_CASE(GORCIW)
   }

llvm/lib/Target/RISCV/RISCVInstrInfoZfh.td

 // ratified yet.
 //
 //===----------------------------------------------------------------------===//
 
 //===----------------------------------------------------------------------===//
 // RISC-V specific DAG Nodes.
 //===----------------------------------------------------------------------===//
 
-def SDT_RISCVFMV_H_X_RV32
-    : SDTypeProfile<1, 1, [SDTCisVT<0, f16>, SDTCisVT<1, i32>]>;
-def SDT_RISCVFMV_H_X_RV64
-    : SDTypeProfile<1, 1, [SDTCisVT<0, f16>, SDTCisVT<1, i64>]>;
-def SDT_RISCVFMV_X_ANYEXTH_RV64
-    : SDTypeProfile<1, 1, [SDTCisVT<0, i64>, SDTCisVT<1, f16>]>;
-def SDT_RISCVFMV_X_ANYEXTH_RV32
-    : SDTypeProfile<1, 1, [SDTCisVT<0, i32>, SDTCisVT<1, f16>]>;
-
-def riscv_fmv_h_x_rv32
-    : SDNode<"RISCVISD::FMV_H_X_RV32", SDT_RISCVFMV_H_X_RV32>;
-def riscv_fmv_h_x_rv64
-    : SDNode<"RISCVISD::FMV_H_X_RV64", SDT_RISCVFMV_H_X_RV64>;
-def riscv_fmv_x_anyexth_rv64
-    : SDNode<"RISCVISD::FMV_X_ANYEXTH_RV64", SDT_RISCVFMV_X_ANYEXTH_RV64>;
-def riscv_fmv_x_anyexth_rv32
-    : SDNode<"RISCVISD::FMV_X_ANYEXTH_RV32", SDT_RISCVFMV_X_ANYEXTH_RV32>;
+def SDT_RISCVFMV_H_X
+    : SDTypeProfile<1, 1, [SDTCisVT<0, f16>, SDTCisVT<1, XLenVT>]>;
+def SDT_RISCVFMV_X_ANYEXTH
+    : SDTypeProfile<1, 1, [SDTCisVT<0, XLenVT>, SDTCisVT<1, f16>]>;
+
+def riscv_fmv_h_x
+    : SDNode<"RISCVISD::FMV_H_X", SDT_RISCVFMV_H_X>;
+def riscv_fmv_x_anyexth
+    : SDNode<"RISCVISD::FMV_X_ANYEXTH", SDT_RISCVFMV_X_ANYEXTH>;
 
 //===----------------------------------------------------------------------===//
 // Instruction class templates
 //===----------------------------------------------------------------------===//
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0 in
 class FPFMAH_rrr_frm<RISCVOpcode opcode, string opcodestr>
     : RVInstR4<0b10, opcode, (outs FPR16:$rd),

 
 defm : LdPat<load, FLH>;
 
 /// Stores
 
 defm : StPat<store, FSH, FPR16>;
 
 /// Float conversion operations
 // f32 -> f16, f16 -> f32
 def : Pat<(fpround FPR32:$rs1), (FCVT_H_S FPR32:$rs1, 0b111)>;
 def : Pat<(fpextend FPR16:$rs1), (FCVT_S_H FPR16:$rs1)>;
 
+def : Pat<(riscv_fmv_h_x GPR:$src), (FMV_H_X GPR:$src)>;
+def : Pat<(riscv_fmv_x_anyexth FPR16:$src), (FMV_X_H FPR16:$src)>;
 } // Predicates = [HasStdExtZfh]
 
 let Predicates = [HasStdExtZfh, IsRV32] in {
-def : Pat<(riscv_fmv_h_x_rv32 GPR:$src), (FMV_H_X GPR:$src)>;
-def : Pat<(riscv_fmv_x_anyexth_rv32 FPR16:$src), (FMV_X_H FPR16:$src)>;
-
 // float->[u]int. Round-to-zero must be used.
 def : Pat<(fp_to_sint FPR16:$rs1), (FCVT_W_H $rs1, 0b001)>;
 def : Pat<(fp_to_uint FPR16:$rs1), (FCVT_WU_H $rs1, 0b001)>;
 
 // [u]int->float. Match GCC and default to using dynamic rounding mode.
 def : Pat<(sint_to_fp GPR:$rs1), (FCVT_H_W $rs1, 0b111)>;
 def : Pat<(uint_to_fp GPR:$rs1), (FCVT_H_WU $rs1, 0b111)>;
 } // Predicates = [HasStdExtZfh, IsRV32]
 
 let Predicates = [HasStdExtZfh, IsRV64] in {
-def : Pat<(riscv_fmv_h_x_rv64 GPR:$src), (FMV_H_X GPR:$src)>;
-def : Pat<(riscv_fmv_x_anyexth_rv64 FPR16:$src), (FMV_X_H FPR16:$src)>;
-
 // FP->[u]int32 is mostly handled by the FP->[u]int64 patterns. This is safe
 // because fpto[u|s]i produces poison if the value can't fit into the target.
 // We match the single case below because fcvt.wu.s sign-extends its result so
 // is cheaper than fcvt.lu.h+sext.w.
 def : Pat<(sext_inreg (assertzexti32 (fp_to_uint FPR16:$rs1)), i32),
           (FCVT_WU_H $rs1, 0b001)>;
 
 // FP->[u]int64