[DAGCombiner] Add vector demanded elements support to computeKnownBits

[RKSimon]

Summary

Currently computeKnownBits returns the common known zero/one bits for all elements of vector data, when we may only be interested in one/some of the elements.

This patch adds a DemandedElts argument that allows us to specify the elements we actually care about. The original computeKnownBits implementation calls with a DemandedElts demanding all elements to match current behaviour. Scalar types set this to 1.

The approach was found to be easier than trying to add a per-element known bits solution, for a similar usefulness given the combines where computeKnownBits is typically used.

I've only added support for a few opcodes so far (the ones that have proven straightforward to test), all others will default to demanding all elements but can be updated in due course.

I could add DemandedElts support to computeKnownBitsForTargetNode in this patch if required (this could be useful for x86 target shuffles...).

[efriedma]

Maybe it would make more sense to do this in the IR version of computeKnownBits first? We should prefer to perform optimizations on IR where possible.

I think this approach makes sense; we don't want to track each individual bit of a very wide vector.

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

2101

Need to check that SubIdx is less than the number of elements in the vector.

2549

You need to check whether ConstEltNo is less than the number of elements in the vector.

[RKSimon]

Added the asserts suggested by Eli.

Improved VECTOR_SHUFFLE demanded elts handling - as long as the shuffle mask element isn't UNDEF then we can use the input demanded elts. This might be too pessimistic and we should be able to ignore UNDEFs as long as we don't need the element.

I'll investigate the InstCombine version - I don't know as much about the value tracking code so it might take a little longer.

[efriedma]

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

2550

Are your sure it isn't possible to trigger this assertion? I think DAGCombine will delete an extractelement with an out-of-range index, but you can't depend on that happening before this code runs.

[RKSimon]

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

2550

OK thanks - I misunderstood your request for a check - I'll fix it.

[RKSimon]

Improved index checking for EXTRACT_SUBVECTOR and EXTRACT_VECTOR_ELT.

Use APInt::[] bit getter instead of doing it manually.

Use APInt::setBit bit setter instead of doing it manually.

[zvi]

In D25691#572643, @RKSimon wrote:

I'll investigate the InstCombine version - I don't know as much about the value tracking code so it might take a little longer.

At the moment, instcombine can't handle cases such as:

define <4 x i32> @all_zeros_1(i32 %a) {
  %u = insertelement <4 x i32> <i32 undef, i32 0, i32 0, i32 0>, i32 %a, i32 0
  %l = and <4 x i32> %u, <i32 0, i32 -1, i32 -1, i32 -1>
  ret <4 x i32> %l
}

or

define <4 x i32> @redundant_and(i32 %a) {
  %u = insertelement <4 x i32> <i32 undef, i32 0, i32 0, i32 0>, i32 %a, i32 0
  %l = and <4 x i32> %u, <i32 -1, i32 0, i32 0, i32 0>
  ret <4 x i32> %l
}

It would be nice if both instcombine and dagcombine shared the same DemandedBits infrastructure, though i don't have a plan to achieve this.

[RKSimon]

In D25691#577325, @zvi wrote:

It would be nice if both instcombine and dagcombine shared the same DemandedBits infrastructure, though i don't have a plan to achieve this.

Agreed, we should be trying to add similar features were possible. After this patch and we've matured this a little further on the DAGCombiner side implementing an InstCombine equivalent should be quite a bit quicker.

[RKSimon]

ping?

[bjope]

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

2057

Shouldn't we remove this TODO now? Or is that a different idea about the possibility to fetch a set of individual known bits for each element in one request?
If originating value is a vector, then the caller could call computeKnownBits for one demanded element at a time. Doing it for all elements in one go will be compilcated since the recursion could be different for different elements.

2530

I guess this comment needs to be revised, or maybe removed.
And at least the TODO should be removed.

2554

I suggest that you explicitly show that a decision was made to not track a specific element here. And maybe more importantly show that we can not use the original DemandedElts in the recursive call by selecting a new bit mask.
This could be done by making the recursion like this:

computeKnownBits(InVec, KnownZero, KnownOne, APInt::getAllOnesValue(VecVT.getVectorNumElements()), Depth + 1);

2561

Any reason for not providing the DemandedElts in this recursive call?

[bjope]

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

2561

Sorry, I just realised that BSWAP wasn't the only operation not providing DemandedElts in the recursive call.
So I have no special interest in BSWAP. But I guess that there is a general TODO to pass DemandedElts in many more recursive calls.

[RKSimon]

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

2057

What that TODO is talking about is returning separate KnownZero and KnownOne values for every vector element and not combining them down to the 'shared' bits. Something that could end up requiring a significant refactor of this function and every caller.

This TODO was written when I still thought that would be necessary, but I haven't been able to make a strong enough case for that approach yet and just using the shared bits of demanded elts might be satisfactory for the cases I have so far.

2530

No problem.

2554

This is what the computeKnownBits without the DemandedElts argument now does. I'll tweak the comments to make this clearer.

2561

As I mentioned in the summary, I've only added support for a few opcodes so far (the ones that have proven straightforward to test). The hope is that others can be added as they are required. I'd prefer to just add a TODO to the top of the function and to every outstanding opcode, unless you think it necessary?

[RKSimon]

Updated based on Bjorn's comments

[bjope]

No further comments from me.

But I agree with earlier comments that it could be nice if the computeKnownBits (and probably also computeNumSignBits) in both InstCombine and SelectionDAG could share same infrastructure. I guess that sometimes we want to do similar updates to those methods, and that is probably easier if they at least are kept fairly in sync about how they work and what operations they can handle.
But changing everything in one commit is not so wise either. So this commit looks like a good first step when it comes to adding support for "demended elts".

[RKSimon]

ping?

[bjope]

LGTM

include/llvm/CodeGen/SelectionDAG.h

public:
 
   /// Return true if 'Op & Mask' is known to be zero.  We
   /// use this predicate to simplify operations downstream.  Op and Mask are
   /// known to be the same type.
   bool MaskedValueIsZero(SDValue Op, const APInt &Mask, unsigned Depth = 0)
     const;
 
   /// Determine which bits of Op are known to be either zero or one and return
-  /// them in the KnownZero/KnownOne bitsets.  Targets can implement the
-  /// computeKnownBitsForTargetNode method in the TargetLowering class to allow
-  /// target nodes to be understood.
+  /// them in the KnownZero/KnownOne bitsets. For vectors, the known bits are
+  /// those that are shared by every vector element.
+  /// Targets can implement the computeKnownBitsForTargetNode method in the
+  /// TargetLowering class to allow target nodes to be understood.
   void computeKnownBits(SDValue Op, APInt &KnownZero, APInt &KnownOne,
                         unsigned Depth = 0) const;
 
+  /// Determine which bits of Op are known to be either zero or one and return
+  /// them in the KnownZero/KnownOne bitsets. The DemandedElts argument allows
+  /// us to only collect the known bits that are shared by the requested vector
+  /// elements.
+  /// Targets can implement the computeKnownBitsForTargetNode method in the
+  /// TargetLowering class to allow target nodes to be understood.
+  void computeKnownBits(SDValue Op, APInt &KnownZero, APInt &KnownOne,
+                        const APInt &DemandedElts, unsigned Depth = 0) const;
+
   /// Test if the given value is known to have exactly one bit set. This differs
   /// from computeKnownBits in that it doesn't necessarily determine which bit
   /// is set.
   bool isKnownToBeAPowerOfTwo(SDValue Val) const;
 
   /// Return the number of times the sign bit of the register is replicated into
   /// the other bits. We know that at least 1 bit is always equal to the sign
   /// bit (itself), but other cases can give us information. For example,

lib/CodeGen/SelectionDAG/SelectionDAG.cpp

 bool SelectionDAG::MaskedValueIsZero(SDValue Op, const APInt &Mask,
                                      unsigned Depth) const {
   APInt KnownZero, KnownOne;
   computeKnownBits(Op, KnownZero, KnownOne, Depth);
   return (KnownZero & Mask) == Mask;
 }
 
 /// Determine which bits of Op are known to be either zero or one and return
-/// them in the KnownZero/KnownOne bitsets.
+/// them in the KnownZero/KnownOne bitsets. For vectors, the known bits are
+/// those that are shared by every vector element.
 void SelectionDAG::computeKnownBits(SDValue Op, APInt &KnownZero,
                                     APInt &KnownOne, unsigned Depth) const {
+  EVT VT = Op.getValueType();
+  APInt DemandedElts = VT.isVector()
+                           ? APInt::getAllOnesValue(VT.getVectorNumElements())
+                           : APInt(1, 1);
+  computeKnownBits(Op, KnownZero, KnownOne, DemandedElts, Depth);
+}
+
+/// Determine which bits of Op are known to be either zero or one and return
+/// them in the KnownZero/KnownOne bitsets. The DemandedElts argument allows
+/// us to only collect the known bits that are shared by the requested vector
+/// elements.
+void SelectionDAG::computeKnownBits(SDValue Op, APInt &KnownZero,
+                                    APInt &KnownOne, const APInt &DemandedElts,
+                                    unsigned Depth) const {
   unsigned BitWidth = Op.getScalarValueSizeInBits();
 
   KnownZero = KnownOne = APInt(BitWidth, 0);   // Don't know anything.
   if (Depth == 6)
     return;  // Limit search depth.
 
   APInt KnownZero2, KnownOne2;
+  unsigned NumElts = DemandedElts.getBitWidth();
+
+  if (DemandedElts == APInt(NumElts, 0))
+    return;  // No demanded elts, better to assume we don't know anything.
 
   switch (Op.getOpcode()) {
   case ISD::Constant:
     // We know all of the bits for a constant!
     KnownOne = cast<ConstantSDNode>(Op)->getAPIntValue();
     KnownZero = ~KnownOne;
     break;
   case ISD::BUILD_VECTOR:
-    // Collect the known bits that are shared by every vector element.
+    // Collect the known bits that are shared by every demanded vector element.
+    assert(NumElts == Op.getValueType().getVectorNumElements() &&
+           "Unexpected vector size");
     KnownZero = KnownOne = APInt::getAllOnesValue(BitWidth);
-    for (SDValue SrcOp : Op->ops()) {
+    for (unsigned i = 0, e = Op.getNumOperands(); i != e; ++i) {
+      if (!(DemandedElts & APInt::getOneBitSet(NumElts, i)))
+        continue;
+
+      SDValue SrcOp = Op.getOperand(i);
       computeKnownBits(SrcOp, KnownZero2, KnownOne2, Depth + 1);
 
       // BUILD_VECTOR can implicitly truncate sources, we must handle this.
       if (SrcOp.getValueSizeInBits() != BitWidth) {
         assert(SrcOp.getValueSizeInBits() > BitWidth &&
                "Expected BUILD_VECTOR implicit truncation");
         KnownOne2 = KnownOne2.trunc(BitWidth);
         KnownZero2 = KnownZero2.trunc(BitWidth);
       }
 
-      // Known bits are the values that are shared by every element.
+      // Known bits are the values that are shared by every demanded element.
       // TODO: support per-element known bits.
       KnownOne &= KnownOne2;
       KnownZero &= KnownZero2;
     }
     break;
+  case ISD::VECTOR_SHUFFLE: {
+    // Collect the known bits that are shared by every vector element referenced
+    // by the shuffle.
+    APInt DemandedLHS(NumElts, 0), DemandedRHS(NumElts, 0);
+    KnownZero = KnownOne = APInt::getAllOnesValue(BitWidth);
+    const ShuffleVectorSDNode *SVN = cast<ShuffleVectorSDNode>(Op);
+    assert(NumElts == SVN->getMask().size() && "Unexpected vector size");
+    for (int M : SVN->getMask()) {
+      if (M < 0) {
+        // For UNDEF elements, we don't know anything about the common state of
+        // the shuffle result.
+        KnownZero = KnownOne = APInt(BitWidth, 0);
+        break;
+      }
+      APInt DemandedElt = APInt::getOneBitSet(NumElts, (unsigned)M % NumElts);
+      if ((unsigned)M < NumElts)
+        DemandedLHS |= DemandedElt;
+      else
+        DemandedRHS |= DemandedElt;
+    }
+    // Known bits are the values that are shared by every demanded element.
+    if (DemandedLHS != APInt(NumElts, 0)) {
+      SDValue LHS = Op.getOperand(0);
+      computeKnownBits(LHS, KnownZero2, KnownOne2, DemandedLHS, Depth + 1);
+      KnownOne &= KnownOne2;
+      KnownZero &= KnownZero2;
+    }
+    if (DemandedRHS != APInt(NumElts, 0)) {
+      SDValue RHS = Op.getOperand(1);
+      computeKnownBits(RHS, KnownZero2, KnownOne2, DemandedRHS, Depth + 1);
+      KnownOne &= KnownOne2;
+      KnownZero &= KnownZero2;
+    }
+    break;
+  }
+  case ISD::EXTRACT_SUBVECTOR: {
+    // Offset the demanded elts by the subvector index.
+    if (ConstantSDNode *SubIdx = dyn_cast<ConstantSDNode>(Op.getOperand(1))) {
+      SDValue Src = Op.getOperand(0);
+      uint64_t Idx = SubIdx->getZExtValue();
+      unsigned NumSrcElts = Src.getValueType().getVectorNumElements();
+      APInt DemandedSrc = DemandedElts.zext(NumSrcElts).shl(Idx);
+      computeKnownBits(Src, KnownZero, KnownOne, DemandedSrc, Depth + 1);
+    }
+    break;
+  }
   case ISD::AND:
     // If either the LHS or the RHS are Zero, the result is zero.
-    computeKnownBits(Op.getOperand(1), KnownZero, KnownOne, Depth+1);
-    computeKnownBits(Op.getOperand(0), KnownZero2, KnownOne2, Depth+1);
+    computeKnownBits(Op.getOperand(1), KnownZero, KnownOne, DemandedElts,
+                     Depth + 1);
+    computeKnownBits(Op.getOperand(0), KnownZero2, KnownOne2, DemandedElts,
+                     Depth + 1);
 
     // Output known-1 bits are only known if set in both the LHS & RHS.
     KnownOne &= KnownOne2;
     // Output known-0 are known to be clear if zero in either the LHS | RHS.
     KnownZero |= KnownZero2;
     break;
   case ISD::OR:
     computeKnownBits(Op.getOperand(1), KnownZero, KnownOne, Depth+1);
case ISD::SIGN_EXTEND_INREG: {
     APInt InputDemandedBits = APInt::getLowBitsSet(BitWidth, EBits);
 
     // If the sign extended bits are demanded, we know that the sign
     // bit is demanded.
     InSignBit = InSignBit.zext(BitWidth);
     if (NewBits.getBoolValue())
       InputDemandedBits |= InSignBit;
 
-    computeKnownBits(Op.getOperand(0), KnownZero, KnownOne, Depth+1);
+    computeKnownBits(Op.getOperand(0), KnownZero, KnownOne, DemandedElts,
+                     Depth + 1);
     KnownOne &= InputDemandedBits;
     KnownZero &= InputDemandedBits;
 
     // If the sign bit of the input is known set or clear, then we know the
     // top bits of the result.
     if (KnownZero.intersects(InSignBit)) {         // Input sign bit known clear
       KnownZero |= NewBits;
       KnownOne  &= ~NewBits;
case ISD::LOAD: {
     break;
   }
   case ISD::ZERO_EXTEND: {
     EVT InVT = Op.getOperand(0).getValueType();
     unsigned InBits = InVT.getScalarSizeInBits();
     APInt NewBits   = APInt::getHighBitsSet(BitWidth, BitWidth - InBits);
     KnownZero = KnownZero.trunc(InBits);
     KnownOne = KnownOne.trunc(InBits);
-    computeKnownBits(Op.getOperand(0), KnownZero, KnownOne, Depth+1);
+    computeKnownBits(Op.getOperand(0), KnownZero, KnownOne, DemandedElts,
+                     Depth + 1);
     KnownZero = KnownZero.zext(BitWidth);
     KnownOne = KnownOne.zext(BitWidth);
     KnownZero |= NewBits;
     break;
   }
   case ISD::SIGN_EXTEND: {
     EVT InVT = Op.getOperand(0).getValueType();
     unsigned InBits = InVT.getScalarSizeInBits();
     APInt NewBits   = APInt::getHighBitsSet(BitWidth, BitWidth - InBits);
 
     KnownZero = KnownZero.trunc(InBits);
     KnownOne = KnownOne.trunc(InBits);
-    computeKnownBits(Op.getOperand(0), KnownZero, KnownOne, Depth+1);
+    computeKnownBits(Op.getOperand(0), KnownZero, KnownOne, DemandedElts,
+                     Depth + 1);
 
     // Note if the sign bit is known to be zero or one.
     bool SignBitKnownZero = KnownZero.isNegative();
     bool SignBitKnownOne  = KnownOne.isNegative();
 
     KnownZero = KnownZero.zext(BitWidth);
     KnownOne = KnownOne.zext(BitWidth);
 
case ISD::EXTRACT_ELEMENT: {
     KnownOne = KnownOne.getHiBits(KnownOne.getBitWidth() - Index * BitWidth);
 
     // Remove high part of known bit mask
     KnownZero = KnownZero.trunc(BitWidth);
     KnownOne = KnownOne.trunc(BitWidth);
     break;
   }
   case ISD::EXTRACT_VECTOR_ELT: {
     // At the moment we keep this simple and skip tracking the specific
     // element. This way we get the lowest common denominator for all elements
     // of the vector.
     // TODO: get information for given vector element
+    SDValue InVec = Op.getOperand(0);
+    SDValue EltNo = Op.getOperand(1);
+    EVT VecVT = InVec.getValueType();
     const unsigned BitWidth = Op.getValueSizeInBits();
-    const unsigned EltBitWidth = Op.getOperand(0).getScalarValueSizeInBits();
+    const unsigned EltBitWidth = VecVT.getScalarSizeInBits();
     // If BitWidth > EltBitWidth the value is anyext:ed. So we do not know
     // anything about the extended bits.
     if (BitWidth > EltBitWidth) {
       KnownZero = KnownZero.trunc(EltBitWidth);
       KnownOne = KnownOne.trunc(EltBitWidth);
     }
-    computeKnownBits(Op.getOperand(0), KnownZero, KnownOne, Depth+1);
+    // If we know the element index, just demand that vector element, otherwise
+    // demand them all.
+    if (ConstantSDNode *ConstEltNo = dyn_cast<ConstantSDNode>(EltNo)) {
+      APInt DemandedElt = APInt::getOneBitSet(VecVT.getVectorNumElements(),
+                                              ConstEltNo->getZExtValue());
+      computeKnownBits(InVec, KnownZero, KnownOne, DemandedElt, Depth + 1);
+    } else {
+      computeKnownBits(InVec, KnownZero, KnownOne, Depth + 1);
+    }
     if (BitWidth > EltBitWidth) {
       KnownZero = KnownZero.zext(BitWidth);
       KnownOne = KnownOne.zext(BitWidth);
     }
     break;
   }
   case ISD::BSWAP: {
     computeKnownBits(Op.getOperand(0), KnownZero2, KnownOne2, Depth+1);
     KnownZero = KnownZero2.byteSwap();
     KnownOne = KnownOne2.byteSwap();
     break;
   }
   case ISD::SMIN:
   case ISD::SMAX:
   case ISD::UMIN:
   case ISD::UMAX: {

test/CodeGen/X86/known-bits-vector.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=i686-unknown-unknown -mattr=+avx | FileCheck %s --check-prefix=X32
 ; RUN: llc < %s -mtriple=x86_64-unknown-unknown -mattr=+avx | FileCheck %s --check-prefix=X64
 
 define i32 @knownbits_mask_extract_sext(<8 x i16> %a0) nounwind {
 ; X32-LABEL: knownbits_mask_extract_sext:
 ; X32:       # BB#0:
-; X32-NEXT:    vandps {{\.LCPI.*}}, %xmm0, %xmm0
-; X32-NEXT:    vmovd %xmm0, %eax
-; X32-NEXT:    cwtl
+; X32-NEXT:    vpand {{\.LCPI.*}}, %xmm0, %xmm0
+; X32-NEXT:    vpextrw $0, %xmm0, %eax
 ; X32-NEXT:    retl
 ;
 ; X64-LABEL: knownbits_mask_extract_sext:
 ; X64:       # BB#0:
-; X64-NEXT:    vandps {{.*}}(%rip), %xmm0, %xmm0
-; X64-NEXT:    vmovd %xmm0, %eax
-; X64-NEXT:    cwtl
+; X64-NEXT:    vpand {{.*}}(%rip), %xmm0, %xmm0
+; X64-NEXT:    vpextrw $0, %xmm0, %eax
 ; X64-NEXT:    retq
   %1 = and <8 x i16> %a0, <i16 15, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1, i16 -1>
   %2 = extractelement <8 x i16> %1, i32 0
   %3 = sext i16 %2 to i32
   ret i32 %3
 }
 
 define float @knownbits_mask_extract_uitofp(<2 x i64> %a0) nounwind {
 ; X32-LABEL: knownbits_mask_extract_uitofp:
 ; X32:       # BB#0:
 ; X32-NEXT:    pushl %ebp
 ; X32-NEXT:    movl %esp, %ebp
 ; X32-NEXT:    andl $-8, %esp
 ; X32-NEXT:    subl $16, %esp
 ; X32-NEXT:    vpxor %xmm1, %xmm1, %xmm1
 ; X32-NEXT:    vpblendw {{.*#+}} xmm0 = xmm0[0],xmm1[1,2,3],xmm0[4,5,6,7]
-; X32-NEXT:    vpextrd $1, %xmm0, %eax
 ; X32-NEXT:    vmovq %xmm0, {{[0-9]+}}(%esp)
-; X32-NEXT:    xorl %ecx, %ecx
-; X32-NEXT:    testl %eax, %eax
-; X32-NEXT:    setns %cl
 ; X32-NEXT:    fildll {{[0-9]+}}(%esp)
-; X32-NEXT:    fadds {{\.LCPI.*}}(,%ecx,4)
 ; X32-NEXT:    fstps {{[0-9]+}}(%esp)
-; X32-NEXT:    vmovss {{.*#+}} xmm0 = mem[0],zero,zero,zero
-; X32-NEXT:    vmovss %xmm0, (%esp)
-; X32-NEXT:    flds (%esp)
+; X32-NEXT:    flds {{[0-9]+}}(%esp)
 ; X32-NEXT:    movl %ebp, %esp
 ; X32-NEXT:    popl %ebp
 ; X32-NEXT:    retl
 ;
 ; X64-LABEL: knownbits_mask_extract_uitofp:
 ; X64:       # BB#0:
 ; X64-NEXT:    vpxor %xmm1, %xmm1, %xmm1
 ; X64-NEXT:    vpblendw {{.*#+}} xmm0 = xmm0[0],xmm1[1,2,3],xmm0[4,5,6,7]
 ; X64-NEXT:    vmovq %xmm0, %rax
-; X64-NEXT:    testq %rax, %rax
-; X64-NEXT:    js .LBB1_1
-; X64-NEXT:  # BB#2:
-; X64-NEXT:    vcvtsi2ssq %rax, %xmm2, %xmm0
-; X64-NEXT:    retq
-; X64-NEXT:  .LBB1_1:
-; X64-NEXT:    movq %rax, %rcx
-; X64-NEXT:    shrq %rcx
-; X64-NEXT:    andl $1, %eax
-; X64-NEXT:    orq %rcx, %rax
 ; X64-NEXT:    vcvtsi2ssq %rax, %xmm2, %xmm0
-; X64-NEXT:    vaddss %xmm0, %xmm0, %xmm0
 ; X64-NEXT:    retq
   %1 = and <2 x i64> %a0, <i64 65535, i64 -1>
   %2 = extractelement <2 x i64> %1, i32 0
   %3 = uitofp i64 %2 to float
   ret float %3
 }
 
 define <4 x i32> @knownbits_mask_shuffle_sext(<8 x i16> %a0) nounwind {
 ; X32-LABEL: knownbits_mask_shuffle_sext:
 ; X32:       # BB#0:
 ; X32-NEXT:    vpand {{\.LCPI.*}}, %xmm0, %xmm0
-; X32-NEXT:    vpshufd {{.*#+}} xmm0 = xmm0[2,3,0,1]
-; X32-NEXT:    vpmovsxwd %xmm0, %xmm0
+; X32-NEXT:    vpxor %xmm1, %xmm1, %xmm1
+; X32-NEXT:    vpunpckhwd {{.*#+}} xmm0 = xmm0[4],xmm1[4],xmm0[5],xmm1[5],xmm0[6],xmm1[6],xmm0[7],xmm1[7]
 ; X32-NEXT:    retl
 ;
 ; X64-LABEL: knownbits_mask_shuffle_sext:
 ; X64:       # BB#0:
 ; X64-NEXT:    vpand {{.*}}(%rip), %xmm0, %xmm0
-; X64-NEXT:    vpshufd {{.*#+}} xmm0 = xmm0[2,3,0,1]
-; X64-NEXT:    vpmovsxwd %xmm0, %xmm0
+; X64-NEXT:    vpxor %xmm1, %xmm1, %xmm1
+; X64-NEXT:    vpunpckhwd {{.*#+}} xmm0 = xmm0[4],xmm1[4],xmm0[5],xmm1[5],xmm0[6],xmm1[6],xmm0[7],xmm1[7]
 ; X64-NEXT:    retq
   %1 = and <8 x i16> %a0, <i16 -1, i16 -1, i16 -1, i16 -1, i16 15, i16 15, i16 15, i16 15>
   %2 = shufflevector <8 x i16> %1, <8 x i16> undef, <4 x i32> <i32 4, i32 5, i32 6, i32 7>
   %3 = sext <4 x i16> %2 to <4 x i32>
   ret <4 x i32> %3
 }
 
 define <4 x float> @knownbits_mask_shuffle_uitofp(<4 x i32> %a0) nounwind {
 ; X32-LABEL: knownbits_mask_shuffle_uitofp:
 ; X32:       # BB#0:
 ; X32-NEXT:    vpand {{\.LCPI.*}}, %xmm0, %xmm0
 ; X32-NEXT:    vpshufd {{.*#+}} xmm0 = xmm0[2,2,3,3]
-; X32-NEXT:    vpblendw {{.*#+}} xmm1 = xmm0[0],mem[1],xmm0[2],mem[3],xmm0[4],mem[5],xmm0[6],mem[7]
-; X32-NEXT:    vpsrld $16, %xmm0, %xmm0
-; X32-NEXT:    vpblendw {{.*#+}} xmm0 = xmm0[0],mem[1],xmm0[2],mem[3],xmm0[4],mem[5],xmm0[6],mem[7]
-; X32-NEXT:    vaddps {{\.LCPI.*}}, %xmm0, %xmm0
-; X32-NEXT:    vaddps %xmm0, %xmm1, %xmm0
+; X32-NEXT:    vcvtdq2ps %xmm0, %xmm0
 ; X32-NEXT:    retl
 ;
 ; X64-LABEL: knownbits_mask_shuffle_uitofp:
 ; X64:       # BB#0:
 ; X64-NEXT:    vpand {{.*}}(%rip), %xmm0, %xmm0
 ; X64-NEXT:    vpshufd {{.*#+}} xmm0 = xmm0[2,2,3,3]
-; X64-NEXT:    vpblendw {{.*#+}} xmm1 = xmm0[0],mem[1],xmm0[2],mem[3],xmm0[4],mem[5],xmm0[6],mem[7]
-; X64-NEXT:    vpsrld $16, %xmm0, %xmm0
-; X64-NEXT:    vpblendw {{.*#+}} xmm0 = xmm0[0],mem[1],xmm0[2],mem[3],xmm0[4],mem[5],xmm0[6],mem[7]
-; X64-NEXT:    vaddps {{.*}}(%rip), %xmm0, %xmm0
-; X64-NEXT:    vaddps %xmm0, %xmm1, %xmm0
+; X64-NEXT:    vcvtdq2ps %xmm0, %xmm0
 ; X64-NEXT:    retq
   %1 = and <4 x i32> %a0, <i32 -1, i32 -1, i32 255, i32 4085>
   %2 = shufflevector <4 x i32> %1, <4 x i32> undef, <4 x i32> <i32 2, i32 2, i32 3, i32 3>
   %3 = uitofp <4 x i32> %2 to <4 x float>
   ret <4 x float> %3
 }