[SLP]Adjust GEP indices types when trying to build entries.

[ABataev]

Summary

Need to adjust the types of GEPs indices when building the tree
entries/operands. Otherwise some of the nodes might differ and
vectorizer is unable to correctly find them and count their cost.

[RKSimon]

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

3876

SmallVector<ValueList> Operands(1, ValueList());

[anton-afanasyev]

Add test showing different costs?

[ABataev]

In D113792#3129920, @anton-afanasyev wrote:

Add test showing different costs?

There is no cost difference, just the tests are crashing.

[anton-afanasyev]

There is no cost difference, just the tests are crashing.

Ok, I see, so add such test to this commit?

[ABataev]

In D113792#3129941, @anton-afanasyev wrote:

There is no cost difference, just the tests are crashing.

Ok, I see, so add such test to this commit?

We already have these tests. Just before we did this adjustment during vector code emission but instead we should do it when building the tree entry. I have some other patches that depend on this one.

[ABataev]

Rebase + address comments

[RKSimon]

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

6315–6316

auto *GEP0 = cast<GetElementPtrInst>(VL0);

6318

for (int J = 1, N = GEP0->getNumOperands(); J < N; ++J) {

6322

Value *V = Builder.CreateGEP(GEP0->getSourceElementType(), Op0, OpVecs);

[ABataev]

Address comments.

[vporpo]

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

3881

Update comment explaining why this is done here instead of VectorizeTree() ?

[ABataev]

Address comments

[vporpo]

LGTM

[Orlando]

Hi, I can't build with this commit (see inline comment).

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

3891

This is failing to build for me because IndexIdx isn't captured here.

[ABataev]

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

3891

Fixed already in cdf8a53c1d33ad79be857991fbe4e7e7ced71021

[Orlando]

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

3891

Aha thanks, sorry for the noise.

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

case Instruction::GetElementPtr: {
                        ReuseShuffleIndicies);
           return;
         }
       }
 
       TreeEntry *TE = newTreeEntry(VL, Bundle /*vectorized*/, S, UserTreeIdx,
                                    ReuseShuffleIndicies);
       LLVM_DEBUG(dbgs() << "SLP: added a vector of GEPs.\n");
-      TE->setOperandsInOrder();
-      for (unsigned i = 0, e = 2; i < e; ++i) {
-        ValueList Operands;
-        // Prepare the operand vector.
-        for (Value *V : VL)
-          Operands.push_back(cast<Instruction>(V)->getOperand(i));
-
-        buildTree_rec(Operands, Depth + 1, {TE, i});
+      SmallVector<ValueList, 2> Operands(2);
+      // Prepare the operand vector for pointer operands.
+        Operands.front().push_back(
+            cast<GetElementPtrInst>(V)->getPointerOperand());
+      TE->setOperand(0, Operands.front());
+      // Need to cast all indices to the same type before vectorization to
+      // avoid crash.
+      // Required to be able to find correct matches between different gather
+      // nodes and reuse the vectorized values rather than trying to gather them
+      // again.
+      const int IndexIdx = 1;
+      Type *VL0Ty = VL0->getOperand(IndexIdx)->getType();
+      Type *Ty = all_of(VL,
+                        [VL0Ty](Value *V) {
+                          return VL0Ty == cast<GetElementPtrInst>(V)
+                                              ->getOperand(IndexIdx)
+                                              ->getType();
+                        })
+                     ? VL0Ty
+                     : DL->getIndexType(cast<GetElementPtrInst>(VL0)
+                                            ->getPointerOperandType()
+                                            ->getScalarType());
+      // Prepare the operand vector.
+      for (Value *V : VL) {
+        auto *Op = cast<Instruction>(V)->getOperand(IndexIdx);
+        auto *CI = cast<ConstantInt>(Op);
+        Operands.back().push_back(ConstantExpr::getIntegerCast(
+            CI, Ty, CI->getValue().isSignBitSet()));
       }
+      TE->setOperand(IndexIdx, Operands.back());
+
+      for (unsigned I = 0, Ops = Operands.size(); I < Ops; ++I)
+        buildTree_rec(Operands[I], 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) !=
case Instruction::Store: {
 
       Value *V = propagateMetadata(ST, E->Scalars);
 
       E->VectorizedValue = V;
       ++NumVectorInstructions;
       return V;
     }
     case Instruction::GetElementPtr: {
+      auto *GEP0 = cast<GetElementPtrInst>(VL0);
       setInsertPointAfterBundle(E);
 
       Value *Op0 = vectorizeTree(E->getOperand(0));
 
-      std::vector<Value *> OpVecs;
-      for (int j = 1, e = cast<GetElementPtrInst>(VL0)->getNumOperands(); j < e;
-           ++j) {
-        ValueList &VL = E->getOperand(j);
-        // Need to cast all elements to the same type before vectorization to
-        // avoid crash.
-        Type *VL0Ty = VL0->getOperand(j)->getType();
-        Type *Ty = llvm::all_of(
-                       VL, [VL0Ty](Value *V) { return VL0Ty == V->getType(); })
-                       ? VL0Ty
-                       : DL->getIndexType(cast<GetElementPtrInst>(VL0)
-                                              ->getPointerOperandType()
-                                              ->getScalarType());
-        for (Value *&V : VL) {
-          auto *CI = cast<ConstantInt>(V);
-          V = ConstantExpr::getIntegerCast(CI, Ty,
-                                           CI->getValue().isSignBitSet());
-        }
-        Value *OpVec = vectorizeTree(VL);
+      SmallVector<Value *> OpVecs;
+      for (int J = 1, N = GEP0->getNumOperands(); J < N; ++J) {
+        Value *OpVec = vectorizeTree(E->getOperand(J));
         OpVecs.push_back(OpVec);
       }
 
-      Value *V = Builder.CreateGEP(
-          cast<GetElementPtrInst>(VL0)->getSourceElementType(), Op0, OpVecs);
+      Value *V = Builder.CreateGEP(GEP0->getSourceElementType(), Op0, OpVecs);
       if (Instruction *I = dyn_cast<Instruction>(V))
         V = propagateMetadata(I, E->Scalars);
 
       ShuffleBuilder.addInversedMask(E->ReorderIndices);
       ShuffleBuilder.addMask(E->ReuseShuffleIndices);
       V = ShuffleBuilder.finalize(V);
 
       E->VectorizedValue = V;