[RISCV] Add a DAG combine for (czero_eq X, (xor Y, 1)) -> (czero_ne X, Y) if Y is 0 or 1.

[craig.topper]

Summary

This is an alternative to D155288 that can handle other sources of
xori like FP compares. Unfortunately, it misses the i64 setge case
on RV32 in condops.ll.

[mgudim]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

12983

Why not express this using pattern?

[craig.topper]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

12983

I'm not sure what you mean.

[craig.topper]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

12983

Do you mean an isel pattern in tablegen? I'd need a PatFrag or ComplexPattern to call MaskValueIsZero. So there doesn't seem to be any advantage.

[mgudim]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

12983

def : Pat<(XLenVT (riscv_czero_eqz GPR:$rs1, (xor GPR:$rc, 1))),

(CZERO_NEZ GPR:$rs1, GPR:$rc)>;

[mgudim]

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

12983

right. Got it, thanks.

[mgudim]

LGTM, but it seems weird to me that we have to re-implement such basic optimisations instead of generic code doing this. @craig.topper @asb I know I asked this is the call, but I still don't understand why we emit target-specific node for SELECT so early?

[craig.topper]

In D155328#4502274, @mgudim wrote:

LGTM, but it seems weird to me that we have to re-implement such basic optimisations instead of generic code doing this. @craig.topper @asb I know I asked this is the call, but I still don't understand why we emit target-specific node for SELECT so early?

There isn’t a generic combine for this on select after type legalization. It depends on getBooleanContents to know the value for true. So I think it only exists for i1.

[mgudim]

In D155328#4502297, @craig.topper wrote:

In D155328#4502274, @mgudim wrote:

LGTM, but it seems weird to me that we have to re-implement such basic optimisations instead of generic code doing this. @craig.topper @asb I know I asked this is the call, but I still don't understand why we emit target-specific node for SELECT so early?

There isn’t a generic combine for this on select after type legalization. It depends on getBooleanContents to know the value for true. So I think it only exists for i1.

Let's pretend that "SELECT" was legal. (I commented out setOperationAction(ISD::SELECT, XLenVT, Custom). Run this code through llc (llc -march=riscv64 -O2)

define i64 @foo(i64 %x, i64 %y, i64 %u, i64 %v) {
  %c = icmp sge i64 %x, %y
  %select_ = select i1 %c, i64 %u, i64 %v
  ret i64 %select_
}

Generic DAG combine will remove the xor after legalization:

Legalized selection DAG: %bb.0 'foo:'
SelectionDAG has 17 nodes:
  t0: ch,glue = EntryToken
          t2: i64,ch = CopyFromReg t0, Register:i64 %0
          t4: i64,ch = CopyFromReg t0, Register:i64 %1
        t20: i64 = setcc t2, t4, setlt:ch
      t22: i64 = xor t20, Constant:i64<1> <- THIS XOR
      t6: i64,ch = CopyFromReg t0, Register:i64 %2
      t8: i64,ch = CopyFromReg t0, Register:i64 %3
    t11: i64 = select t22, t6, t8
  t13: ch,glue = CopyToReg t0, Register:i64 $x10, t11
  t14: ch = RISCVISD::RET_GLUE t13, Register:i64 $x10, t13:1


Optimized legalized selection DAG: %bb.0 'foo:'
SelectionDAG has 15 nodes:
  t0: ch,glue = EntryToken
        t2: i64,ch = CopyFromReg t0, Register:i64 %0
        t4: i64,ch = CopyFromReg t0, Register:i64 %1
      t20: i64 = setcc t2, t4, setlt:ch
      t8: i64,ch = CopyFromReg t0, Register:i64 %3
      t6: i64,ch = CopyFromReg t0, Register:i64 %2
    t23: i64 = select t20, t8, t6
  t13: ch,glue = CopyToReg t0, Register:i64 $x10, t23
  t14: ch = RISCVISD::RET_GLUE t13, Register:i64 $x10, t13:1

It is done by DAGCombiner.cpp, extractBooleanFlip which does the right thing depending on getBooleanContents

[asb]

I'd started down this route but then took the path of D155288 due to the fact this won't catch the RV32 cases. Did you have thoughts on those?

[mgudim]

@asb

Did you have thoughts on those?

I haven't looked yet. I am in favour of merging https://reviews.llvm.org/D155288 (maybe together with this patch) now and we can keep looking for a better solution.

[asb]

How about we merge this and then continue the discussion about something like D155288 to handle the remaining cases. This change LGTM.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

if (N->getValueType(0) == MVT::i64 && Subtarget.is64Bit()) {
         return SDValue(N, 0); // Return N so it doesn't get rechecked.
       }
     }
     return SDValue();
   case ISD::TRUNCATE:
     return performTRUNCATECombine(N, DAG, Subtarget);
   case ISD::SELECT:
     return performSELECTCombine(N, DAG, Subtarget);
+  case RISCVISD::CZERO_EQZ:
+  case RISCVISD::CZERO_NEZ:
+    // czero_eq X, (xor Y, 1) -> czero_ne X, Y if Y is 0 or 1.
+    // czero_ne X, (xor Y, 1) -> czero_eq X, Y if Y is 0 or 1.
+    if (N->getOperand(1).getOpcode() == ISD::XOR &&
+        isOneConstant(N->getOperand(1).getOperand(1))) {
+      SDValue Cond = N->getOperand(1).getOperand(0);
+      APInt Mask = APInt::getBitsSetFrom(Cond.getValueSizeInBits(), 1);
+      if (DAG.MaskedValueIsZero(Cond, Mask)) {
+        unsigned NewOpc = N->getOpcode() == RISCVISD::CZERO_EQZ
+                              ? RISCVISD::CZERO_NEZ
+                              : RISCVISD::CZERO_EQZ;
+        return DAG.getNode(NewOpc, SDLoc(N), N->getValueType(0),
+                           N->getOperand(0), Cond);
+      }
+    }
+    return SDValue();
+
   case RISCVISD::SELECT_CC: {
     // Transform
     SDValue LHS = N->getOperand(0);
     SDValue RHS = N->getOperand(1);
     SDValue CC = N->getOperand(2);
     ISD::CondCode CCVal = cast<CondCodeSDNode>(CC)->get();
     SDValue TrueV = N->getOperand(3);
     SDValue FalseV = N->getOperand(4);

llvm/test/CodeGen/RISCV/condops.ll

 ; RV64I-NEXT:    mv a2, a3
 ; RV64I-NEXT:  .LBB24_2:
 ; RV64I-NEXT:    mv a0, a2
 ; RV64I-NEXT:    ret
 ;
 ; RV64XVENTANACONDOPS-LABEL: setge:
 ; RV64XVENTANACONDOPS:       # %bb.0:
 ; RV64XVENTANACONDOPS-NEXT:    slt a0, a0, a1
-; RV64XVENTANACONDOPS-NEXT:    xori a0, a0, 1
-; RV64XVENTANACONDOPS-NEXT:    vt.maskcn a1, a3, a0
-; RV64XVENTANACONDOPS-NEXT:    vt.maskc a0, a2, a0
+; RV64XVENTANACONDOPS-NEXT:    vt.maskc a1, a3, a0
+; RV64XVENTANACONDOPS-NEXT:    vt.maskcn a0, a2, a0
 ; RV64XVENTANACONDOPS-NEXT:    or a0, a0, a1
 ; RV64XVENTANACONDOPS-NEXT:    ret
 ;
 ; RV64XTHEADCONDMOV-LABEL: setge:
 ; RV64XTHEADCONDMOV:       # %bb.0:
 ; RV64XTHEADCONDMOV-NEXT:    slt a0, a0, a1
 ; RV64XTHEADCONDMOV-NEXT:    th.mvnez a2, a3, a0
 ; RV64XTHEADCONDMOV-NEXT:    mv a0, a2

 ; RV32ZICOND-NEXT:    czero.nez a2, a7, a1
 ; RV32ZICOND-NEXT:    czero.eqz a1, a5, a1
 ; RV32ZICOND-NEXT:    or a1, a1, a2
 ; RV32ZICOND-NEXT:    ret
 ;
 ; RV64ZICOND-LABEL: setge:
 ; RV64ZICOND:       # %bb.0:
 ; RV64ZICOND-NEXT:    slt a0, a0, a1
-; RV64ZICOND-NEXT:    xori a0, a0, 1
-; RV64ZICOND-NEXT:    czero.nez a1, a3, a0
-; RV64ZICOND-NEXT:    czero.eqz a0, a2, a0
+; RV64ZICOND-NEXT:    czero.eqz a1, a3, a0
+; RV64ZICOND-NEXT:    czero.nez a0, a2, a0
 ; RV64ZICOND-NEXT:    or a0, a0, a1
 ; RV64ZICOND-NEXT:    ret
   %rc = icmp sge i64 %a, %b
   %sel = select i1 %rc, i64 %rs1, i64 %rs2
   ret i64 %sel
 }
 
 define i64 @setlt(i64 %a, i64 %b, i64 %rs1, i64 %rs2) {

 ; RV64I-NEXT:    mv a2, a3
 ; RV64I-NEXT:  .LBB26_2:
 ; RV64I-NEXT:    mv a0, a2
 ; RV64I-NEXT:    ret
 ;
 ; RV64XVENTANACONDOPS-LABEL: setle:
 ; RV64XVENTANACONDOPS:       # %bb.0:
 ; RV64XVENTANACONDOPS-NEXT:    slt a0, a1, a0
-; RV64XVENTANACONDOPS-NEXT:    xori a0, a0, 1
-; RV64XVENTANACONDOPS-NEXT:    vt.maskcn a1, a3, a0
-; RV64XVENTANACONDOPS-NEXT:    vt.maskc a0, a2, a0
+; RV64XVENTANACONDOPS-NEXT:    vt.maskc a1, a3, a0
+; RV64XVENTANACONDOPS-NEXT:    vt.maskcn a0, a2, a0
 ; RV64XVENTANACONDOPS-NEXT:    or a0, a0, a1
 ; RV64XVENTANACONDOPS-NEXT:    ret
 ;
 ; RV64XTHEADCONDMOV-LABEL: setle:
 ; RV64XTHEADCONDMOV:       # %bb.0:
 ; RV64XTHEADCONDMOV-NEXT:    slt a0, a1, a0
 ; RV64XTHEADCONDMOV-NEXT:    th.mvnez a2, a3, a0
 ; RV64XTHEADCONDMOV-NEXT:    mv a0, a2

 ; RV32ZICOND-NEXT:    czero.nez a2, a7, a1
 ; RV32ZICOND-NEXT:    czero.eqz a1, a5, a1
 ; RV32ZICOND-NEXT:    or a1, a1, a2
 ; RV32ZICOND-NEXT:    ret
 ;
 ; RV64ZICOND-LABEL: setle:
 ; RV64ZICOND:       # %bb.0:
 ; RV64ZICOND-NEXT:    slt a0, a1, a0
-; RV64ZICOND-NEXT:    xori a0, a0, 1
-; RV64ZICOND-NEXT:    czero.nez a1, a3, a0
-; RV64ZICOND-NEXT:    czero.eqz a0, a2, a0
+; RV64ZICOND-NEXT:    czero.eqz a1, a3, a0
+; RV64ZICOND-NEXT:    czero.nez a0, a2, a0
 ; RV64ZICOND-NEXT:    or a0, a0, a1
 ; RV64ZICOND-NEXT:    ret
   %rc = icmp sle i64 %a, %b
   %sel = select i1 %rc, i64 %rs1, i64 %rs2
   ret i64 %sel
 }
 
 define i64 @setugt(i64 %a, i64 %b, i64 %rs1, i64 %rs2) {

 ; RV64I-NEXT:    mv a2, a3
 ; RV64I-NEXT:  .LBB28_2:
 ; RV64I-NEXT:    mv a0, a2
 ; RV64I-NEXT:    ret
 ;
 ; RV64XVENTANACONDOPS-LABEL: setuge:
 ; RV64XVENTANACONDOPS:       # %bb.0:
 ; RV64XVENTANACONDOPS-NEXT:    sltu a0, a0, a1
-; RV64XVENTANACONDOPS-NEXT:    xori a0, a0, 1
-; RV64XVENTANACONDOPS-NEXT:    vt.maskcn a1, a3, a0
-; RV64XVENTANACONDOPS-NEXT:    vt.maskc a0, a2, a0
+; RV64XVENTANACONDOPS-NEXT:    vt.maskc a1, a3, a0
+; RV64XVENTANACONDOPS-NEXT:    vt.maskcn a0, a2, a0
 ; RV64XVENTANACONDOPS-NEXT:    or a0, a0, a1
 ; RV64XVENTANACONDOPS-NEXT:    ret
 ;
 ; RV64XTHEADCONDMOV-LABEL: setuge:
 ; RV64XTHEADCONDMOV:       # %bb.0:
 ; RV64XTHEADCONDMOV-NEXT:    sltu a0, a0, a1
 ; RV64XTHEADCONDMOV-NEXT:    th.mvnez a2, a3, a0
 ; RV64XTHEADCONDMOV-NEXT:    mv a0, a2

 ; RV32ZICOND-NEXT:    czero.nez a2, a7, a1
 ; RV32ZICOND-NEXT:    czero.eqz a1, a5, a1
 ; RV32ZICOND-NEXT:    or a1, a1, a2
 ; RV32ZICOND-NEXT:    ret
 ;
 ; RV64ZICOND-LABEL: setuge:
 ; RV64ZICOND:       # %bb.0:
 ; RV64ZICOND-NEXT:    sltu a0, a0, a1
-; RV64ZICOND-NEXT:    xori a0, a0, 1
-; RV64ZICOND-NEXT:    czero.nez a1, a3, a0
-; RV64ZICOND-NEXT:    czero.eqz a0, a2, a0
+; RV64ZICOND-NEXT:    czero.eqz a1, a3, a0
+; RV64ZICOND-NEXT:    czero.nez a0, a2, a0
 ; RV64ZICOND-NEXT:    or a0, a0, a1
 ; RV64ZICOND-NEXT:    ret
   %rc = icmp uge i64 %a, %b
   %sel = select i1 %rc, i64 %rs1, i64 %rs2
   ret i64 %sel
 }
 
 define i64 @setult(i64 %a, i64 %b, i64 %rs1, i64 %rs2) {

 ; RV64I-NEXT:    mv a2, a3
 ; RV64I-NEXT:  .LBB30_2:
 ; RV64I-NEXT:    mv a0, a2
 ; RV64I-NEXT:    ret
 ;
 ; RV64XVENTANACONDOPS-LABEL: setule:
 ; RV64XVENTANACONDOPS:       # %bb.0:
 ; RV64XVENTANACONDOPS-NEXT:    sltu a0, a1, a0
-; RV64XVENTANACONDOPS-NEXT:    xori a0, a0, 1
-; RV64XVENTANACONDOPS-NEXT:    vt.maskcn a1, a3, a0
-; RV64XVENTANACONDOPS-NEXT:    vt.maskc a0, a2, a0
+; RV64XVENTANACONDOPS-NEXT:    vt.maskc a1, a3, a0
+; RV64XVENTANACONDOPS-NEXT:    vt.maskcn a0, a2, a0
 ; RV64XVENTANACONDOPS-NEXT:    or a0, a0, a1
 ; RV64XVENTANACONDOPS-NEXT:    ret
 ;
 ; RV64XTHEADCONDMOV-LABEL: setule:
 ; RV64XTHEADCONDMOV:       # %bb.0:
 ; RV64XTHEADCONDMOV-NEXT:    sltu a0, a1, a0
 ; RV64XTHEADCONDMOV-NEXT:    th.mvnez a2, a3, a0
 ; RV64XTHEADCONDMOV-NEXT:    mv a0, a2

 ; RV32ZICOND-NEXT:    czero.nez a2, a7, a1
 ; RV32ZICOND-NEXT:    czero.eqz a1, a5, a1
 ; RV32ZICOND-NEXT:    or a1, a1, a2
 ; RV32ZICOND-NEXT:    ret
 ;
 ; RV64ZICOND-LABEL: setule:
 ; RV64ZICOND:       # %bb.0:
 ; RV64ZICOND-NEXT:    sltu a0, a1, a0
-; RV64ZICOND-NEXT:    xori a0, a0, 1
-; RV64ZICOND-NEXT:    czero.nez a1, a3, a0
-; RV64ZICOND-NEXT:    czero.eqz a0, a2, a0
+; RV64ZICOND-NEXT:    czero.eqz a1, a3, a0
+; RV64ZICOND-NEXT:    czero.nez a0, a2, a0
 ; RV64ZICOND-NEXT:    or a0, a0, a1
 ; RV64ZICOND-NEXT:    ret
   %rc = icmp ule i64 %a, %b
   %sel = select i1 %rc, i64 %rs1, i64 %rs2
   ret i64 %sel
 }
 
 define i64 @seteq_zero(i64 %a, i64 %rs1, i64 %rs2) {

 ; RV64I-NEXT:  # %bb.1:
 ; RV64I-NEXT:    mv a0, a1
 ; RV64I-NEXT:  .LBB56_2:
 ; RV64I-NEXT:    ret
 ;
 ; RV64XVENTANACONDOPS-LABEL: setune_32:
 ; RV64XVENTANACONDOPS:       # %bb.0:
 ; RV64XVENTANACONDOPS-NEXT:    feq.s a2, fa0, fa1
-; RV64XVENTANACONDOPS-NEXT:    xori a2, a2, 1
-; RV64XVENTANACONDOPS-NEXT:    vt.maskcn a1, a1, a2
-; RV64XVENTANACONDOPS-NEXT:    vt.maskc a0, a0, a2
+; RV64XVENTANACONDOPS-NEXT:    vt.maskc a1, a1, a2
+; RV64XVENTANACONDOPS-NEXT:    vt.maskcn a0, a0, a2
 ; RV64XVENTANACONDOPS-NEXT:    or a0, a0, a1
 ; RV64XVENTANACONDOPS-NEXT:    ret
 ;
 ; RV64XTHEADCONDMOV-LABEL: setune_32:
 ; RV64XTHEADCONDMOV:       # %bb.0:
 ; RV64XTHEADCONDMOV-NEXT:    feq.s a2, fa0, fa1
 ; RV64XTHEADCONDMOV-NEXT:    th.mvnez a0, a1, a2
 ; RV64XTHEADCONDMOV-NEXT:    ret
 ;
 ; RV32ZICOND-LABEL: setune_32:
 ; RV32ZICOND:       # %bb.0:
 ; RV32ZICOND-NEXT:    feq.s a2, fa0, fa1
-; RV32ZICOND-NEXT:    xori a2, a2, 1
-; RV32ZICOND-NEXT:    czero.nez a1, a1, a2
-; RV32ZICOND-NEXT:    czero.eqz a0, a0, a2
+; RV32ZICOND-NEXT:    czero.eqz a1, a1, a2
+; RV32ZICOND-NEXT:    czero.nez a0, a0, a2
 ; RV32ZICOND-NEXT:    or a0, a0, a1
 ; RV32ZICOND-NEXT:    ret
 ;
 ; RV64ZICOND-LABEL: setune_32:
 ; RV64ZICOND:       # %bb.0:
 ; RV64ZICOND-NEXT:    feq.s a2, fa0, fa1
-; RV64ZICOND-NEXT:    xori a2, a2, 1
-; RV64ZICOND-NEXT:    czero.nez a1, a1, a2
-; RV64ZICOND-NEXT:    czero.eqz a0, a0, a2
+; RV64ZICOND-NEXT:    czero.eqz a1, a1, a2
+; RV64ZICOND-NEXT:    czero.nez a0, a0, a2
 ; RV64ZICOND-NEXT:    or a0, a0, a1
 ; RV64ZICOND-NEXT:    ret
   %rc = fcmp une float %a, %b
   %sel = select i1 %rc, i32 %rs1, i32 %rs2
   ret i32 %sel
 }
 
 define i64 @setune_64(float %a, float %b, i64 %rs1, i64 %rs2) {

 ; RV64I-NEXT:  # %bb.1:
 ; RV64I-NEXT:    mv a0, a1
 ; RV64I-NEXT:  .LBB57_2:
 ; RV64I-NEXT:    ret
 ;
 ; RV64XVENTANACONDOPS-LABEL: setune_64:
 ; RV64XVENTANACONDOPS:       # %bb.0:
 ; RV64XVENTANACONDOPS-NEXT:    feq.s a2, fa0, fa1
-; RV64XVENTANACONDOPS-NEXT:    xori a2, a2, 1
-; RV64XVENTANACONDOPS-NEXT:    vt.maskcn a1, a1, a2
-; RV64XVENTANACONDOPS-NEXT:    vt.maskc a0, a0, a2
+; RV64XVENTANACONDOPS-NEXT:    vt.maskc a1, a1, a2
+; RV64XVENTANACONDOPS-NEXT:    vt.maskcn a0, a0, a2
 ; RV64XVENTANACONDOPS-NEXT:    or a0, a0, a1
 ; RV64XVENTANACONDOPS-NEXT:    ret
 ;
 ; RV64XTHEADCONDMOV-LABEL: setune_64:
 ; RV64XTHEADCONDMOV:       # %bb.0:
 ; RV64XTHEADCONDMOV-NEXT:    feq.s a2, fa0, fa1
 ; RV64XTHEADCONDMOV-NEXT:    th.mvnez a0, a1, a2
 ; RV64XTHEADCONDMOV-NEXT:    ret
 ;
 ; RV32ZICOND-LABEL: setune_64:
 ; RV32ZICOND:       # %bb.0:
 ; RV32ZICOND-NEXT:    feq.s a4, fa0, fa1
-; RV32ZICOND-NEXT:    xori a4, a4, 1
-; RV32ZICOND-NEXT:    czero.nez a2, a2, a4
-; RV32ZICOND-NEXT:    czero.eqz a0, a0, a4
+; RV32ZICOND-NEXT:    czero.eqz a2, a2, a4
+; RV32ZICOND-NEXT:    czero.nez a0, a0, a4
 ; RV32ZICOND-NEXT:    or a0, a0, a2
-; RV32ZICOND-NEXT:    czero.nez a2, a3, a4
-; RV32ZICOND-NEXT:    czero.eqz a1, a1, a4
+; RV32ZICOND-NEXT:    czero.eqz a2, a3, a4
+; RV32ZICOND-NEXT:    czero.nez a1, a1, a4
 ; RV32ZICOND-NEXT:    or a1, a1, a2
 ; RV32ZICOND-NEXT:    ret
 ;
 ; RV64ZICOND-LABEL: setune_64:
 ; RV64ZICOND:       # %bb.0:
 ; RV64ZICOND-NEXT:    feq.s a2, fa0, fa1
-; RV64ZICOND-NEXT:    xori a2, a2, 1
-; RV64ZICOND-NEXT:    czero.nez a1, a1, a2
-; RV64ZICOND-NEXT:    czero.eqz a0, a0, a2
+; RV64ZICOND-NEXT:    czero.eqz a1, a1, a2
+; RV64ZICOND-NEXT:    czero.nez a0, a0, a2
 ; RV64ZICOND-NEXT:    or a0, a0, a1
 ; RV64ZICOND-NEXT:    ret
   %rc = fcmp une float %a, %b
   %sel = select i1 %rc, i64 %rs1, i64 %rs2
   ret i64 %sel
 }