[X86] Don't select (cmp (and, imm), 0) to testw

[craig.topper]

Summary

X86ISelDAGToDAG tries to analyze ANDs compared with 0 to optimize to narrower immediates using subregisters.

I don't think we should be optimizing to 16-bit test instructions. It goes against our normal behavior of promoting i16 operations to i32. It only saves one byte due to the need to add a 0x66 prefix. I think it would also be subject to a length changing prefix penalty in the decoders on Intel CPUs.

[spatel]

I agree that this seems wrong for the general case. What about with -minsize though?

If we're going to remove the block entirely, I'd prefer to leave a comment in its place to explain why we have the hole between 8-bit and 32-bit.

[craig.topper]

Do we have other examples of prefering i16 instructions in minsize?

[spatel]

In D38273#882516, @craig.topper wrote:

Do we have other examples of prefering i16 instructions in minsize?

Found this in X86TargetLowering::EmitCmp():

// Only promote the compare up to I32 if it is a 16 bit operation
// with an immediate.  16 bit immediates are to be avoided.
if ((Op0.getValueType() == MVT::i16 &&
     (isa<ConstantSDNode>(Op0) || isa<ConstantSDNode>(Op1))) &&
    !DAG.getMachineFunction().getFunction()->optForMinSize() &&

[craig.topper]

Qualified with optforminsize instead.

[spatel]

LGTM.

lib/Target/X86/X86ISelDAGToDAG.cpp

if ((N0.getOpcode() == ISD::AND ||
         // Replace SUB|CMP with TEST, since SUB has two outputs while TEST has
         // one, do not call ReplaceAllUsesWith.
         ReplaceUses(SDValue(Node, (Opcode == X86ISD::SUB ? 1 : 0)),
                     SDValue(NewNode, 0));
         CurDAG->RemoveDeadNode(Node);
         return;
       }
 
-      // For example, "testl %eax, $32776" to "testw %ax, $32776".
-      if (isUInt<16>(Mask) && N0.getValueType() != MVT::i16 &&
-          (!(Mask & 0x8000) || hasNoSignedComparisonUses(Node))) {
-        SDValue Imm = CurDAG->getTargetConstant(Mask, dl, MVT::i16);
-        SDValue Reg = N0.getOperand(0);
-
-        // Extract the 16-bit subregister.
-        SDValue Subreg = CurDAG->getTargetExtractSubreg(X86::sub_16bit, dl,
-                                                        MVT::i16, Reg);
-
-        // Emit a testw.
-        SDNode *NewNode = CurDAG->getMachineNode(X86::TEST16ri, dl, MVT::i32,
-                                                 Subreg, Imm);
-        // Replace SUB|CMP with TEST, since SUB has two outputs while TEST has
-        // one, do not call ReplaceAllUsesWith.
-        ReplaceUses(SDValue(Node, (Opcode == X86ISD::SUB ? 1 : 0)),
-                    SDValue(NewNode, 0));
-        CurDAG->RemoveDeadNode(Node);
-        return;
-      }
-
       // For example, "testq %rax, $268468232" to "testl %eax, $268468232".
       if (isUInt<32>(Mask) && N0.getValueType() == MVT::i64 &&
           (!(Mask & 0x80000000) || hasNoSignedComparisonUses(Node))) {
         SDValue Imm = CurDAG->getTargetConstant(Mask, dl, MVT::i32);
         SDValue Reg = N0.getOperand(0);
 
         // Extract the 32-bit subregister.
         SDValue Subreg = CurDAG->getTargetExtractSubreg(X86::sub_32bit, dl,

test/CodeGen/X86/tbm_patterns.ll

; CHECK-NEXT:    retq
   %t3 = select i1 %t2, i32 %b, i32 %t1
   ret i32 %t3
 }
 
 define i32 @test_x86_tbm_bextri_u32_z2(i32 %a, i32 %b, i32 %c) nounwind {
 ; CHECK-LABEL: test_x86_tbm_bextri_u32_z2:
 ; CHECK:       # BB#0:
 ; CHECK-NEXT:    shrl $4, %edi
-; CHECK-NEXT:    testw $4095, %di # imm = 0xFFF
+; CHECK-NEXT:    testl $4095, %edi # imm = 0xFFF
 ; CHECK-NEXT:    cmovnel %edx, %esi
 ; CHECK-NEXT:    movl %esi, %eax
 ; CHECK-NEXT:    retq
   %t0 = lshr i32 %a, 4
   %t1 = and i32 %t0, 4095
   %t2 = icmp eq i32 %t1, 0
   %t3 = select i1 %t2, i32 %b, i32 %c
   ret i32 %t3
; CHECK-NEXT:    retq
   %t3 = select i1 %t2, i64 %b, i64 %t1
   ret i64 %t3
 }
 
 define i64 @test_x86_tbm_bextri_u64_z2(i64 %a, i64 %b, i64 %c) nounwind {
 ; CHECK-LABEL: test_x86_tbm_bextri_u64_z2:
 ; CHECK:       # BB#0:
 ; CHECK-NEXT:    shrl $4, %edi
-; CHECK-NEXT:    testw $4095, %di # imm = 0xFFF
+; CHECK-NEXT:    testl $4095, %edi # imm = 0xFFF
 ; CHECK-NEXT:    cmovneq %rdx, %rsi
 ; CHECK-NEXT:    movq %rsi, %rax
 ; CHECK-NEXT:    retq
   %t0 = lshr i64 %a, 4
   %t1 = and i64 %t0, 4095
   %t2 = icmp eq i64 %t1, 0
   %t3 = select i1 %t2, i64 %b, i64 %c
   ret i64 %t3

test/CodeGen/X86/test-shrink-bug.ll

 ; RUN: llc < %s | FileCheck %s
 
 ; Codegen shouldn't reduce the comparison down to testb $-1, %al
 ; because that changes the result of the signed test.
 ; PR5132
-; CHECK: testw  $255, %ax
+; CHECK: testl  $255, %eax
 
 target datalayout = "e-p:32:32:32-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:32:64-f32:32:32-f64:32:64-v64:64:64-v128:128:128-a0:0:64-f80:128:128"
 target triple = "i386-apple-darwin10.0"
 
 @g_14 = global i8 -6, align 1                     ; <i8*> [#uses=1]
 
 declare i32 @func_16(i8 signext %p_19, i32 %p_20) nounwind
 

test/CodeGen/X86/test-shrink.ll

 
 yes:
   call void @bar()
   ret void
 no:
   ret void
 }
 ; CHECK-64-LABEL: g64x16:
-; CHECK-64:   testw $-32640, %[[A0W:di|cx]]
+; CHECK-64:   testl $32896, %[[A0D:edi|ecx]]
 ; CHECK-64:   ret
 ; CHECK-32-LABEL: g64x16:
-; CHECK-32:   testw $-32640, %ax
+; CHECK-32:   testl $32896, %eax
 ; CHECK-32:   ret
 define void @g64x16(i64 inreg %x) nounwind {
   %t = and i64 %x, 32896
   %s = icmp eq i64 %t, 0
   br i1 %s, label %yes, label %no
 
 yes:
   call void @bar()
   ret void
 no:
   ret void
 }
 ; CHECK-64-LABEL: g32x16:
-; CHECK-64:   testw $-32640, %[[A0W]]
+; CHECK-64:   testl $32896, %[[A0D]]
 ; CHECK-64:   ret
 ; CHECK-32-LABEL: g32x16:
-; CHECK-32:   testw $-32640, %ax
+; CHECK-32:   testl $32896, %eax
 ; CHECK-32:   ret
 define void @g32x16(i32 inreg %x) nounwind {
   %t = and i32 %x, 32896
   %s = icmp eq i32 %t, 0
   br i1 %s, label %yes, label %no
 
 yes:
   call void @bar()
   ret void
 no:
   ret void
 }
 ; CHECK-64-LABEL: g64x32:
-; CHECK-64:   testl $268468352, %e[[A0W]]
+; CHECK-64:   testl $268468352, %[[A0D]]
 ; CHECK-64:   ret
 ; CHECK-32-LABEL: g64x32:
 ; CHECK-32:   testl $268468352, %eax
 ; CHECK-32:   ret
 define void @g64x32(i64 inreg %x) nounwind {
   %t = and i64 %x, 268468352
   %s = icmp eq i64 %t, 0
   br i1 %s, label %yes, label %no