[X86] X86ISelLowering::combineSextInRegCmov(): also handle i8 CMOV's

[lebedev.ri]

Summary

As noted by @andreadb in https://reviews.llvm.org/D59035#inline-525780

If we have sext ([trunc] (cmov C0, C1)), we can instead do cmov (sext ([trunc] (C0))), (sext ([trunc] (C1)))

[lebedev.ri]

test/CodeGen/X86/cmov-promotion.ll

155–157 ↗

(On Diff #190817)

And we now have i16 ops, which i think we want to avoid?
Do we want to do all this in i32, and trunc to i16 in the end?

[craig.topper]

test/CodeGen/X86/cmov-promotion.ll

155–157 ↗

(On Diff #190817)

Yeah. If the VT is i16 we should use i32 instead.

[lebedev.ri]

test/CodeGen/X86/cmov-promotion.ll

155–157 ↗

(On Diff #190817)

Okay, no surprise there, i'll take a look..

[lebedev.ri]

Promote i16 CMOV's to i32, truncate afterwards.

[craig.topper]

LGTM

[lebedev.ri]

In D59412#1431337, @craig.topper wrote:

LGTM

Thank you for the review.
Care to stamp D59035 as well?

llvm/trunk/lib/Target/X86/X86ISelLowering.cpp

static SDValue combineBT(SDNode *N, SelectionDAG &DAG,
   if (SDValue DemandedN1 = DAG.GetDemandedBits(N1, DemandedMask))
     return DAG.getNode(X86ISD::BT, SDLoc(N), MVT::i32, N0, DemandedN1);
 
   return SDValue();
 }
 
 // Try to combine sext_in_reg of a cmov of constants by extending the constants.
 static SDValue combineSextInRegCmov(SDNode *N, SelectionDAG &DAG) {
-  EVT VT = N->getValueType(0);
+  assert(N->getOpcode() == ISD::SIGN_EXTEND_INREG);
+
+  EVT DstVT = N->getValueType(0);
 
   SDValue N0 = N->getOperand(0);
   SDValue N1 = N->getOperand(1);
   EVT ExtraVT = cast<VTSDNode>(N1)->getVT();
 
-  if (ExtraVT != MVT::i16)
+  if (ExtraVT != MVT::i8 && ExtraVT != MVT::i16)
     return SDValue();
 
-  // Look through single use any_extends.
-  if (N0.getOpcode() == ISD::ANY_EXTEND && N0.hasOneUse())
+  // Look through single use any_extends / truncs.
+  SDValue IntermediateBitwidthOp;
+  if ((N0.getOpcode() == ISD::ANY_EXTEND || N0.getOpcode() == ISD::TRUNCATE) &&
+      N0.hasOneUse()) {
+    IntermediateBitwidthOp = N0;
     N0 = N0.getOperand(0);
+  }
 
   // See if we have a single use cmov.
   if (N0.getOpcode() != X86ISD::CMOV || !N0.hasOneUse())
     return SDValue();
 
   SDValue CMovOp0 = N0.getOperand(0);
   SDValue CMovOp1 = N0.getOperand(1);
 
   // Make sure both operands are constants.
   if (!isa<ConstantSDNode>(CMovOp0.getNode()) ||
       !isa<ConstantSDNode>(CMovOp1.getNode()))
     return SDValue();
 
   SDLoc DL(N);
 
-  // If we looked through an any_extend above, add one to the constants.
-  if (N0.getValueType() != VT) {
-    CMovOp0 = DAG.getNode(ISD::ANY_EXTEND, DL, VT, CMovOp0);
-    CMovOp1 = DAG.getNode(ISD::ANY_EXTEND, DL, VT, CMovOp1);
+  // If we looked through an any_extend/trunc above, add one to the constants.
+  if (IntermediateBitwidthOp) {
+    unsigned IntermediateOpc = IntermediateBitwidthOp.getOpcode();
+    CMovOp0 = DAG.getNode(IntermediateOpc, DL, DstVT, CMovOp0);
+    CMovOp1 = DAG.getNode(IntermediateOpc, DL, DstVT, CMovOp1);
   }
 
-  CMovOp0 = DAG.getNode(ISD::SIGN_EXTEND_INREG, DL, VT, CMovOp0, N1);
-  CMovOp1 = DAG.getNode(ISD::SIGN_EXTEND_INREG, DL, VT, CMovOp1, N1);
+  CMovOp0 = DAG.getNode(ISD::SIGN_EXTEND_INREG, DL, DstVT, CMovOp0, N1);
+  CMovOp1 = DAG.getNode(ISD::SIGN_EXTEND_INREG, DL, DstVT, CMovOp1, N1);
 
-  return DAG.getNode(X86ISD::CMOV, DL, VT, CMovOp0, CMovOp1,
+  EVT CMovVT = DstVT;
+  // We do not want i16 CMOV's. Promote to i32 and truncate afterwards.
+  if (DstVT == MVT::i16) {
+    CMovVT = MVT::i32;
+    CMovOp0 = DAG.getNode(ISD::ZERO_EXTEND, DL, CMovVT, CMovOp0);
+    CMovOp1 = DAG.getNode(ISD::ZERO_EXTEND, DL, CMovVT, CMovOp1);
+  }
+
+  SDValue CMov = DAG.getNode(X86ISD::CMOV, DL, CMovVT, CMovOp0, CMovOp1,
                              N0.getOperand(2), N0.getOperand(3));
+
+  if (CMovVT != DstVT)
+    CMov = DAG.getNode(ISD::TRUNCATE, DL, DstVT, CMov);
+
+  return CMov;
 }
 
 static SDValue combineSignExtendInReg(SDNode *N, SelectionDAG &DAG,
                                       const X86Subtarget &Subtarget) {
+  assert(N->getOpcode() == ISD::SIGN_EXTEND_INREG);
+
   if (SDValue V = combineSextInRegCmov(N, DAG))
     return V;
 
   EVT VT = N->getValueType(0);
   SDValue N0 = N->getOperand(0);
   SDValue N1 = N->getOperand(1);
   EVT ExtraVT = cast<VTSDNode>(N1)->getVT();
   SDLoc dl(N);

llvm/trunk/test/CodeGen/X86/cmov-promotion.ll

; NO_CMOV-NEXT:    retl
   %ret = zext i32 %t0 to i64
   ret i64 %ret
 }
 
 define i16 @cmov_spromotion_8_to_16(i1 %c) {
 ; CMOV-LABEL: cmov_spromotion_8_to_16:
 ; CMOV:       # %bb.0:
 ; CMOV-NEXT:    testb $1, %dil
-; CMOV-NEXT:    movl $117, %eax
-; CMOV-NEXT:    movl $237, %ecx
-; CMOV-NEXT:    cmovnel %eax, %ecx
-; CMOV-NEXT:    movsbl %cl, %eax
+; CMOV-NEXT:    movl $117, %ecx
+; CMOV-NEXT:    movl $65517, %eax # imm = 0xFFED
+; CMOV-NEXT:    cmovnel %ecx, %eax
 ; CMOV-NEXT:    # kill: def $ax killed $ax killed $eax
 ; CMOV-NEXT:    retq
 ;
 ; NO_CMOV-LABEL: cmov_spromotion_8_to_16:
 ; NO_CMOV:       # %bb.0:
 ; NO_CMOV-NEXT:    testb $1, {{[0-9]+}}(%esp)
 ; NO_CMOV-NEXT:    movb $117, %al
 ; NO_CMOV-NEXT:    jne .LBB6_2
 ; NO_CMOV-NEXT:  # %bb.1:
 ; NO_CMOV-NEXT:    movb $-19, %al
 ; NO_CMOV-NEXT:  .LBB6_2:
 ; NO_CMOV-NEXT:    movsbl %al, %eax
 ; NO_CMOV-NEXT:    # kill: def $ax killed $ax killed $eax
 ; NO_CMOV-NEXT:    retl
   %t0 = select i1 %c, i8 117, i8 -19
   %ret = sext i8 %t0 to i16
   ret i16 %ret
 }
 
 define i32 @cmov_spromotion_8_to_32(i1 %c) {
 ; CMOV-LABEL: cmov_spromotion_8_to_32:
 ; CMOV:       # %bb.0:
 ; CMOV-NEXT:    testb $1, %dil
-; CMOV-NEXT:    movl $126, %eax
-; CMOV-NEXT:    movl $255, %ecx
-; CMOV-NEXT:    cmovnel %eax, %ecx
-; CMOV-NEXT:    movsbl %cl, %eax
+; CMOV-NEXT:    movl $126, %ecx
+; CMOV-NEXT:    movl $-1, %eax
+; CMOV-NEXT:    cmovnel %ecx, %eax
 ; CMOV-NEXT:    retq
 ;
 ; NO_CMOV-LABEL: cmov_spromotion_8_to_32:
 ; NO_CMOV:       # %bb.0:
 ; NO_CMOV-NEXT:    testb $1, {{[0-9]+}}(%esp)
 ; NO_CMOV-NEXT:    movb $126, %al
 ; NO_CMOV-NEXT:    jne .LBB7_2
 ; NO_CMOV-NEXT:  # %bb.1:
 ; NO_CMOV-NEXT:    movb $-1, %al
 ; NO_CMOV-NEXT:  .LBB7_2:
 ; NO_CMOV-NEXT:    movsbl %al, %eax
 ; NO_CMOV-NEXT:    retl
   %t0 = select i1 %c, i8 12414, i8 -1
   %ret = sext i8 %t0 to i32
   ret i32 %ret
 }
 
 define i64 @cmov_spromotion_8_to_64(i1 %c) {
 ; CMOV-LABEL: cmov_spromotion_8_to_64:
 ; CMOV:       # %bb.0:
 ; CMOV-NEXT:    testb $1, %dil
-; CMOV-NEXT:    movl $126, %eax
-; CMOV-NEXT:    movl $255, %ecx
-; CMOV-NEXT:    cmovnel %eax, %ecx
-; CMOV-NEXT:    movsbq %cl, %rax
+; CMOV-NEXT:    movl $126, %ecx
+; CMOV-NEXT:    movq $-1, %rax
+; CMOV-NEXT:    cmovneq %rcx, %rax
 ; CMOV-NEXT:    retq
 ;
 ; NO_CMOV-LABEL: cmov_spromotion_8_to_64:
 ; NO_CMOV:       # %bb.0:
 ; NO_CMOV-NEXT:    testb $1, {{[0-9]+}}(%esp)
 ; NO_CMOV-NEXT:    movb $126, %al
 ; NO_CMOV-NEXT:    jne .LBB8_2
 ; NO_CMOV-NEXT:  # %bb.1: