[SLP] Don't vectorize stores of non-packed types (like i1, i2)

[bjope]

Summary

In the spirit of commit fc783e91e0c0696e (llvm-svn: 248943) we
shouldn't vectorize stores of non-packed types (i.e. types that
has padding between consecutive variables in a scalar layout,
but being packed in a vector layout).

The problem was detected as a miscompile in a downstream test case.

[anton-afanasyev]

Looks good, but could you please precommit test (and rebase) to see actual output difference?

[bjope]

Rebase after pre-committing the test case.

[bjope]

In D94446#2492844, @anton-afanasyev wrote:

Looks good, but could you please precommit test (and rebase) to see actual output difference?

Done. Still ok?

[anton-afanasyev]

Yes, thanks.

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

case Instruction::GetElementPtr: {
 
         buildTree_rec(Operands, Depth + 1, {TE, i});
       }
       return;
     }
     case Instruction::Store: {
       // Check if the stores are consecutive or if we need to swizzle them.
       llvm::Type *ScalarTy = cast<StoreInst>(VL0)->getValueOperand()->getType();
+      // Avoid types that are padded when being allocated as scalars, while
+      // being packed together in a vector (such as i1).
+      if (DL->getTypeSizeInBits(ScalarTy) !=
+          DL->getTypeAllocSizeInBits(ScalarTy)) {
+        BS.cancelScheduling(VL, VL0);
+        newTreeEntry(VL, None /*not vectorized*/, S, UserTreeIdx,
+                     ReuseShuffleIndicies);
+        LLVM_DEBUG(dbgs() << "SLP: Gathering stores of non-packed type.\n");
+        return;
+      }
       // Make sure all stores in the bundle are simple - we can't vectorize
       // atomic or volatile stores.
       SmallVector<Value *, 4> PointerOps(VL.size());
       ValueList Operands(VL.size());
       auto POIter = PointerOps.begin();
       auto OIter = Operands.begin();
       for (Value *V : VL) {
         auto *SI = cast<StoreInst>(V);

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

bb1:                                              ; preds = %entry
   %r  = zext i2 %p2 to i8
   ret i8 %r
 }
 
 declare void @f(i64, i64)
 
 define void @test4(i32 %a, i28* %ptr) {
 ; Check that we do not vectorize types that are padded to a bigger ones.
-; FIXME: This is not correct! See D94446.
 ;
 ; CHECK-LABEL: @test4(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[TRUNC:%.*]] = trunc i32 [[A:%.*]] to i28
 ; CHECK-NEXT:    [[GEP1:%.*]] = getelementptr i28, i28* [[PTR:%.*]], i32 1
 ; CHECK-NEXT:    [[GEP2:%.*]] = getelementptr i28, i28* [[PTR]], i32 2
 ; CHECK-NEXT:    [[GEP3:%.*]] = getelementptr i28, i28* [[PTR]], i32 3
-; CHECK-NEXT:    [[TMP0:%.*]] = insertelement <4 x i28> poison, i28 [[TRUNC]], i32 0
-; CHECK-NEXT:    [[TMP1:%.*]] = insertelement <4 x i28> [[TMP0]], i28 [[TRUNC]], i32 1
-; CHECK-NEXT:    [[TMP2:%.*]] = insertelement <4 x i28> [[TMP1]], i28 [[TRUNC]], i32 2
-; CHECK-NEXT:    [[TMP3:%.*]] = insertelement <4 x i28> [[TMP2]], i28 [[TRUNC]], i32 3
-; CHECK-NEXT:    [[TMP4:%.*]] = bitcast i28* [[PTR]] to <4 x i28>*
-; CHECK-NEXT:    store <4 x i28> [[TMP3]], <4 x i28>* [[TMP4]], align 4
+; CHECK-NEXT:    store i28 [[TRUNC]], i28* [[PTR]], align 4
+; CHECK-NEXT:    store i28 [[TRUNC]], i28* [[GEP1]], align 4
+; CHECK-NEXT:    store i28 [[TRUNC]], i28* [[GEP2]], align 4
+; CHECK-NEXT:    store i28 [[TRUNC]], i28* [[GEP3]], align 4
 ; CHECK-NEXT:    ret void
 ;
 entry:
   %trunc = trunc i32 %a to i28
   %gep1 = getelementptr i28, i28* %ptr, i32 1
   %gep2 = getelementptr i28, i28* %ptr, i32 2
   %gep3 = getelementptr i28, i28* %ptr, i32 3
   store i28 %trunc, i28* %ptr
   store i28 %trunc, i28* %gep1
   store i28 %trunc, i28* %gep2
   store i28 %trunc, i28* %gep3
   ret void
 }