[RISCV] Try to perform select => binop combines when conditional operations are available

[asb]

Summary

When condops aren't available, these transformations are performed in lowerSELECT or in a DAGCombine on RISCVISD::SELECT_CC. This patch performs the first two of these combines, which are unconditionally profitable when conditional operations are enabled.

I'm posting this for comment on the general approach of performing this transformation as a dagcombine on select, and get feedback on the desirability of performing it even when condops aren't available (right now it causes a slight regression in a couple of the tests in min-max.ll). There are obviously a number of other condops related regressions, but this seemed like a relatively easily separable initial step. I'll also note that it's not possible to remove this transformation from lowerSELECT (and just rely on the ISD::SELECT combine) without causing codegen regressions, because a select might be produced through legalisation and lowered before the dag combiner runs again.

[craig.topper]

I had wondered if it made sense to handle Zicond/XventanaCondOps in LowerSelect by creating RISCV::CZERO_EQZ/CZERO_NEZ opcodes. Would cut down on the number of isel patterns.

[asb]

In D149166#4297768, @craig.topper wrote:

I had wondered if it made sense to handle Zicond/XventanaCondOps in LowerSelect by creating RISCV::CZERO_EQZ/CZERO_NEZ opcodes. Would cut down on the number of isel patterns.

Thanks, that's not an option I'd explored but I like that it might provide more uniformity for the condops vs non-condops. Let me have a play with that alternative.

[craig.topper]

In D149166#4298860, @asb wrote:

In D149166#4297768, @craig.topper wrote:

I had wondered if it made sense to handle Zicond/XventanaCondOps in LowerSelect by creating RISCV::CZERO_EQZ/CZERO_NEZ opcodes. Would cut down on the number of isel patterns.

Thanks, that's not an option I'd explored but I like that it might provide more uniformity for the condops vs non-condops. Let me have a play with that alternative.

Reverse ping. Any update on this?

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

 
 static SDValue combineSelectToBinOp(SDNode *N, SelectionDAG &DAG,
                                     const RISCVSubtarget &Subtarget) {
   SDValue CondV = N->getOperand(0);
   SDValue TrueV = N->getOperand(1);
   SDValue FalseV = N->getOperand(2);
   MVT VT = N->getSimpleValueType(0);
   SDLoc DL(N);
+  MVT XLenVT = Subtarget.getXLenVT();
+  bool HasCondOps = DAG.getTargetLoweringInfo().getOperationAction(
+                        ISD::SELECT, XLenVT) == TargetLowering::Legal;
 
   if (!Subtarget.hasShortForwardBranchOpt()) {
     // (select c, -1, y) -> -c | y
     if (isAllOnesConstant(TrueV)) {
       SDValue Neg = DAG.getNegative(CondV, DL, VT);
       return DAG.getNode(ISD::OR, DL, VT, Neg, FalseV);
     }
     // (select c, y, -1) -> (c-1) | y
     if (isAllOnesConstant(FalseV)) {
       SDValue Neg = DAG.getNode(ISD::ADD, DL, VT, CondV,
                                 DAG.getAllOnesConstant(DL, VT));
       return DAG.getNode(ISD::OR, DL, VT, Neg, TrueV);
     }
 
+    // The following combines may not be profitable when condops are
+    // available. TODO: Re-enable these combines when profitable.
+    if (HasCondOps)
+      return SDValue();
+
     // (select c, 0, y) -> (c-1) & y
     if (isNullConstant(TrueV)) {
       SDValue Neg = DAG.getNode(ISD::ADD, DL, VT, CondV,
                                 DAG.getAllOnesConstant(DL, VT));
       return DAG.getNode(ISD::AND, DL, VT, Neg, FalseV);
     }
     // (select c, y, 0) -> -c & y
     if (isNullConstant(FalseV)) {
static SDValue tryFoldSelectIntoOp(SDNode *N, SelectionDAG &DAG,
   return DAG.getNode(TrueVal.getOpcode(), DL, VT, FalseVal, NewSel);
 }
 
 static SDValue performSELECTCombine(SDNode *N, SelectionDAG &DAG,
                                     const RISCVSubtarget &Subtarget) {
   if (Subtarget.hasShortForwardBranchOpt())
     return SDValue();
 
+  MVT XLenVT = Subtarget.getXLenVT();
+
   // Only support XLenVT.
-  if (N->getValueType(0) != Subtarget.getXLenVT())
+  if (N->getValueType(0) != XLenVT)
     return SDValue();
 
   SDValue TrueVal = N->getOperand(1);
   SDValue FalseVal = N->getOperand(2);
   if (SDValue V = tryFoldSelectIntoOp(N, DAG, TrueVal, FalseVal, /*Swapped*/false))
     return V;
-  return tryFoldSelectIntoOp(N, DAG, FalseVal, TrueVal, /*Swapped*/true);
+  if (SDValue V =
+          tryFoldSelectIntoOp(N, DAG, FalseVal, TrueVal, /*Swapped*/ true))
+    return V;
+
+  // Only attempt further combines if the condition type is XLenVT.
+  if (N->getOperand(0).getValueType() != XLenVT)
+    return SDValue();
+
+  // Attempt further combines iff condops support is enabled. TODO: explore
+  // enabling even when condops are disabled.
+  const TargetLowering &TLI = DAG.getTargetLoweringInfo();
+  if (TLI.getOperationAction(ISD::SELECT, XLenVT) == TargetLowering::Legal)
+    return combineSelectToBinOp(N, DAG, Subtarget);
+
+  return SDValue();
 }
 
 // If we're concatenating a series of vector loads like
 // concat_vectors (load v4i8, p+0), (load v4i8, p+n), (load v4i8, p+n*2) ...
 // Then we can turn this into a strided load by widening the vector elements
 // vlse32 p, stride=n
 static SDValue performCONCAT_VECTORSCombine(SDNode *N, SelectionDAG &DAG,
                                             const RISCVSubtarget &Subtarget,

llvm/test/CodeGen/RISCV/select-binop-identity.ll

 ; SFB64-NEXT:  # %bb.1:
 ; SFB64-NEXT:    and a2, a2, a1
 ; SFB64-NEXT:  .LBB0_2:
 ; SFB64-NEXT:    mv a0, a2
 ; SFB64-NEXT:    ret
 ;
 ; VTCONDOPS64-LABEL: and_select_all_ones_i32:
 ; VTCONDOPS64:       # %bb.0:
-; VTCONDOPS64-NEXT:    li a3, -1
-; VTCONDOPS64-NEXT:    vt.maskcn a3, a3, a0
-; VTCONDOPS64-NEXT:    vt.maskc a0, a1, a0
-; VTCONDOPS64-NEXT:    or a0, a0, a3
+; VTCONDOPS64-NEXT:    addi a0, a0, -1
+; VTCONDOPS64-NEXT:    or a0, a0, a1
 ; VTCONDOPS64-NEXT:    and a0, a0, a2
 ; VTCONDOPS64-NEXT:    ret
 ;
 ; ZICOND-LABEL: and_select_all_ones_i32:
 ; ZICOND:       # %bb.0:
-; ZICOND-NEXT:    li a3, -1
-; ZICOND-NEXT:    czero.nez a3, a3, a0
-; ZICOND-NEXT:    czero.eqz a0, a1, a0
-; ZICOND-NEXT:    or a0, a0, a3
+; ZICOND-NEXT:    addi a0, a0, -1
+; ZICOND-NEXT:    or a0, a0, a1
 ; ZICOND-NEXT:    and a0, a0, a2
 ; ZICOND-NEXT:    ret
   %a = select i1 %c, i32 %x, i32 -1
   %b = and i32 %a, %y
   ret i32 %b
 }
 
 define i64 @and_select_all_ones_i64(i1 zeroext %c, i64 %x, i64 %y) {

 ; SFB64-NEXT:  # %bb.1:
 ; SFB64-NEXT:    and a2, a2, a1
 ; SFB64-NEXT:  .LBB1_2:
 ; SFB64-NEXT:    mv a0, a2
 ; SFB64-NEXT:    ret
 ;
 ; VTCONDOPS64-LABEL: and_select_all_ones_i64:
 ; VTCONDOPS64:       # %bb.0:
-; VTCONDOPS64-NEXT:    vt.maskcn a1, a1, a0
-; VTCONDOPS64-NEXT:    li a3, -1
-; VTCONDOPS64-NEXT:    vt.maskc a0, a3, a0
+; VTCONDOPS64-NEXT:    neg a0, a0
 ; VTCONDOPS64-NEXT:    or a0, a0, a1
 ; VTCONDOPS64-NEXT:    and a0, a2, a0
 ; VTCONDOPS64-NEXT:    ret
 ;
 ; ZICOND32-LABEL: and_select_all_ones_i64:
 ; ZICOND32:       # %bb.0:
-; ZICOND32-NEXT:    czero.nez a2, a2, a0
-; ZICOND32-NEXT:    li a5, -1
-; ZICOND32-NEXT:    czero.eqz a5, a5, a0
-; ZICOND32-NEXT:    or a2, a5, a2
-; ZICOND32-NEXT:    czero.nez a0, a1, a0
-; ZICOND32-NEXT:    or a0, a5, a0
+; ZICOND32-NEXT:    neg a0, a0
+; ZICOND32-NEXT:    or a2, a0, a2
+; ZICOND32-NEXT:    or a0, a0, a1
 ; ZICOND32-NEXT:    and a0, a3, a0
 ; ZICOND32-NEXT:    and a1, a4, a2
 ; ZICOND32-NEXT:    ret
 ;
 ; ZICOND64-LABEL: and_select_all_ones_i64:
 ; ZICOND64:       # %bb.0:
-; ZICOND64-NEXT:    czero.nez a1, a1, a0
-; ZICOND64-NEXT:    li a3, -1
-; ZICOND64-NEXT:    czero.eqz a0, a3, a0
+; ZICOND64-NEXT:    neg a0, a0
 ; ZICOND64-NEXT:    or a0, a0, a1
 ; ZICOND64-NEXT:    and a0, a2, a0
 ; ZICOND64-NEXT:    ret
   %a = select i1 %c, i64 -1, i64 %x
   %b = and i64 %y, %a
   ret i64 %b
 }