[InstCombine] Update instcombine for vectorOps to use new shufflevector semantics

[ManuelJBrito]

Summary

This patch updates the transformations in InstCombineVectorOps to use the new shufflevector semantics that say that undefined values in the mask yield poison.

To prevent miscompilations we have to match with m_Poison instead of m_Undef. Otherwise, we might introduce poison where there was previously undef.

[nlopes]

LGTM, thanks!

[nikic]

llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp

1263

I don't understand what is going on in this conditional. Why do we need the undef check here? We already checked above that either the base operand is poison or all elements are present.

[ManuelJBrito]

llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp

1263

Yes you are correct. It's redundant. Thanks !

[ManuelJBrito]

Remove redundant check

[nikic]

LGTM

llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp

if (InsertElementInst *IEI = dyn_cast<InsertElementInst>(V)) {
     Value *VecOp    = IEI->getOperand(0);
     Value *ScalarOp = IEI->getOperand(1);
     Value *IdxOp    = IEI->getOperand(2);
 
     if (!isa<ConstantInt>(IdxOp))
       return false;
     unsigned InsertedIdx = cast<ConstantInt>(IdxOp)->getZExtValue();
 
-    if (isa<UndefValue>(ScalarOp)) {  // inserting undef into vector.
+    if (isa<PoisonValue>(ScalarOp)) {  // inserting poison into vector.
       // We can handle this if the vector we are inserting into is
       // transitively ok.
       if (collectSingleShuffleElements(VecOp, LHS, RHS, Mask)) {
-        // If so, update the mask to reflect the inserted undef.
+        // If so, update the mask to reflect the inserted poison.
         Mask[InsertedIdx] = -1;
         return true;
       }
     } else if (ExtractElementInst *EI = dyn_cast<ExtractElementInst>(ScalarOp)){
       if (isa<ConstantInt>(EI->getOperand(1))) {
         unsigned ExtractedIdx =
         cast<ConstantInt>(EI->getOperand(1))->getZExtValue();
         unsigned NumLHSElts =
while (CurrIE) {
     FirstIE = CurrIE;
     CurrIE = NextIE;
   }
 
   // If this is just a single insertelement (not a sequence), we are done.
   if (FirstIE == &InsElt)
     return nullptr;
 
-  // If we are not inserting into an undef vector, make sure we've seen an
+  // If we are not inserting into a poison vector, make sure we've seen an
   // insert into every element.
   // TODO: If the base vector is not undef, it might be better to create a splat
   //       and then a select-shuffle (blend) with the base vector.
-  if (!match(FirstIE->getOperand(0), m_Undef()))
+  if (!match(FirstIE->getOperand(0), m_Poison()))
     if (!ElementPresent.all())
       return nullptr;
 
   // Create the insert + shuffle.
   Type *Int32Ty = Type::getInt32Ty(InsElt.getContext());
   PoisonValue *PoisonVec = PoisonValue::get(VecTy);
   Constant *Zero = ConstantInt::get(Int32Ty, 0);
   if (!cast<ConstantInt>(FirstIE->getOperand(2))->isZero())
     FirstIE = InsertElementInst::Create(PoisonVec, SplatVal, Zero, "", &InsElt);
 
-  // Splat from element 0, but replace absent elements with undef in the mask.
+  // Splat from element 0, but replace absent elements with poison in the mask.
   SmallVector<int, 16> Mask(NumElements, 0);
   for (unsigned i = 0; i != NumElements; ++i)
     if (!ElementPresent[i])
       Mask[i] = -1;
 
   return new ShuffleVectorInst(FirstIE, Mask);
 }
 
 /// Try to fold an insert element into an existing splat shuffle by changing
 /// the shuffle's mask to include the index of this insert element.
 static Instruction *foldInsEltIntoSplat(InsertElementInst &InsElt) {
   // Check if the vector operand of this insert is a canonical splat shuffle.
   auto *Shuf = dyn_cast<ShuffleVectorInst>(InsElt.getOperand(0));
Instruction *InstCombinerImpl::visitShuffleVectorInst(ShuffleVectorInst &SVI) {
   SmallVector<int, 16> newMask;
   bool isSplat = true;
   int SplatElt = -1;
   // Create a new mask for the new ShuffleVectorInst so that the new
   // ShuffleVectorInst is equivalent to the original one.
   for (unsigned i = 0; i < VWidth; ++i) {
     int eltMask;
     if (Mask[i] < 0) {
-      // This element is an undef value.
+      // This element is a poison value.
       eltMask = -1;
     } else if (Mask[i] < (int)LHSWidth) {
       // This element is from left hand side vector operand.
       //
       // If LHS is going to be replaced (case 1, 2, or 4), calculate the
       // new mask value for the element.
       if (newLHS != LHS) {
         eltMask = LHSMask[Mask[i]];
-        // If the value selected is an undef value, explicitly specify it
+        // If the value selected is an poison value, explicitly specify it
         // with a -1 mask value.
-        if (eltMask >= (int)LHSOp0Width && isa<UndefValue>(LHSOp1))
+        if (eltMask >= (int)LHSOp0Width && isa<PoisonValue>(LHSOp1))
           eltMask = -1;
       } else
         eltMask = Mask[i];
     } else {
       // This element is from right hand side vector operand
       //
-      // If the value selected is an undef value, explicitly specify it
+      // If the value selected is a poison value, explicitly specify it
       // with a -1 mask value. (case 1)
-      if (match(RHS, m_Undef()))
+      if (match(RHS, m_Poison()))
         eltMask = -1;
       // If RHS is going to be replaced (case 3 or 4), calculate the
       // new mask value for the element.
       else if (newRHS != RHS) {
         eltMask = RHSMask[Mask[i]-LHSWidth];
-        // If the value selected is an undef value, explicitly specify it
+        // If the value selected is an poison value, explicitly specify it
         // with a -1 mask value.
         if (eltMask >= (int)RHSOp0Width) {
-          assert(match(RHSShuffle->getOperand(1), m_Undef()) &&
+          assert(match(RHSShuffle->getOperand(1), m_Poison()) &&
                  "should have been check above");
           eltMask = -1;
         }
       } else
         eltMask = Mask[i]-LHSWidth;
 
       // If LHS's width is changed, shift the mask value accordingly.
       // If newRHS == nullptr, i.e. LHSOp0 == RHSOp0, we want to remap any
for (unsigned i = 0; i < VWidth; ++i) {
 
     newMask.push_back(eltMask);
   }
 
   // If the result mask is equal to one of the original shuffle masks,
   // or is a splat, do the replacement.
   if (isSplat || newMask == LHSMask || newMask == RHSMask || newMask == Mask) {
     if (!newRHS)
-      newRHS = UndefValue::get(newLHS->getType());
+      newRHS = PoisonValue::get(newLHS->getType());
     return new ShuffleVectorInst(newLHS, newRHS, newMask);
   }
 
   return MadeChange ? &SVI : nullptr;
 }

llvm/test/Transforms/InstCombine/broadcast.ll

;
   %res = fadd <4 x float> %a1, %ins4
   ret <4 x float> %res
 }
 
 ; The insert is changed to allow the canonical shuffle-splat pattern from element 0.
 
 define <4 x float> @splat_undef1(float %arg) {
 ; CHECK-LABEL: @splat_undef1(
-; CHECK-NEXT:    [[TMP1:%.*]] = insertelement <4 x float> poison, float [[ARG:%.*]], i64 0
-; CHECK-NEXT:    [[T6:%.*]] = shufflevector <4 x float> [[TMP1]], <4 x float> poison, <4 x i32> <i32 poison, i32 0, i32 0, i32 0>
+; CHECK-NEXT:    [[T4:%.*]] = insertelement <4 x float> undef, float [[ARG:%.*]], i64 1
+; CHECK-NEXT:    [[T5:%.*]] = insertelement <4 x float> [[T4]], float [[ARG:%.*]], i64 2
+; CHECK-NEXT:    [[T6:%.*]] = insertelement <4 x float> [[T5]], float [[ARG:%.*]], i64 3
 ; CHECK-NEXT:    ret <4 x float> [[T6]]
 ;
   %t = insertelement <4 x float> undef, float %arg, i32 1
   %t4 = insertelement <4 x float> %t, float %arg, i32 1
   %t5 = insertelement <4 x float> %t4, float %arg, i32 2
   %t6 = insertelement <4 x float> %t5, float %arg, i32 3
   ret <4 x float> %t6
 }
 
 ; Re-uses the existing first insertelement.
 
 define <4 x float> @splat_undef2(float %arg) {
 ; CHECK-LABEL: @splat_undef2(
 ; CHECK-NEXT:    [[T:%.*]] = insertelement <4 x float> undef, float [[ARG:%.*]], i64 0
-; CHECK-NEXT:    [[T6:%.*]] = shufflevector <4 x float> [[T]], <4 x float> poison, <4 x i32> <i32 0, i32 poison, i32 0, i32 0>
+; CHECK-NEXT:    [[T5:%.*]] = insertelement <4 x float> [[T]], float [[ARG:%.*]], i64 2
+; CHECK-NEXT:    [[T6:%.*]] = insertelement <4 x float> [[T5]], float [[ARG:%.*]], i64 3
 ; CHECK-NEXT:    ret <4 x float> [[T6]]
 ;
   %t = insertelement <4 x float> undef, float %arg, i32 0
   %t5 = insertelement <4 x float> %t, float %arg, i32 2
   %t6 = insertelement <4 x float> %t5, float %arg, i32 3
   ret <4 x float> %t6
 }
 

 }
 
 ; Multiple undef elements are ok.
 ; TODO: Multiple uses triggers the transform at %t4, but we should sink/scalarize/CSE the splats?
 
 define <4 x float> @splat_undef3(float %arg) {
 ; CHECK-LABEL: @splat_undef3(
 ; CHECK-NEXT:    [[T:%.*]] = insertelement <4 x float> undef, float [[ARG:%.*]], i64 0
-; CHECK-NEXT:    [[T4:%.*]] = shufflevector <4 x float> [[T]], <4 x float> poison, <4 x i32> <i32 0, i32 0, i32 poison, i32 poison>
-; CHECK-NEXT:    [[T6:%.*]] = shufflevector <4 x float> [[T]], <4 x float> poison, <4 x i32> zeroinitializer
+; CHECK-NEXT:    [[T4:%.*]] = insertelement <4 x float> [[T]], float [[ARG:%.*]], i64 1
+; CHECK-NEXT:    [[T5:%.*]] = insertelement <4 x float> [[T4]], float [[ARG:%.*]], i64 2
+; CHECK-NEXT:    [[T6:%.*]] = insertelement <4 x float> [[T5]], float [[ARG:%.*]], i64 3
 ; CHECK-NEXT:    [[T7:%.*]] = fadd <4 x float> [[T6]], [[T4]]
 ; CHECK-NEXT:    ret <4 x float> [[T7]]
 ;
   %t = insertelement <4 x float> undef, float %arg, i32 0
   %t4 = insertelement <4 x float> %t, float %arg, i32 1
   %t5 = insertelement <4 x float> %t4, float %arg, i32 2
   %t6 = insertelement <4 x float> %t5, float %arg, i32 3
   %t7 = fadd <4 x float> %t6, %t4

llvm/test/Transforms/InstCombine/vec_shuffle-inseltpoison.ll

;
   %t7 = shufflevector <4 x i32> %t6, <4 x i32> poison, <4 x i32> zeroinitializer
   ret <4 x i32> %t7
 }
 
 ; Test fold of two shuffles where the two shufflevector inputs's op1 are the same.
 
 define <8 x i8> @test11(<16 x i8> %t6) {
 ; CHECK-LABEL: @test11(
-; CHECK-NEXT:    [[T3:%.*]] = shufflevector <16 x i8> [[T6:%.*]], <16 x i8> undef, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7>
+; CHECK-NEXT:    [[T3:%.*]] = shufflevector <16 x i8> [[T6:%.*]], <16 x i8> poison, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7>
 ; CHECK-NEXT:    ret <8 x i8> [[T3]]
 ;
   %t1 = shufflevector <16 x i8> %t6, <16 x i8> poison, <4 x i32> <i32 0, i32 1, i32 2, i32 3>
   %t2 = shufflevector <16 x i8> %t6, <16 x i8> poison, <4 x i32> <i32 4, i32 5, i32 6, i32 7>
   %t3 = shufflevector <4 x i8> %t1, <4 x i8> %t2, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7>
   ret <8 x i8> %t3
 }
 

llvm/test/Transforms/InstCombine/vec_shuffle.ll

;
   %t7 = shufflevector <4 x i32> %t6, <4 x i32> undef, <4 x i32> zeroinitializer
   ret <4 x i32> %t7
 }
 
 ; Test fold of two shuffles where the two shufflevector inputs's op1 are the same.
 
 define <8 x i8> @test11(<16 x i8> %t6) {
 ; CHECK-LABEL: @test11(
-; CHECK-NEXT:    [[T3:%.*]] = shufflevector <16 x i8> [[T6:%.*]], <16 x i8> undef, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7>
+; CHECK-NEXT:    [[T3:%.*]] = shufflevector <16 x i8> [[T6:%.*]], <16 x i8> poison, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7>
 ; CHECK-NEXT:    ret <8 x i8> [[T3]]
 ;
   %t1 = shufflevector <16 x i8> %t6, <16 x i8> undef, <4 x i32> <i32 0, i32 1, i32 2, i32 3>
   %t2 = shufflevector <16 x i8> %t6, <16 x i8> undef, <4 x i32> <i32 4, i32 5, i32 6, i32 7>
   %t3 = shufflevector <4 x i8> %t1, <4 x i8> %t2, <8 x i32> <i32 0, i32 1, i32 2, i32 3, i32 4, i32 5, i32 6, i32 7>
   ret <8 x i8> %t3
 }
 

llvm/test/Transforms/SLPVectorizer/X86/pr49081.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -mtriple=x86_64-- -passes=slp-vectorizer,instcombine -S < %s | FileCheck %s
 
 ; These conversions should be vectorized by reviews.llvm.org/D57059
 
 define dso_local <4 x float> @foo(<4 x i32> %0) {
 ; CHECK-LABEL: @foo(
 ; CHECK-NEXT:    [[TMP2:%.*]] = extractelement <4 x i32> [[TMP0:%.*]], i64 1
 ; CHECK-NEXT:    [[TMP3:%.*]] = sitofp i32 [[TMP2]] to float
 ; CHECK-NEXT:    [[TMP4:%.*]] = insertelement <4 x float> undef, float [[TMP3]], i64 0
-; CHECK-NEXT:    [[TMP5:%.*]] = shufflevector <4 x float> [[TMP4]], <4 x float> poison, <4 x i32> <i32 0, i32 0, i32 poison, i32 poison>
+; CHECK-NEXT:    [[TMP5:%.*]] = insertelement <4 x float> [[TMP4]], float [[TMP3]], i64 1
 ; CHECK-NEXT:    [[TMP6:%.*]] = shufflevector <4 x i32> [[TMP0]], <4 x i32> poison, <2 x i32> <i32 2, i32 3>
 ; CHECK-NEXT:    [[TMP7:%.*]] = sitofp <2 x i32> [[TMP6]] to <2 x float>
 ; CHECK-NEXT:    [[TMP8:%.*]] = shufflevector <2 x float> [[TMP7]], <2 x float> poison, <4 x i32> <i32 0, i32 1, i32 poison, i32 poison>
 ; CHECK-NEXT:    [[TMP9:%.*]] = shufflevector <4 x float> [[TMP5]], <4 x float> [[TMP8]], <4 x i32> <i32 0, i32 1, i32 4, i32 5>
 ; CHECK-NEXT:    ret <4 x float> [[TMP9]]
 ;
   %2 = extractelement <4 x i32> %0, i32 1
   %3 = sitofp i32 %2 to float