[X86] Prevent LowerSELECT from causing suboptimal codegen for __builtn_ffs(X) - 1.

[craig.topper]

Summary

LowerSELECT sees the CMP with 0 and wants to use a trick with SUB
and SBB. But we can use the flags from the BSF/TZCNT.

Fixes PR46203.

[RKSimon]

LGTM

llvm/lib/Target/X86/X86ISelLowering.cpp

 //===-- X86ISelLowering.cpp - X86 DAG Lowering Implementation -------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
SDValue X86TargetLowering::LowerSELECT(SDValue Op, SelectionDAG &DAG) const {
   // (select (x != 0), y, -1) -> (sign_bit (x - 1)) | y
   // (select (x != 0), -1, y) -> ~(sign_bit (x - 1)) | y
   // (select (and (x , 0x1) == 0), y, (z ^ y) ) -> (-(and (x , 0x1)) & z ) ^ y
   // (select (and (x , 0x1) == 0), y, (z | y) ) -> (-(and (x , 0x1)) & z ) | y
   if (Cond.getOpcode() == X86ISD::SETCC &&
       Cond.getOperand(1).getOpcode() == X86ISD::CMP &&
       isNullConstant(Cond.getOperand(1).getOperand(1))) {
     SDValue Cmp = Cond.getOperand(1);
+    SDValue CmpOp0 = Cmp.getOperand(0);
     unsigned CondCode = Cond.getConstantOperandVal(0);
 
-    if ((isAllOnesConstant(Op1) || isAllOnesConstant(Op2)) &&
+    // Special handling for __builtin_ffs(X) - 1 pattern which looks like
+    // (select (seteq X, 0), -1, (cttz_zero_undef X)). Disable the special
+    // handle to keep the CMP with 0. This should be removed by
+    // optimizeCompareInst by using the flags from the BSR/TZCNT used for the
+    // cttz_zero_undef.
+    auto MatchFFSMinus1 = [&](SDValue Op1, SDValue Op2) {
+      return (Op1.getOpcode() == ISD::CTTZ_ZERO_UNDEF && Op1.hasOneUse() &&
+              Op1.getOperand(0) == CmpOp0 && isAllOnesConstant(Op2));
+    };
+    if (Subtarget.hasCMov() && (VT == MVT::i32 || VT == MVT::i64) &&
+        ((CondCode == X86::COND_NE && MatchFFSMinus1(Op1, Op2)) ||
+         (CondCode == X86::COND_E && MatchFFSMinus1(Op2, Op1)))) {
+      // Keep Cmp.
+    } else if ((isAllOnesConstant(Op1) || isAllOnesConstant(Op2)) &&
         (CondCode == X86::COND_E || CondCode == X86::COND_NE)) {
       SDValue Y = isAllOnesConstant(Op2) ? Op1 : Op2;
-      SDValue CmpOp0 = Cmp.getOperand(0);
 
       SDVTList VTs = DAG.getVTList(Op.getValueType(), MVT::i32);
       SDVTList CmpVTs = DAG.getVTList(CmpOp0.getValueType(), MVT::i32);
 
       // Apply further optimizations for special cases
       // (select (x != 0), -1, 0) -> neg & sbb
       // (select (x == 0), 0, -1) -> neg & sbb
       if (isNullConstant(Y) &&
if (Subtarget.hasCMov() && (VT == MVT::i32 || VT == MVT::i64) &&
 
       if (isAllOnesConstant(Op1) != (CondCode == X86::COND_E))
         Res = DAG.getNOT(DL, Res, Res.getValueType());
 
       return DAG.getNode(ISD::OR, DL, Res.getValueType(), Res, Y);
     } else if (!Subtarget.hasCMov() && CondCode == X86::COND_E &&
                Cmp.getOperand(0).getOpcode() == ISD::AND &&
                isOneConstant(Cmp.getOperand(0).getOperand(1))) {
-      SDValue CmpOp0 = Cmp.getOperand(0);
       SDValue Src1, Src2;
       // true if Op2 is XOR or OR operator and one of its operands
       // is equal to Op1
       // ( a , a op b) || ( b , a op b)
       auto isOrXorPattern = [&]() {
         if ((Op2.getOpcode() == ISD::XOR || Op2.getOpcode() == ISD::OR) &&
             (Op2.getOperand(0) == Op1 || Op2.getOperand(1) == Op1)) {
           Src1 =

llvm/test/CodeGen/X86/dagcombine-select.ll

; BMI-NEXT:    retq
   ret i32 %add
 }
 
 ; This matches the pattern emitted for __builtin_ffs - 1
 define i32 @cttz_32_eq_select_ffs_m1(i32 %v) nounwind {
 ; NOBMI-LABEL: cttz_32_eq_select_ffs_m1:
 ; NOBMI:       # %bb.0:
 ; NOBMI-NEXT:    bsfl %edi, %ecx
-; NOBMI-NEXT:    xorl %eax, %eax
-; NOBMI-NEXT:    cmpl $1, %edi
-; NOBMI-NEXT:    sbbl %eax, %eax
-; NOBMI-NEXT:    orl %ecx, %eax
+; NOBMI-NEXT:    movl $-1, %eax
+; NOBMI-NEXT:    cmovnel %ecx, %eax
 ; NOBMI-NEXT:    retq
 ;
 ; BMI-LABEL: cttz_32_eq_select_ffs_m1:
 ; BMI:       # %bb.0:
 ; BMI-NEXT:    tzcntl %edi, %ecx
-; BMI-NEXT:    xorl %eax, %eax
-; BMI-NEXT:    cmpl $1, %edi
-; BMI-NEXT:    sbbl %eax, %eax
-; BMI-NEXT:    orl %ecx, %eax
+; BMI-NEXT:    movl $-1, %eax
+; BMI-NEXT:    cmovael %ecx, %eax
 ; BMI-NEXT:    retq
 
   %cnt = tail call i32 @llvm.cttz.i32(i32 %v, i1 true)
   %tobool = icmp eq i32 %v, 0
   %sel = select i1 %tobool, i32 -1, i32 %cnt
   ret i32 %sel
 }
 
 define i32 @cttz_32_ne_select_ffs_m1(i32 %v) nounwind {
 ; NOBMI-LABEL: cttz_32_ne_select_ffs_m1:
 ; NOBMI:       # %bb.0:
 ; NOBMI-NEXT:    bsfl %edi, %ecx
-; NOBMI-NEXT:    xorl %eax, %eax
-; NOBMI-NEXT:    cmpl $1, %edi
-; NOBMI-NEXT:    sbbl %eax, %eax
-; NOBMI-NEXT:    orl %ecx, %eax
+; NOBMI-NEXT:    movl $-1, %eax
+; NOBMI-NEXT:    cmovnel %ecx, %eax
 ; NOBMI-NEXT:    retq
 ;
 ; BMI-LABEL: cttz_32_ne_select_ffs_m1:
 ; BMI:       # %bb.0:
 ; BMI-NEXT:    tzcntl %edi, %ecx
-; BMI-NEXT:    xorl %eax, %eax
-; BMI-NEXT:    cmpl $1, %edi
-; BMI-NEXT:    sbbl %eax, %eax
-; BMI-NEXT:    orl %ecx, %eax
+; BMI-NEXT:    movl $-1, %eax
+; BMI-NEXT:    cmovael %ecx, %eax
 ; BMI-NEXT:    retq
 
   %cnt = tail call i32 @llvm.cttz.i32(i32 %v, i1 true)
   %tobool = icmp ne i32 %v, 0
   %sel = select i1 %tobool, i32 %cnt, i32 -1
   ret i32 %sel
 }