[X86] Teach X86TargetLowering::targetShrinkDemandedConstant to set non-demanded bits if it helps created an and mask that can be matched as a zero extend.

[craig.topper]

Summary

I had to hack the bswap recognition to allow unshrunk masks to make this work.

Fixes PR36689.

[niravd]

Is there any chance of the right shift needing this case as well?

In any case, this LGTM modulo nit.

lib/CodeGen/SelectionDAG/DAGCombiner.cpp

4404 ↗

(On Diff #138228)

I think we should put this case right after the 0xFF00 case.

[craig.topper]

The pattern matching detects (and (shift)) and (shift (and)) since we canonicalize the order of shifts and ands. This patch handles 0xffff for (and (srl)) and (shl (and))

llvm/trunk/lib/CodeGen/SelectionDAG/DAGCombiner.cpp

SDValue DAGCombiner::MatchBSwapHWordLow(SDNode *N, SDValue N0, SDValue N1,
   if (N0.getOpcode() == ISD::AND && N0.getOperand(0).getOpcode() == ISD::SRL)
       std::swap(N0, N1);
   if (N1.getOpcode() == ISD::AND && N1.getOperand(0).getOpcode() == ISD::SHL)
       std::swap(N0, N1);
   if (N0.getOpcode() == ISD::AND) {
     if (!N0.getNode()->hasOneUse())
       return SDValue();
     ConstantSDNode *N01C = dyn_cast<ConstantSDNode>(N0.getOperand(1));
-    if (!N01C || N01C->getZExtValue() != 0xFF00)
+    // Also handle 0xffff since the LHS is guaranteed to have zeros there.
+    // This is needed for X86.
+    if (!N01C || (N01C->getZExtValue() != 0xFF00 &&
+                  N01C->getZExtValue() != 0xFFFF))
       return SDValue();
     N0 = N0.getOperand(0);
     LookPassAnd0 = true;
   }
 
   if (N1.getOpcode() == ISD::AND) {
     if (!N1.getNode()->hasOneUse())
       return SDValue();
if (!LookPassAnd0 && N00.getOpcode() == ISD::AND) {
     LookPassAnd0 = true;
   }
 
   SDValue N10 = N1->getOperand(0);
   if (!LookPassAnd1 && N10.getOpcode() == ISD::AND) {
     if (!N10.getNode()->hasOneUse())
       return SDValue();
     ConstantSDNode *N101C = dyn_cast<ConstantSDNode>(N10.getOperand(1));
-    if (!N101C || N101C->getZExtValue() != 0xFF00)
+    // Also allow 0xFFFF since the bits will be shifted out. This is needed
+    // for X86.
+    if (!N101C || (N101C->getZExtValue() != 0xFF00 &&
+                   N101C->getZExtValue() != 0xFFFF))
       return SDValue();
     N10 = N10.getOperand(0);
     LookPassAnd1 = true;
   }
 
   if (N00 != N10)
     return SDValue();
 
if (!N1C)
     return false;
 
   unsigned MaskByteOffset;
   switch (N1C->getZExtValue()) {
   default:
     return false;
   case 0xFF:       MaskByteOffset = 0; break;
   case 0xFF00:     MaskByteOffset = 1; break;
+  case 0xFFFF:
+    // In case demanded bits didn't clear the bits that will be shifted out.
+    // This is needed for X86.
+    if (Opc == ISD::SRL || (Opc == ISD::AND && Opc0 == ISD::SHL)) {
+      MaskByteOffset = 1;
+      break;
+    }
+    return false;
   case 0xFF0000:   MaskByteOffset = 2; break;
   case 0xFF000000: MaskByteOffset = 3; break;
   }
 
   // Look for (x & 0xff) << 8 as well as ((x << 8) & 0xff00).
   if (Opc == ISD::AND) {
     if (MaskByteOffset == 0 || MaskByteOffset == 2) {
       // (x >> 8) & 0xff

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

X86TargetLowering::targetShrinkDemandedConstant(SDValue Op,
   // Calculate a possible zero extend mask for this constant.
   APInt ZeroExtendMask = APInt::getLowBitsSet(Size, Width);
 
   // If we aren't changing the mask, just return true to keep it and prevent
   // the caller from optimizing.
   if (ZeroExtendMask == Mask)
     return true;
 
-  // Make sure the bits in the ZeroExtendMask are also set in the original mask.
-  // TODO: We should be able to set bits that aren't demanded too.
-  if (!ZeroExtendMask.isSubsetOf(Mask))
+  // Make sure the new mask can be represented by a combination of mask bits
+  // and non-demanded bits.
+  if (!ZeroExtendMask.isSubsetOf(Mask | ~Demanded))
     return false;
 
   // Replace the constant with the zero extend mask.
   SDLoc DL(Op);
   SDValue NewC = TLO.DAG.getConstant(ZeroExtendMask, DL, VT);
   SDValue NewOp = TLO.DAG.getNode(ISD::AND, DL, VT, Op.getOperand(0), NewC);
   return TLO.CombineTo(Op, NewOp);
 }

llvm/trunk/test/CodeGen/X86/pr12360.ll

entry:
   %tobool = icmp ne i1 %x, 0
   ret i1 %tobool
 }
 
 ; check that we don't build a trunc when other bits are needed
 define zeroext i1 @f4(i32 %x) {
 ; CHECK-LABEL: f4:
 ; CHECK:       ## %bb.0: ## %entry
-; CHECK-NEXT:    shrl $15, %edi
-; CHECK-NEXT:    andl $1, %edi
-; CHECK-NEXT:    movl %edi, %eax
+; CHECK-NEXT:    movzwl %di, %eax
+; CHECK-NEXT:    shrl $15, %eax
+; CHECK-NEXT:    ## kill: def $al killed $al killed $eax
 ; CHECK-NEXT:    retq
 
 entry:
   %y = and i32 %x, 32768
   %z = icmp ne i32 %y, 0
   ret i1 %z
 }

llvm/trunk/test/CodeGen/X86/pr32284.ll

 ; 686-O0-NEXT:    notl %ecx
 ; 686-O0-NEXT:    testl %eax, %eax
 ; 686-O0-NEXT:    sete %dl
 ; 686-O0-NEXT:    movzbl %dl, %eax
 ; 686-O0-NEXT:    movl var_16, %esi
 ; 686-O0-NEXT:    movl %ecx, %edi
 ; 686-O0-NEXT:    xorl %esi, %edi
 ; 686-O0-NEXT:    andl %edi, %eax
-; 686-O0-NEXT:    movb %al, %dl
-; 686-O0-NEXT:    movzbl %dl, %eax
 ; 686-O0-NEXT:    orl %eax, %ecx
 ; 686-O0-NEXT:    movl %ecx, (%esp)
 ; 686-O0-NEXT:    movl $0, {{[0-9]+}}(%esp)
 ; 686-O0-NEXT:    movl var_13, %eax
 ; 686-O0-NEXT:    notl %eax
 ; 686-O0-NEXT:    movl %eax, var_46
 ; 686-O0-NEXT:    leal -8(%ebp), %esp
 ; 686-O0-NEXT:    popl %esi

 ; 686-NEXT:    movl var_13, %ecx
 ; 686-NEXT:    xorl %eax, %eax
 ; 686-NEXT:    testl %ecx, %ecx
 ; 686-NEXT:    notl %ecx
 ; 686-NEXT:    sete %al
 ; 686-NEXT:    movl var_16, %edx
 ; 686-NEXT:    xorl %ecx, %edx
 ; 686-NEXT:    andl %eax, %edx
-; 686-NEXT:    movzbl %dl, %eax
-; 686-NEXT:    orl %ecx, %eax
-; 686-NEXT:    movl %eax, (%esp)
+; 686-NEXT:    orl %ecx, %edx
+; 686-NEXT:    movl %edx, (%esp)
 ; 686-NEXT:    movl $0, {{[0-9]+}}(%esp)
 ; 686-NEXT:    movl %ecx, var_46
 ; 686-NEXT:    movl %ebp, %esp
 ; 686-NEXT:    popl %ebp
 ; 686-NEXT:    retl
 entry:
   %a = alloca i64, align 8
   %0 = load i32, i32* @var_13, align 4

llvm/trunk/test/CodeGen/X86/zext-demanded.ll

; CHECK-NEXT:    retq
   %z = lshr i32 %y, 9
   ret i32 %z
 }
 
 ; TODO: We could turn this and into a zero extend.
 define i32 @test7(i32 %x) {
 ; CHECK-LABEL: test7:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    # kill: def $edi killed $edi def $rdi
-; CHECK-NEXT:    andl $65534, %edi # imm = 0xFFFE
-; CHECK-NEXT:    leal 1(%rdi), %eax
+; CHECK-NEXT:    orl $1, %edi
+; CHECK-NEXT:    movzwl %di, %eax
 ; CHECK-NEXT:    retq
   %y = and i32 %x, 65534
   %z = or i32 %y, 1
   ret i32 %z
 }
 
 ; We actually get a movzx on this one, but only because we canonicalize the and
 ; after the or before SimplifyDemandedBits messes it up.

 ; CHECK-NEXT:    movzwl %ax, %eax
 ; CHECK-NEXT:    retq
   %a1 = and i64 %x, 65535
   %a2 = mul i64 %x, 65535
   %r = and i64 %a1, %a2
   ret i64 %r
 }
 
+define i32 @PR36689(i32*) {
+; CHECK-LABEL: PR36689:
+; CHECK:       # %bb.0:
+; CHECK-NEXT:    movzwl (%rdi), %eax
+; CHECK-NEXT:    orl $255, %eax
+; CHECK-NEXT:    retq
+  %2 = load i32, i32* %0
+  %3 = and i32 %2, 65280
+  %4 = or i32 %3, 255
+  ret i32 %4
+}