Fix wrong mask generated for _mm_blendv_epi8 (BUG 24532)

[mkuper]

Summary

BUILD_VECTORtoBlendMask never updates second part of returned MaskValue and so it is always all 1s without the notion about the actual value of Lane2Cond.
This patch attempts to fix it; It breaks test/CodeGen/X86/vector-blend.ll, but the testcase itself checks for the incorrect code

[filcab]

Hi Alexander,

It seems like you're on track to solving the bug, but there's some details that I find weird.

Please also:
1 - Fix the test-case that's wrong
2 - Add a test-case

Adding Chandler, just to be sure there's no minute detail we might be forgetting (or for him to tell me the "undef" problem I mentioned is not a problem).

lib/Target/X86/X86ISelLowering.cpp

11105

It looks like the -1 serves as an "undef". In that case, you seem to be bailing out too early.
If Lane1Cond is 1, and Lane2Cond is undef, then we can just do what Lane1Cond says (same for when Lane1Cond is undef, but Lane2Cond is not).

I also prefer the comment above in the place where it was. It explains why the loop loops on NumElemsInLane, and not NumElems.

[mkuper]

I agree with Filipe.
Alexander, if you don't mind, I'll upload a fixed patch myself, this should be fairly simple.

[mkuper]

Posted new patch as D13505.
Note that there are actually two distinct bugs in this code...

[mkuper]

D13505 has been committed, so this is no longer relevant.

lib/Target/X86/X86ISelLowering.cpp

 static bool BUILD_VECTORtoBlendMask(BuildVectorSDNode *BuildVector,
                                     unsigned &MaskValue) {
   MaskValue = 0;
   unsigned NumElems = BuildVector->getNumOperands();
   // There are 2 lanes if (NumElems > 8), and 1 lane otherwise.
   unsigned NumLanes = (NumElems - 1) / 8 + 1;
   unsigned NumElemsInLane = NumElems / NumLanes;
 
-  // Blend for v16i16 should be symmetric for the both lanes.
   for (unsigned i = 0; i < NumElemsInLane; ++i) {
     SDValue EltCond = BuildVector->getOperand(i);
     SDValue SndLaneEltCond =
         (NumLanes == 2) ? BuildVector->getOperand(i + NumElemsInLane) : EltCond;
 
     int Lane1Cond = -1, Lane2Cond = -1;
     if (isa<ConstantSDNode>(EltCond))
       Lane1Cond = !isZero(EltCond);
     if (isa<ConstantSDNode>(SndLaneEltCond))
       Lane2Cond = !isZero(SndLaneEltCond);
 
-    if (Lane1Cond == Lane2Cond || Lane2Cond < 0)
-      // Lane1Cond != 0, means we want the first argument.
-      // Lane1Cond == 0, means we want the second argument.
-      // The encoding of this argument is 0 for the first argument, 1
-      // for the second. Therefore, invert the condition.
-      MaskValue |= !Lane1Cond << i;
-    else if (Lane1Cond < 0)
-      MaskValue |= !Lane2Cond << i;
-    else
-      return false;
+    // Blend for v16i16 should be symmetric for the both lanes.
+    if (Lane1Cond != Lane2Cond)
+      return false;
+
+    MaskValue |= !Lane2Cond << (i + NumElemsInLane);
   }
   return true;
 }
 
 /// \brief Try to lower a VSELECT instruction to a vector shuffle.
 static SDValue lowerVSELECTtoVectorShuffle(SDValue Op,
                                            const X86Subtarget *Subtarget,
                                            SelectionDAG &DAG) {