[DAGCombiner][TLI] SimplifySetCC(): recanonicalize `(X&1) != 0` (PR51312)

[lebedev.ri]

Summary

(x&1)==1 is not canonical, and we'd canonicalize it to (x&1)!=0 already,
but that excludes it from a few folds, which eventually leads to a worse codegen.
This essentially fixes a regression after we started to canonicalize
add/xor reductions w/ i1 element type to a parity check.

[craig.topper]

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

3875

Does this renormalize only happen if VT doesn't match Op0.getValueType()?

llvm/test/CodeGen/SystemZ/int-cmp-47.ll

270

Is this a regression?

llvm/test/CodeGen/X86/parity-vec.ll

66

Is this a different issue?

[lebedev.ri]

llvm/test/CodeGen/SystemZ/int-cmp-47.ll

270

@uweigand - suggestions on how to avoid this? (i guess this was initially added in rL195473)

[lebedev.ri]

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

3875

Yes, and i've guarded against infinite looping via if (N1C->isOne() || Op0 != OrigOp0) {

[lebedev.ri]

Actually, let's not.

llvm/test/CodeGen/X86/parity-vec.ll

66

It's similar in a way, here we need (x&1)^1 or x^1.
But allowing Cond == ISD::SETNE results in a swamp of regressions.,
with a few wins. Perhaps that highlights that the approach is wrong.

llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp

if (N0.getOpcode() == ISD::ZERO_EXTEND) {
             Val = DAG.getNode(ISD::AND, dl, N0.getValueType(),
                               N0.getOperand(0).getOperand(0),
                               N0.getOperand(1));
           }
 
           return DAG.getSetCC(dl, VT, Val, N1,
                               Cond == ISD::SETEQ ? ISD::SETNE : ISD::SETEQ);
         }
-      } else if (N1C->isOne()) {
+      } else if (N1C->isNullValue() || N1C->isOne()) {
         SDValue Op0 = N0;
         if (Op0.getOpcode() == ISD::TRUNCATE)
           Op0 = Op0.getOperand(0);
 
-        if ((Op0.getOpcode() == ISD::XOR) &&
+        // FIXME: allow comparison with zero.
+        if (N1C->isOne() && (Op0.getOpcode() == ISD::XOR) &&
             Op0.getOperand(0).getOpcode() == ISD::SETCC &&
             Op0.getOperand(1).getOpcode() == ISD::SETCC) {
           SDValue XorLHS = Op0.getOperand(0);
           SDValue XorRHS = Op0.getOperand(1);
           // Ensure that the input setccs return an i1 type or 0/1 value.
           if (Op0.getValueType() == MVT::i1 ||
               (getBooleanContents(XorLHS.getOperand(0).getValueType()) ==
                       ZeroOrOneBooleanContent &&
                getBooleanContents(XorRHS.getOperand(0).getValueType()) ==
                         ZeroOrOneBooleanContent)) {
             // (xor (setcc), (setcc)) == / != 1 -> (setcc) != / == (setcc)
             Cond = (Cond == ISD::SETEQ) ? ISD::SETNE : ISD::SETEQ;
             return DAG.getSetCC(dl, VT, XorLHS, XorRHS, Cond);
           }
         }
-        if (Op0.getOpcode() == ISD::AND && isOneConstant(Op0.getOperand(1))) {
-          // If this is (X&1) == / != 1, normalize it to (X&1) != / == 0.
+        if (Op0.getOpcode() == ISD::AND && isOneConstant(Op0.getOperand(1)) &&
+            (N1C->isOne() ||
+             (Cond == ISD::SETNE && !DCI.isAfterLegalizeDAG()))) {
+          // If this is (X&1) == / != 1,   normalize it to (X&1) != / == 0.
+          // If this is (X&1)      != 0, renormalize it to (X&1)      != 0.
+          SDValue OrigOp0 = Op0;
           if (Op0.getValueType().bitsGT(VT))
             Op0 = DAG.getNode(ISD::AND, dl, VT,
                           DAG.getNode(ISD::TRUNCATE, dl, VT, Op0.getOperand(0)),
                           DAG.getConstant(1, dl, VT));
           else if (Op0.getValueType().bitsLT(VT))
             Op0 = DAG.getNode(ISD::AND, dl, VT,
                         DAG.getNode(ISD::ANY_EXTEND, dl, VT, Op0.getOperand(0)),
                         DAG.getConstant(1, dl, VT));
-
-          return DAG.getSetCC(dl, VT, Op0,
-                              DAG.getConstant(0, dl, Op0.getValueType()),
-                              Cond == ISD::SETEQ ? ISD::SETNE : ISD::SETEQ);
+          if (N1C->isOne() || Op0 != OrigOp0) {
+            if (N1C->isOne())
+              Cond = (Cond == ISD::SETEQ) ? ISD::SETNE : ISD::SETEQ;
+            return DAG.getSetCC(
+                dl, VT, Op0, DAG.getConstant(0, dl, Op0.getValueType()), Cond);
+          }
         }
-        if (Op0.getOpcode() == ISD::AssertZext &&
+        // FIXME: allow comparison with zero.
+        if (N1C->isOne() && Op0.getOpcode() == ISD::AssertZext &&
             cast<VTSDNode>(Op0.getOperand(1))->getVT() == MVT::i1)
           return DAG.getSetCC(dl, VT, Op0,
                               DAG.getConstant(0, dl, Op0.getValueType()),
                               Cond == ISD::SETEQ ? ISD::SETNE : ISD::SETEQ);
       }
     }
 
     // Given:

llvm/test/CodeGen/SystemZ/int-cmp-47.ll

 exit:
   ret void
 }
 
 ; Check that we can fold an SHR into a TMxx mask.
 define void @f12(i64 %a) {
 ; CHECK-LABEL: f12:
 ; CHECK:       # %bb.0: # %entry
-; CHECK-NEXT:    tmhh %r2, 256
+; CHECK-NEXT:    srlg %r0, %r2, 56
+; CHECK-NEXT:    tmll %r0, 1
 ; CHECK-NEXT:    bner %r14
 ; CHECK-NEXT:  .LBB11_1: # %store
 ; CHECK-NEXT:    lgrl %r1, g@GOT
 ; CHECK-NEXT:    mvhi 0(%r1), 1
 ; CHECK-NEXT:    br %r14
 entry:
   %shr = lshr i64 %a, 56
   %and = and i64 %shr, 1

llvm/test/CodeGen/SystemZ/knownbits.ll

 ; CHECK-LABEL: f1:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    risbgn %r0, %r2, 63, 191, 0
 ; CHECK-NEXT:    risbgn %r1, %r3, 63, 191, 0
 ; CHECK-NEXT:    vlvgp %v0, %r0, %r1
 ; CHECK-NEXT:    vrepig %v1, 1
 ; CHECK-NEXT:    vx %v0, %v0, %v1
 ; CHECK-NEXT:    vlgvg %r0, %v0, 0
-; CHECK-NEXT:    cgijlh %r0, 0, .LBB1_3
+; CHECK-NEXT:    cijlh %r0, 0, .LBB1_3
 ; CHECK-NEXT:  # %bb.1:
 ; CHECK-NEXT:    vlgvg %r0, %v0, 1
-; CHECK-NEXT:    cgijlh %r0, 0, .LBB1_3
+; CHECK-NEXT:    cijlh %r0, 0, .LBB1_3
 ; CHECK-NEXT:  # %bb.2:
 ; CHECK-NEXT:  .LBB1_3:
   %1 = and i64 %a0, 1
   %2 = and i64 %a1, 1
   %3 = insertelement <2 x i64> undef, i64 %1, i32 0
   %4 = insertelement <2 x i64> %3, i64 %2, i32 1
   %5 = xor <2 x i64> %4, <i64 1, i64 1>
   %6 = extractelement <2 x i64> %5, i32 0

llvm/test/CodeGen/X86/bool-math.ll

 ; X64-NEXT:    shrq $32, %rax
 ; X64-NEXT:    xorl %edi, %eax
 ; X64-NEXT:    andl $1, %eax
 ; X64-NEXT:    # kill: def $al killed $al killed $rax
 ; X64-NEXT:    retq
 ;
 ; X32-LABEL: opaque_constant:
 ; X32:       # %bb.0:
-; X32-NEXT:    movl {{[0-9]+}}(%esp), %eax
-; X32-NEXT:    xorl {{[0-9]+}}(%esp), %eax
-; X32-NEXT:    andl $1, %eax
-; X32-NEXT:    # kill: def $al killed $al killed $eax
+; X32-NEXT:    movb {{[0-9]+}}(%esp), %al
+; X32-NEXT:    xorb {{[0-9]+}}(%esp), %al
+; X32-NEXT:    andb $1, %al
 ; X32-NEXT:    retl
   %andx = and i48 %x, 4294967296
   %andy = and i48 %y, 4294967296
   %cmp1 = icmp ne i48 %andx, 0
   %cmp2 = icmp ne i48 %andy, 0
   %r = xor i1 %cmp1, %cmp2
   ret i1 %r
 }
 

llvm/test/CodeGen/X86/parity-vec.ll

; CHECK-NEXT:    retq
   ret i1 %r
 }
 define i1 @canonical_parity(<16 x i1> %x) {
 ; CHECK-LABEL: canonical_parity:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    psllw $7, %xmm0
 ; CHECK-NEXT:    pmovmskb %xmm0, %eax
 ; CHECK-NEXT:    popcntw %ax, %ax
-; CHECK-NEXT:    testb $1, %al
-; CHECK-NEXT:    setne %al
+; CHECK-NEXT:    # kill: def $al killed $al killed $ax
 ; CHECK-NEXT:    retq
   %i1 = bitcast <16 x i1> %x to i16
   %i2 = call i16 @llvm.ctpop.i16(i16 %i1)
   %i3 = and i16 %i2, 1
   %i4 = icmp ne i16 %i3, 0
   ret i1 %i4
 }
 define i1 @canonical_parity_noncanonical_pred(<16 x i1> %x) {

 }
 define i1 @canonical_nonparity(<16 x i1> %x) {
 ; CHECK-LABEL: canonical_nonparity:
 ; CHECK:       # %bb.0:
 ; CHECK-NEXT:    psllw $7, %xmm0
 ; CHECK-NEXT:    pmovmskb %xmm0, %eax
 ; CHECK-NEXT:    popcntw %ax, %ax
 ; CHECK-NEXT:    testb $1, %al
 ; CHECK-NEXT:    sete %al
 ; CHECK-NEXT:    retq
   %i1 = bitcast <16 x i1> %x to i16
   %i2 = call i16 @llvm.ctpop.i16(i16 %i1)
   %i3 = and i16 %i2, 1
   %i4 = icmp eq i16 %i3, 0
   ret i1 %i4
 }
 define i1 @canonical_nonparity_noncanonical_pred(<16 x i1> %x) {