[RISCV] Block vmsgeu.vi with 0 immediate in Isel

[Chenbing.Zheng]

Summary

For vmsgeu.vi with 0, we know this is always true,
so we can instead it with vmset.m (unmasked) or
vmset.m+vmand.mm (masked).

[Chenbing.Zheng]

1. clang-format
2. modify vmandn to vmand

[Chenbing.Zheng]

Fix vmset generating error

[craig.topper]

Shouldn't you remove VPatCompareUnsignedZero from RISCVInstrInfoVPseudos.td?

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

817

This could be done by using a new version of simm5_plus1 that excludes 0 in VPatCompare_VI for the vmsltu case.

863

Same here.

[Chenbing.Zheng]

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

817

New simm5_plus1 excludes 0, and I Keep pattern "defm : VPatCompareUnsignedZero<"int_riscv_vmsltu", "PseudoVMSNE">" to match vmsltu 0，and del this two case. Understanding right ?

[craig.topper]

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

817

Remove the VPatCOmpareUnisignedZero. The vmsltu.vx will then be able to match. The 0 will select to X0.

[Chenbing.Zheng]

delete VPatCompareUnsignedZero

[Chenbing.Zheng]

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

817

I try it, but vmsleu.vi -1 still generate.
VPattern of vmsltu.vx is different from vmsltu.vi , it cause vmsltu.vx canot be match ?
So I keep these two cases, and delate VPatCompareUnsignedZero in my newest patch.

[craig.topper]

I posted an alternative version that handles vsltu.vi without RISCVISelDAGToDAG.cpp changes here https://reviews.llvm.org/D116723

[Chenbing.Zheng]

In D116584#3224373, @craig.topper wrote:

I posted an alternative version that handles vsltu.vi without RISCVISelDAGToDAG.cpp changes here https://reviews.llvm.org/D116723

I read your patch, it is a good solution. I will rebase after for your patch merge into the main

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

817

This could be done by using a new version of simm5_plus1 that excludes 0 in VPatCompare_VI for the vmsltu case.

simm5_plus1 newest code in branch main. I thought is it define range -15 ~ 16 ? havn't excludes 0? or there are some new patch havn't push to main?

def simm5_plus1 : Operand<XLenVT>, ImmLeaf<XLenVT,

[{return (isInt<5>(Imm) && Imm != -16) || Imm == 16;}]> {
let ParserMatchClass = SImm5Plus1AsmOperand;
let MCOperandPredicate = [{
  int64_t Imm;
  if (MCOp.evaluateAsConstantImm(Imm))
    return (isInt<5>(Imm) && Imm != -16) || Imm == 16;
  return MCOp.isBareSymbolRef();
}];

}

[Chenbing.Zheng]

rebase https://reviews.llvm.org/D116723

[craig.topper]

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

840–841

This code was recently changed to use a macro for each case since they are only vary by instruction suffix. Please rebase.

[Chenbing.Zheng]

rebase main

[Chenbing.Zheng]

clang-format

[craig.topper]

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

830

if (!IsUnsigned || CVal != 0)
  break;
IsCmpUnsignedZero = true;

890

Same

[Chenbing.Zheng]

Refactor some code

[Chenbing.Zheng]

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

830

done

[Chenbing.Zheng]

clean code

[craig.topper]

LGTM

llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp

case ISD::MUL: {
     return;
   }
   case ISD::INTRINSIC_WO_CHAIN: {
     unsigned IntNo = Node->getConstantOperandVal(0);
     switch (IntNo) {
       // By default we do not custom select any intrinsic.
     default:
       break;
     case Intrinsic::riscv_vmsgeu:
     case Intrinsic::riscv_vmsge: {
       SDValue Src1 = Node->getOperand(1);
       SDValue Src2 = Node->getOperand(2);
+      bool IsUnsigned = IntNo == Intrinsic::riscv_vmsgeu;
+      bool IsCmpUnsignedZero = false;
       // Only custom select scalar second operand.
       if (Src2.getValueType() != XLenVT)
         break;
       // Small constants are handled with patterns.
       if (auto *C = dyn_cast<ConstantSDNode>(Src2)) {
         int64_t CVal = C->getSExtValue();
-        if (CVal >= -15 && CVal <= 16)
+        if (CVal >= -15 && CVal <= 16) {
+          if (IsUnsigned && CVal == 0)
+            IsCmpUnsignedZero = true;
+          else
             break;
         }
-      bool IsUnsigned = IntNo == Intrinsic::riscv_vmsgeu;
+      }
       MVT Src1VT = Src1.getSimpleValueType();
-      unsigned VMSLTOpcode, VMNANDOpcode;
+      unsigned VMSLTOpcode, VMNANDOpcode, VMSetOpcode;
       switch (RISCVTargetLowering::getLMUL(Src1VT)) {
       default:
         llvm_unreachable("Unexpected LMUL!");
-#define CASE_VMSLT_VMNAND_OPCODES(lmulenum, suffix) \
-      case RISCVII::VLMUL::lmulenum: \
-        VMSLTOpcode = IsUnsigned ? RISCV::PseudoVMSLTU_VX_##suffix \
-                                 : RISCV::PseudoVMSLT_VX_##suffix; \
-        VMNANDOpcode = RISCV::PseudoVMNAND_MM_##suffix; \
-        break;
-      CASE_VMSLT_VMNAND_OPCODES(LMUL_F8, MF8)
-      CASE_VMSLT_VMNAND_OPCODES(LMUL_F4, MF4)
-      CASE_VMSLT_VMNAND_OPCODES(LMUL_F2, MF2)
-      CASE_VMSLT_VMNAND_OPCODES(LMUL_1, M1)
-      CASE_VMSLT_VMNAND_OPCODES(LMUL_2, M2)
-      CASE_VMSLT_VMNAND_OPCODES(LMUL_4, M4)
-      CASE_VMSLT_VMNAND_OPCODES(LMUL_8, M8)
-#undef CASE_VMSLT_VMNAND_OPCODES
+#define CASE_VMSLT_VMNAND_VMSET_OPCODES(lmulenum, suffix, suffix_b)            \
+  case RISCVII::VLMUL::lmulenum:                                               \
+    VMSLTOpcode = IsUnsigned ? RISCV::PseudoVMSLTU_VX_##suffix                 \
+                             : RISCV::PseudoVMSLT_VX_##suffix;                 \
+    VMNANDOpcode = RISCV::PseudoVMNAND_MM_##suffix;                            \
+    VMSetOpcode = RISCV::PseudoVMSET_M_##suffix_b;                             \
+        CASE_VMSLT_VMNAND_VMSET_OPCODES(LMUL_F8, MF8, B1)
+        CASE_VMSLT_VMNAND_VMSET_OPCODES(LMUL_F4, MF4, B2)
+        CASE_VMSLT_VMNAND_VMSET_OPCODES(LMUL_F2, MF2, B4)
+        CASE_VMSLT_VMNAND_VMSET_OPCODES(LMUL_1, M1, B8)
+        CASE_VMSLT_VMNAND_VMSET_OPCODES(LMUL_2, M2, B16)
+        CASE_VMSLT_VMNAND_VMSET_OPCODES(LMUL_4, M4, B32)
+        CASE_VMSLT_VMNAND_VMSET_OPCODES(LMUL_8, M8, B64)
+#undef CASE_VMSLT_VMNAND_VMSET_OPCODES
       }
       SDValue SEW = CurDAG->getTargetConstant(
           Log2_32(Src1VT.getScalarSizeInBits()), DL, XLenVT);
       SDValue VL;
       selectVLOp(Node->getOperand(3), VL);
 
+      // If vmsgeu with 0 immediate, expand it to vmset.
+      if (IsCmpUnsignedZero) {
+        ReplaceNode(Node, CurDAG->getMachineNode(VMSetOpcode, DL, VT, VL, SEW));
+        return;
+      }
+
       // Expand to
       // vmslt{u}.vx vd, va, x; vmnand.mm vd, vd, vd
       SDValue Cmp = SDValue(
           CurDAG->getMachineNode(VMSLTOpcode, DL, VT, {Src1, Src2, VL, SEW}),
           0);
       ReplaceNode(Node, CurDAG->getMachineNode(VMNANDOpcode, DL, VT,
                                                {Cmp, Cmp, VL, SEW}));
       return;
     }
     case Intrinsic::riscv_vmsgeu_mask:
     case Intrinsic::riscv_vmsge_mask: {
       SDValue Src1 = Node->getOperand(2);
       SDValue Src2 = Node->getOperand(3);
+      bool IsUnsigned = IntNo == Intrinsic::riscv_vmsgeu_mask;
+      bool IsCmpUnsignedZero = false;
       // Only custom select scalar second operand.
       if (Src2.getValueType() != XLenVT)
         break;
       // Small constants are handled with patterns.
       if (auto *C = dyn_cast<ConstantSDNode>(Src2)) {
         int64_t CVal = C->getSExtValue();
-        if (CVal >= -15 && CVal <= 16)
+        if (CVal >= -15 && CVal <= 16) {
+          if (IsUnsigned && CVal == 0)
+            IsCmpUnsignedZero = true;
+          else
             break;
         }
-      bool IsUnsigned = IntNo == Intrinsic::riscv_vmsgeu_mask;
+      }
       MVT Src1VT = Src1.getSimpleValueType();
-      unsigned VMSLTOpcode, VMSLTMaskOpcode, VMXOROpcode, VMANDNOpcode;
+      unsigned VMSLTOpcode, VMSLTMaskOpcode, VMXOROpcode, VMANDNOpcode,
+          VMSetOpcode, VMANDOpcode;
       switch (RISCVTargetLowering::getLMUL(Src1VT)) {
       default:
         llvm_unreachable("Unexpected LMUL!");
-#define CASE_VMSLT_OPCODES(lmulenum, suffix) \
-      case RISCVII::VLMUL::lmulenum: \
-        VMSLTOpcode = IsUnsigned ? RISCV::PseudoVMSLTU_VX_##suffix \
-                                 : RISCV::PseudoVMSLT_VX_##suffix; \
-        VMSLTMaskOpcode = IsUnsigned ? RISCV::PseudoVMSLTU_VX_##suffix##_MASK \
-                                     : RISCV::PseudoVMSLT_VX_##suffix##_MASK; \
-        break;
-      CASE_VMSLT_OPCODES(LMUL_F8, MF8)
-      CASE_VMSLT_OPCODES(LMUL_F4, MF4)
-      CASE_VMSLT_OPCODES(LMUL_F2, MF2)
-      CASE_VMSLT_OPCODES(LMUL_1, M1)
-      CASE_VMSLT_OPCODES(LMUL_2, M2)
-      CASE_VMSLT_OPCODES(LMUL_4, M4)
-      CASE_VMSLT_OPCODES(LMUL_8, M8)
-#undef CASE_VMSLT_OPCODES
+#define CASE_VMSLT_VMSET_OPCODES(lmulenum, suffix, suffix_b)                   \
+  case RISCVII::VLMUL::lmulenum:                                               \
+    VMSLTOpcode = IsUnsigned ? RISCV::PseudoVMSLTU_VX_##suffix                 \
+                             : RISCV::PseudoVMSLT_VX_##suffix;                 \
+    VMSLTMaskOpcode = IsUnsigned ? RISCV::PseudoVMSLTU_VX_##suffix##_MASK      \
+                                 : RISCV::PseudoVMSLT_VX_##suffix##_MASK;      \
+    VMSetOpcode = RISCV::PseudoVMSET_M_##suffix_b;                             \
+        CASE_VMSLT_VMSET_OPCODES(LMUL_F8, MF8, B1)
+        CASE_VMSLT_VMSET_OPCODES(LMUL_F4, MF4, B2)
+        CASE_VMSLT_VMSET_OPCODES(LMUL_F2, MF2, B4)
+        CASE_VMSLT_VMSET_OPCODES(LMUL_1, M1, B8)
+        CASE_VMSLT_VMSET_OPCODES(LMUL_2, M2, B16)
+        CASE_VMSLT_VMSET_OPCODES(LMUL_4, M4, B32)
+        CASE_VMSLT_VMSET_OPCODES(LMUL_8, M8, B64)
+#undef CASE_VMSLT_VMSET_OPCODES
       }
       // Mask operations use the LMUL from the mask type.
       switch (RISCVTargetLowering::getLMUL(VT)) {
       default:
         llvm_unreachable("Unexpected LMUL!");
-#define CASE_VMXOR_VANDN_OPCODES(lmulenum, suffix) \
-      case RISCVII::VLMUL::lmulenum: \
-        VMXOROpcode = RISCV::PseudoVMXOR_MM_##suffix; \
-        VMANDNOpcode = RISCV::PseudoVMANDN_MM_##suffix; \
-        break;
-      CASE_VMXOR_VANDN_OPCODES(LMUL_F8, MF8)
-      CASE_VMXOR_VANDN_OPCODES(LMUL_F4, MF4)
-      CASE_VMXOR_VANDN_OPCODES(LMUL_F2, MF2)
-      CASE_VMXOR_VANDN_OPCODES(LMUL_1, M1)
-      CASE_VMXOR_VANDN_OPCODES(LMUL_2, M2)
-      CASE_VMXOR_VANDN_OPCODES(LMUL_4, M4)
-      CASE_VMXOR_VANDN_OPCODES(LMUL_8, M8)
-#undef CASE_VMXOR_VANDN_OPCODES
+#define CASE_VMXOR_VANDN_VMSET_OPCODES(lmulenum, suffix)                       \
+  case RISCVII::VLMUL::lmulenum:                                               \
+    VMXOROpcode = RISCV::PseudoVMXOR_MM_##suffix;                              \
+    VMANDNOpcode = RISCV::PseudoVMANDN_MM_##suffix;                            \
+    VMANDOpcode = RISCV::PseudoVMAND_MM_##suffix;                              \
+        CASE_VMXOR_VANDN_VMSET_OPCODES(LMUL_F8, MF8)
+        CASE_VMXOR_VANDN_VMSET_OPCODES(LMUL_F4, MF4)
+        CASE_VMXOR_VANDN_VMSET_OPCODES(LMUL_F2, MF2)
+        CASE_VMXOR_VANDN_VMSET_OPCODES(LMUL_1, M1)
+        CASE_VMXOR_VANDN_VMSET_OPCODES(LMUL_2, M2)
+        CASE_VMXOR_VANDN_VMSET_OPCODES(LMUL_4, M4)
+        CASE_VMXOR_VANDN_VMSET_OPCODES(LMUL_8, M8)
+#undef CASE_VMXOR_VANDN_VMSET_OPCODES
       }
       SDValue SEW = CurDAG->getTargetConstant(
           Log2_32(Src1VT.getScalarSizeInBits()), DL, XLenVT);
       SDValue MaskSEW = CurDAG->getTargetConstant(0, DL, XLenVT);
       SDValue VL;
       selectVLOp(Node->getOperand(5), VL);
       SDValue MaskedOff = Node->getOperand(1);
       SDValue Mask = Node->getOperand(4);
+
+      // If vmsgeu_mask with 0 immediate, expand it to {vmset, vmand}.
+      if (IsCmpUnsignedZero) {
+        SDValue VmSet =
+            SDValue(CurDAG->getMachineNode(VMSetOpcode, DL, VT, VL, SEW), 0);
+        ReplaceNode(Node, CurDAG->getMachineNode(VMANDOpcode, DL, VT,
+                                                 {Mask, VmSet, VL, MaskSEW}));
+        return;
+      }
+
       // If the MaskedOff value and the Mask are the same value use
       // vmslt{u}.vx vt, va, x;  vmandn.mm vd, vd, vt
       // This avoids needing to copy v0 to vd before starting the next sequence.
       if (Mask == MaskedOff) {
         SDValue Cmp = SDValue(
             CurDAG->getMachineNode(VMSLTOpcode, DL, VT, {Src1, Src2, VL, SEW}),
             0);
         ReplaceNode(Node, CurDAG->getMachineNode(VMANDNOpcode, DL, VT,

llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td

def : Pat<(vti.Mask (IntrMask (vti.Mask VR:$merge),
                                   (vti.Scalar ImmType:$rs2),
                                   (vti.Mask V0),
                                   VLOpFrag)),
               (PseudoMask VR:$merge, vti.RegClass:$rs1, (DecImm ImmType:$rs2),
                           (vti.Mask V0), GPR:$vl, vti.Log2SEW)>;
   }
 }
 
-multiclass VPatCompareUnsignedZero<string intrinsic, string inst> {
-  foreach vti = AllIntegerVectors in {
-    defvar Intr = !cast<Intrinsic>(intrinsic);
-    defvar Pseudo = !cast<Instruction>(inst#"_VV_"#vti.LMul.MX);
-    def : Pat<(vti.Mask (Intr (vti.Vector vti.RegClass:$rs1),
-                              (vti.Scalar 0), VLOpFrag)),
-              (Pseudo vti.RegClass:$rs1, vti.RegClass:$rs1,
-                      GPR:$vl, vti.Log2SEW)>;
-    defvar IntrMask = !cast<Intrinsic>(intrinsic # "_mask");
-    defvar PseudoMask = !cast<Instruction>(inst#"_VV_"#vti.LMul.MX#"_MASK");
-    def : Pat<(vti.Mask (IntrMask (vti.Mask VR:$merge),
-                                  (vti.Vector vti.RegClass:$rs1),
-                                  (vti.Scalar 0),
-                                  (vti.Mask V0),
-                                  VLOpFrag)),
-              (PseudoMask VR:$merge, vti.RegClass:$rs1, vti.RegClass:$rs1,
-                          (vti.Mask V0), GPR:$vl, vti.Log2SEW)>;
-  }
-}
-
 //===----------------------------------------------------------------------===//
 // Pseudo instructions
 //===----------------------------------------------------------------------===//
 
 let Predicates = [HasVInstructions] in {
 
 //===----------------------------------------------------------------------===//
 // Pseudo Instructions for CodeGen

 // Match vmslt(u).vx intrinsics to vmsle(u).vi if the scalar is -15 to 16. This
 // avoids the user needing to know that there is no vmslt(u).vi instruction.
 // Similar for vmsge(u).vx intrinsics using vmslt(u).vi.
 defm : VPatCompare_VI<"int_riscv_vmslt", "PseudoVMSLE">;
 defm : VPatCompare_VI<"int_riscv_vmsltu", "PseudoVMSLEU", simm5_plus1_nonzero>;
 
 defm : VPatCompare_VI<"int_riscv_vmsge", "PseudoVMSGT">;
 defm : VPatCompare_VI<"int_riscv_vmsgeu", "PseudoVMSGTU", simm5_plus1_nonzero>;
-// Special cases for vmsgeu.vi 0 (always true). Instead match to vmsne.vv.
-// FIXME: We could match this to vmset.m or vmset.m+vmand.mm.
-defm : VPatCompareUnsignedZero<"int_riscv_vmsgeu", "PseudoVMSEQ">;
 
 //===----------------------------------------------------------------------===//
 // 12.9. Vector Integer Min/Max Instructions
 //===----------------------------------------------------------------------===//
 defm : VPatBinaryV_VV_VX<"int_riscv_vminu", "PseudoVMINU", AllIntegerVectors>;
 defm : VPatBinaryV_VV_VX<"int_riscv_vmin", "PseudoVMIN", AllIntegerVectors>;
 defm : VPatBinaryV_VV_VX<"int_riscv_vmaxu", "PseudoVMAXU", AllIntegerVectors>;
 defm : VPatBinaryV_VV_VX<"int_riscv_vmax", "PseudoVMAX", AllIntegerVectors>;

llvm/test/CodeGen/RISCV/rvv/vmsgeu-rv32.ll

%a = call <vscale x 2 x i1> @llvm.riscv.vmsgeu.nxv2i16.i16(
     i32 %1)
 
   ret <vscale x 2 x i1> %a
 }
 
 define <vscale x 2 x i1> @intrinsic_vmsgeu_mask_vi_nxv2i16_i16(<vscale x 2 x i1> %0, <vscale x 2 x i16> %1, <vscale x 2 x i1> %2, i32 %3) nounwind {
 ; CHECK-LABEL: intrinsic_vmsgeu_mask_vi_nxv2i16_i16:
 ; CHECK:       # %bb.0: # %entry
-; CHECK-NEXT:    vmv1r.v v10, v0
 ; CHECK-NEXT:    vsetvli zero, a0, e16, mf2, ta, mu
-; CHECK-NEXT:    vmv1r.v v0, v9
-; CHECK-NEXT:    vmseq.vv v10, v8, v8, v0.t
-; CHECK-NEXT:    vmv1r.v v0, v10
+; CHECK-NEXT:    vmset.m v8
+; CHECK-NEXT:    vmand.mm v0, v9, v8
 ; CHECK-NEXT:    ret
 entry:
   %a = call <vscale x 2 x i1> @llvm.riscv.vmsgeu.mask.nxv2i16.i16(
     <vscale x 2 x i1> %0,
     <vscale x 2 x i16> %1,
     i16 0,
     <vscale x 2 x i1> %2,
     i32 %3)
 
   ret <vscale x 2 x i1> %a
 }
 
 define <vscale x 4 x i1> @intrinsic_vmsgeu_vi_nxv4i16_i16(<vscale x 4 x i16> %0, i32 %1) nounwind {
 ; CHECK-LABEL: intrinsic_vmsgeu_vi_nxv4i16_i16:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    vsetvli zero, a0, e16, m1, ta, mu
-; CHECK-NEXT:    vmseq.vv v0, v8, v8
+; CHECK-NEXT:    vmset.m v0
 ; CHECK-NEXT:    ret
 entry:
   %a = call <vscale x 4 x i1> @llvm.riscv.vmsgeu.nxv4i16.i16(
     <vscale x 4 x i16> %0,
     i16 0,
     i32 %1)
 
   ret <vscale x 4 x i1> %a

llvm/test/CodeGen/RISCV/rvv/vmsgeu-rv64.ll

%a = call <vscale x 2 x i1> @llvm.riscv.vmsgeu.nxv2i16.i16(
     i64 %1)
 
   ret <vscale x 2 x i1> %a
 }
 
 define <vscale x 2 x i1> @intrinsic_vmsgeu_mask_vi_nxv2i16_i16(<vscale x 2 x i1> %0, <vscale x 2 x i16> %1, <vscale x 2 x i1> %2, i64 %3) nounwind {
 ; CHECK-LABEL: intrinsic_vmsgeu_mask_vi_nxv2i16_i16:
 ; CHECK:       # %bb.0: # %entry
-; CHECK-NEXT:    vmv1r.v v10, v0
 ; CHECK-NEXT:    vsetvli zero, a0, e16, mf2, ta, mu
-; CHECK-NEXT:    vmv1r.v v0, v9
-; CHECK-NEXT:    vmseq.vv v10, v8, v8, v0.t
-; CHECK-NEXT:    vmv1r.v v0, v10
+; CHECK-NEXT:    vmset.m v8
+; CHECK-NEXT:    vmand.mm v0, v9, v8
 ; CHECK-NEXT:    ret
 entry:
   %a = call <vscale x 2 x i1> @llvm.riscv.vmsgeu.mask.nxv2i16.i16(
     <vscale x 2 x i1> %0,
     <vscale x 2 x i16> %1,
     i16 0,
     <vscale x 2 x i1> %2,
     i64 %3)
 
   ret <vscale x 2 x i1> %a
 }
 
 define <vscale x 4 x i1> @intrinsic_vmsgeu_vi_nxv4i16_i16(<vscale x 4 x i16> %0, i64 %1) nounwind {
 ; CHECK-LABEL: intrinsic_vmsgeu_vi_nxv4i16_i16:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    vsetvli zero, a0, e16, m1, ta, mu
-; CHECK-NEXT:    vmseq.vv v0, v8, v8
+; CHECK-NEXT:    vmset.m v0
 ; CHECK-NEXT:    ret
 entry:
   %a = call <vscale x 4 x i1> @llvm.riscv.vmsgeu.nxv4i16.i16(
     <vscale x 4 x i16> %0,
     i16 0,
     i64 %1)
 
   ret <vscale x 4 x i1> %a