[RISCV] Reorganize select lowering to pull binop expansion early

This is purely stylistic, but I think makes the code easier to follow.

It isn't quite NFC because it undoes the airthmetic lowering for the
select c, simm12, 0 cases for a processor with both conditional
move forwarding and zicond.

Author: preames

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

@@ -9106,8 +9106,12 @@ static std::optional<bool> matchSetCC(SDValue LHS, SDValue RHS,
   return std::nullopt;
 }
 
-static SDValue combineSelectToBinOp(SDNode *N, SelectionDAG &DAG,
-                                    const RISCVSubtarget &Subtarget) {
+static bool isSimm12Constant(SDValue V) {
+  return isa<ConstantSDNode>(V) && V->getAsAPIntVal().isSignedIntN(12);
+}
+
+static SDValue lowerSelectToBinOp(SDNode *N, SelectionDAG &DAG,
+                                  const RISCVSubtarget &Subtarget) {
   SDValue CondV = N->getOperand(0);
   SDValue TrueV = N->getOperand(1);
   SDValue FalseV = N->getOperand(2);
@@ -9127,14 +9131,17 @@ static SDValue combineSelectToBinOp(SDNode *N, SelectionDAG &DAG,
       return DAG.getNode(ISD::OR, DL, VT, Neg, DAG.getFreeze(TrueV));
     }
 
+    const bool HasCZero = VT.isScalarInteger() &&
+      (Subtarget.hasStdExtZicond() || Subtarget.hasVendorXVentanaCondOps());
+
     // (select c, 0, y) -> (c-1) & y
-    if (isNullConstant(TrueV)) {
+    if (isNullConstant(TrueV) && (!HasCZero || isSimm12Constant(FalseV))) {
       SDValue Neg = DAG.getNode(ISD::ADD, DL, VT, CondV,
                                 DAG.getAllOnesConstant(DL, VT));
       return DAG.getNode(ISD::AND, DL, VT, Neg, DAG.getFreeze(FalseV));
     }
     // (select c, y, 0) -> -c & y
-    if (isNullConstant(FalseV)) {
+    if (isNullConstant(FalseV) && (!HasCZero || isSimm12Constant(TrueV))) {
       SDValue Neg = DAG.getNegative(CondV, DL, VT);
       return DAG.getNode(ISD::AND, DL, VT, Neg, DAG.getFreeze(TrueV));
     }
@@ -9240,10 +9247,6 @@ foldBinOpIntoSelectIfProfitable(SDNode *BO, SelectionDAG &DAG,
   return DAG.getSelect(DL, VT, Sel.getOperand(0), NewT, NewF);
 }
 
-static bool isSimm12Constant(SDValue V) {
-  return isa<ConstantSDNode>(V) && V->getAsAPIntVal().isSignedIntN(12);
-}
-
 SDValue RISCVTargetLowering::lowerSELECT(SDValue Op, SelectionDAG &DAG) const {
   SDValue CondV = Op.getOperand(0);
   SDValue TrueV = Op.getOperand(1);
@@ -9259,26 +9262,17 @@ SDValue RISCVTargetLowering::lowerSELECT(SDValue Op, SelectionDAG &DAG) const {
     return DAG.getNode(ISD::VSELECT, DL, VT, CondSplat, TrueV, FalseV);
   }
 
+  // Try some other optimizations before falling back to generic lowering.
+  if (SDValue V = lowerSelectToBinOp(Op.getNode(), DAG, Subtarget))
+    return V;
+
   // When Zicond or XVentanaCondOps is present, emit CZERO_EQZ and CZERO_NEZ
   // nodes to implement the SELECT. Performing the lowering here allows for
   // greater control over when CZERO_{EQZ/NEZ} are used vs another branchless
   // sequence or RISCVISD::SELECT_CC node (branch-based select).
   if ((Subtarget.hasStdExtZicond() || Subtarget.hasVendorXVentanaCondOps()) &&
       VT.isScalarInteger()) {
 
-    // select c, simm12, 0 -> andi (sub x0, c), simm12
-    if (isSimm12Constant(TrueV) && isNullConstant(FalseV)) {
-      SDValue Mask = DAG.getNegative(CondV, DL, VT);
-      return DAG.getNode(ISD::AND, DL, VT, TrueV, Mask);
-    }
-
-    // select c, 0, simm12 -> andi (addi c, -1), simm12
-    if (isNullConstant(TrueV) && isSimm12Constant(FalseV)) {
-      SDValue Mask = DAG.getNode(ISD::ADD, DL, VT, CondV,
-                                 DAG.getSignedConstant(-1, DL, XLenVT));
-      return DAG.getNode(ISD::AND, DL, VT, FalseV, Mask);
-    }
-
     // (select c, t, 0) -> (czero_eqz t, c)
     if (isNullConstant(FalseV))
       return DAG.getNode(RISCVISD::CZERO_EQZ, DL, VT, TrueV, CondV);
@@ -9332,10 +9326,6 @@ SDValue RISCVTargetLowering::lowerSELECT(SDValue Op, SelectionDAG &DAG) const {
           DAG.getNode(RISCVISD::CZERO_EQZ, DL, VT, TrueV, CondV));
     }
 
-    // Try some other optimizations before falling back to generic lowering.
-    if (SDValue V = combineSelectToBinOp(Op.getNode(), DAG, Subtarget))
-      return V;
-
     // (select c, c1, c2) -> (add (czero_nez c2 - c1, c), c1)
     // (select c, c1, c2) -> (add (czero_eqz c1 - c2, c), c2)
     if (isa<ConstantSDNode>(TrueV) && isa<ConstantSDNode>(FalseV)) {
@@ -9438,9 +9428,6 @@ SDValue RISCVTargetLowering::lowerSELECT(SDValue Op, SelectionDAG &DAG) const {
           SDNodeFlags::Disjoint);
   }
 
-  if (SDValue V = combineSelectToBinOp(Op.getNode(), DAG, Subtarget))
-    return V;
-
   if (Op.hasOneUse()) {
     unsigned UseOpc = Op->user_begin()->getOpcode();
     if (isBinOp(UseOpc) && DAG.isSafeToSpeculativelyExecute(UseOpc)) {

llvm/test/CodeGen/RISCV/cmov-branch-opt.ll

@@ -149,9 +149,8 @@ define signext i32 @test4(i32 signext %x, i32 signext %y, i32 signext %z) {
 ;
 ; CMOV-ZICOND-LABEL: test4:
 ; CMOV-ZICOND:       # %bb.0:
-; CMOV-ZICOND-NEXT:    snez a0, a2
-; CMOV-ZICOND-NEXT:    addi a0, a0, -1
-; CMOV-ZICOND-NEXT:    andi a0, a0, 3
+; CMOV-ZICOND-NEXT:    li a0, 3
+; CMOV-ZICOND-NEXT:    czero.nez a0, a0, a2
 ; CMOV-ZICOND-NEXT:    ret
 ;
 ; SFB-NOZICOND-LABEL: test4:
@@ -165,9 +164,8 @@ define signext i32 @test4(i32 signext %x, i32 signext %y, i32 signext %z) {
 ;
 ; SFB-ZICOND-LABEL: test4:
 ; SFB-ZICOND:       # %bb.0:
-; SFB-ZICOND-NEXT:    snez a0, a2
-; SFB-ZICOND-NEXT:    addi a0, a0, -1
-; SFB-ZICOND-NEXT:    andi a0, a0, 3
+; SFB-ZICOND-NEXT:    li a0, 3
+; SFB-ZICOND-NEXT:    czero.nez a0, a0, a2
 ; SFB-ZICOND-NEXT:    ret
   %c = icmp eq i32 %z, 0
   %a = select i1 %c, i32 3, i32 0