[SLP] Outline GEP chain cost modeling into new TTI interface - NFCI.

[vdmitrie]

Summary

Cost modeling for GEPs should actually be target dependent but is currently
done inside SLP target-independent way.
Sinking it into TTI enables target dependent implementation.
This patch adds new TTI interface and implementation of the basic functionality
trying to retain existing cost modeling.

[RKSimon]

I'd expect this to cause cost changes - have you had any luck creating tests demonstrating that?

llvm/include/llvm/Analysis/TargetTransformInfo.h

2024

Pass by const ref?

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

2817

Drop the nullptr (maybe convert to StringRef?) and add "Calculated costs for Tree" to the only existing dumpTreeCosts call

7228

This kind of thing always feels like a cheat - is the only reason you don't return TCC_Free here because we still need to do the dumpTreeCosts call below?

7251

It's amazing how even a patch that tries to pull functionality out of SLP increases the size and complexity of SLPVectorizer.cpp :-(

[ABataev]

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

7251

It not just tries to pull something out, it introduces new interfaces, which will improve cost modeling. This, of course, may increase the complexity. Unfortunate, but required.

[vdmitrie]

Address comments. Rebase.

[vdmitrie]

In D144770#4153415, @RKSimon wrote:

I'd expect this to cause cost changes - have you had any luck creating tests demonstrating that?

I was not able to make one. Existing tests that are sensitive to this cost modeling are:

• for plain wide loads:

llvm/test/Transforms/SLPVectorizer/X86/remark_horcost.ll
llvm/test/Transforms/SLPVectorizer/X86/remark_not_all_parts.ll

• for gather loads:

llvm/test/Transforms/SLPVectorizer/X86/remark_gather-load-redux-cost.ll

But yep, I agree that this is not a great coverage.

[vdmitrie]

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

7228

This is just a short cut. If we have all the scalar GEPs to remain in vectorized code then vector cost would be the same. We could just return TCC_Free here but as you noted I'd like to have costs printed.

7251

I tried to add/reword some comments. May be that helps a bit.
We need to sink this target specific cost estimation down to TTI somehow (hence enable target specific implementations). I'm open for any suggestions that could help to solve this issue better way.

[ABataev]

In D144770#4157187, @vdmitrie wrote:

In D144770#4153415, @RKSimon wrote:

I'd expect this to cause cost changes - have you had any luck creating tests demonstrating that?

I was not able to make one. Existing tests that are sensitive to this cost modeling are:

• for plain wide loads:

llvm/test/Transforms/SLPVectorizer/X86/remark_horcost.ll
llvm/test/Transforms/SLPVectorizer/X86/remark_not_all_parts.ll

• for gather loads:

llvm/test/Transforms/SLPVectorizer/X86/remark_gather-load-redux-cost.ll

But yep, I agree that this is not a great coverage.

Make them to emit remarks with costs estimations?

[vdmitrie]

In D144770#4157226, @ABataev wrote:

In D144770#4157187, @vdmitrie wrote:

In D144770#4153415, @RKSimon wrote:

I'd expect this to cause cost changes - have you had any luck creating tests demonstrating that?

I was not able to make one. Existing tests that are sensitive to this cost modeling are:

• for plain wide loads:

llvm/test/Transforms/SLPVectorizer/X86/remark_horcost.ll
llvm/test/Transforms/SLPVectorizer/X86/remark_not_all_parts.ll

• for gather loads:

llvm/test/Transforms/SLPVectorizer/X86/remark_gather-load-redux-cost.ll

But yep, I agree that this is not a great coverage.

Make them to emit remarks with costs estimations?

I'm not sure I understand. All these tests do already check for pass-remarks output which includes vec tree cost. Did you mean something else?

[ABataev]

In D144770#4157230, @vdmitrie wrote:

In D144770#4157226, @ABataev wrote:

In D144770#4157187, @vdmitrie wrote:

In D144770#4153415, @RKSimon wrote:

I'd expect this to cause cost changes - have you had any luck creating tests demonstrating that?

I was not able to make one. Existing tests that are sensitive to this cost modeling are:

• for plain wide loads:

llvm/test/Transforms/SLPVectorizer/X86/remark_horcost.ll
llvm/test/Transforms/SLPVectorizer/X86/remark_not_all_parts.ll

• for gather loads:

llvm/test/Transforms/SLPVectorizer/X86/remark_gather-load-redux-cost.ll

But yep, I agree that this is not a great coverage.

Make them to emit remarks with costs estimations?

I'm not sure I understand. All these tests do already check for pass-remarks output which includes vec tree cost. Did you mean something else?

I thought they don't have remarks. Maybe add remarks to some other tests?

[ABataev]

llvm/include/llvm/Analysis/TargetTransformInfo.h

284–291

Better to represent them as bitfields:

bool IsSameBaseAddress:1 = false;
bool IsUniformStride:1 = false;
bool IsKnownStride:1 = false;
unsigned Reserved: 29;

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

2816–2826

This change may be committed separately

7201

Use areAllUsersVectorized?

[vdmitrie]

llvm/include/llvm/Analysis/TargetTransformInfo.h

284–291

That's fair
Will do

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

2816–2826

Split into https://reviews.llvm.org/D144992

7201

Yep. That what I was considering. Just though about addressing it in a follow up patch.

[vdmitrie]

Address comment.
Rebase.

[vdmitrie]

I thought they don't have remarks. Maybe add remarks to some other tests?

Please let me know if you have some specific tests in mind to beef up with pass remark checks.
May be tests with specific patterns? Finding them by looking through every test manually is a bit tedious work.

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

7201

I experimented with replacing single use check with areAllUsersVectorized(). Yes, that's what we want but it makes the patch non-NFC. So I'd like to make that change separately.

[ABataev]

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

1045

Add an assert that all geps are from the same basic block?

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

7179

What if it is scatter vectorize node? VL0 still should be LoadInst.

[vdmitrie]

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

1045

will do

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

7179

We do not call this routine for scatter vectorize loads node. We did that in the past but that was a bug (since we evaluated GEPs twice as the result). For scatter vectorize we evaluate GEPs only when calculating cost for tree node that correspond to pointer operands of the loads. The comment above explains exactly that distinction.

[ABataev]

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

7179

Could you add an assertion for the expected cases? Like ScatterVectorize is not expected, etc.

[vdmitrie]

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

7179

Sure

[vdmitrie]

Address comments to add assertions.
Rebase.

[ABataev]

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

1047–1048

Can it happen at all?

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

7176

Better just return here with diff 0.

[vdmitrie]

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

1047–1048

Technically, yes. Ptrs is not prohibited to contain just single Base entry (which can be a value or a non-GEP instruction). How practical such use would be is the different kind of question.
If we make the assertion stricter by adding add It != Ptrs.end() we anyway have to add ugly fake use for "It" to avoid build time warning when assertions are suppressed.

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

7176

I'd like to not bypass the debug printer in this case.
So here are couple of options:
1 - leave it as is
2 -add debug printer prior to the early re
turn.
Pls let me know which way would you prefer.

[ABataev]

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

1047–1048

Can you add some extra asserts that if there is a single Base, other elements are Undefs/Poisons?

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

7176

Debug print and early exit

[tschuett]

llvm/include/llvm/Analysis/TargetTransformInfo.h

283

Can you static_assert that the size of PointerChainInfo is 4?

[vdmitrie]

Address comments

[vdmitrie]

add zero initializer for reserved field as some compilers complain

[ABataev]

LG with a nit

llvm/include/llvm/Analysis/TargetTransformInfo.h

316

Add the meaningful message

[vdmitrie]

llvm/include/llvm/Analysis/TargetTransformInfo.h

316

Yep. I'll add it. I was not sure about wording. How about "Was size increase justified?" ?

BTW. I need to make another change to the struct (bool => unsigned) as on Windows that works differently and the size appears 8.

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

1055

There is actually a problem with this assertion. I suspected about it but only revealed for sure when tried to test the patch more extensively. The problem is that it is very possible that GEPS are not in the same block.
The only guaranteed is that their current uses (loads or stores) are all in the same block.
Imagine we have some struct with 4 fields of same size.
It is possible that 2nd field is written in one block, and all four fields being written in another block. When we try to vectorize these four stores we may have definition for GEP of 2d element store coming from strictly dominating block.
Any ideas about how this should be handled?

[ABataev]

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

1055

For the vectorized blocks it is not possible - we expect that vectorized GEPs are from the same block, otherwise they will be gathered

[vdmitrie]

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

1055

For GEPs that are operands of scatter vectorize node we probably have this check. I did not verify that but I'm not arguing that it is not true.
We definitely have problem for plain (wide) load/stores. We never build dedicated GEP vec-tree node for them and we never check in SLP vectorizer their pointer operands are defined all in same block when we vectorize loads/stores.

[ABataev]

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

1055

Right, for vectorized loads/store we do not do it, we just don't need it. Then you need to remove it.

[vdmitrie]

Fixed issues revealed with more extensive test run.
Removed asserts from base implementation. Same GEPs origin is not guaranteed and and I don't find asserting for kinds of incoming pointers as quite useful.
Added assertion that Base is provided if sameBase is set.
PointersChainInfo changed to satisfy static assert condition on Windows.
Rebased.

[vdmitrie]

rebase

[ABataev]

LG

llvm/include/llvm/Analysis/TargetTransformInfo.h

public:
   };
 
   /// Estimate the cost of a GEP operation when lowered.
   InstructionCost
   getGEPCost(Type *PointeeType, const Value *Ptr,
              ArrayRef<const Value *> Operands,
              TargetCostKind CostKind = TCK_SizeAndLatency) const;
 
+  /// Describe known properties for a set of pointers.
+  struct PointersChainInfo {
+    /// All the GEPs in a set have same base address.
+    unsigned IsSameBaseAddress : 1;
+    /// These properties only valid if SameBaseAddress is set.
+    /// True if distance between any two neigbouring pointers is same value.
+    unsigned IsUniformStride : 1;
+    /// True if distance between any two neigbouring pointers is a known value.
+    unsigned IsKnownStride : 1;
+    unsigned Reserved : 29;
+
+    bool isSameBase() const { return IsSameBaseAddress; }
+    bool isUniformStride() const {
+      return IsSameBaseAddress && IsUniformStride;
+    }
+    bool isKnownStride() const { return IsSameBaseAddress && IsKnownStride; }
+
+    static PointersChainInfo getKnownUniformStrided() {
+      return {/*IsSameBaseAddress=*/1, /*IsUniformStride=*/1,
+              /*IsKnownStride=*/1, 0};
+    }
+    static PointersChainInfo getUniformStrided() {
+      return {/*IsSameBaseAddress=*/1, /*IsUniformStride=*/1,
+              /*IsKnownStride=*/0, 0};
+    }
+    static PointersChainInfo getKnownNonUniformStrided() {
+      return {/*IsSameBaseAddress=*/1, /*IsUniformStride=*/0,
+              /*IsKnownStride=*/1, 0};
+    }
+    static PointersChainInfo getNonUniformStrided() {
+      return {/*IsSameBaseAddress=*/1, /*IsUniformStride=*/0,
+              /*IsKnownStride=*/0, 0};
+    }
+  };
+  static_assert(sizeof(PointersChainInfo) == 4, "Was size increase justified?");
+
+  /// Estimate the cost of a chain of pointers (typically pointer operands of a
+  /// chain of loads or stores within same block) operations set when lowered.
+  InstructionCost
+  getPointersChainCost(ArrayRef<const Value *> Ptrs, const Value *Base,
+                       const PointersChainInfo &Info,
+                       TargetCostKind CostKind = TTI::TCK_RecipThroughput
+
+  ) const;
+
   /// \returns A value by which our inlining threshold should be multiplied.
   /// This is primarily used to bump up the inlining threshold wholesale on
   /// targets where calls are unusually expensive.
   ///
   /// TODO: This is a rather blunt instrument.  Perhaps altering the costs of
   /// individual classes of instructions would be better.
   unsigned getInliningThresholdMultiplier() const;
 

 
 class TargetTransformInfo::Concept {
 public:
   virtual ~Concept() = 0;
   virtual const DataLayout &getDataLayout() const = 0;
   virtual InstructionCost getGEPCost(Type *PointeeType, const Value *Ptr,
                                      ArrayRef<const Value *> Operands,
                                      TTI::TargetCostKind CostKind) = 0;
+  virtual InstructionCost
+  getPointersChainCost(ArrayRef<const Value *> Ptrs, const Value *Base,
+                       const TTI::PointersChainInfo &Info,
+                       TTI::TargetCostKind CostKind) = 0;
   virtual unsigned getInliningThresholdMultiplier() = 0;
   virtual unsigned adjustInliningThreshold(const CallBase *CB) = 0;
   virtual int getInlinerVectorBonusPercent() = 0;
   virtual InstructionCost getMemcpyCost(const Instruction *I) = 0;
   virtual unsigned
   getEstimatedNumberOfCaseClusters(const SwitchInst &SI, unsigned &JTSize,
                                    ProfileSummaryInfo *PSI,
                                    BlockFrequencyInfo *BFI) = 0;
public:
   }
 
   InstructionCost
   getGEPCost(Type *PointeeType, const Value *Ptr,
              ArrayRef<const Value *> Operands,
              TargetTransformInfo::TargetCostKind CostKind) override {
     return Impl.getGEPCost(PointeeType, Ptr, Operands, CostKind);
   }
+  InstructionCost getPointersChainCost(ArrayRef<const Value *> Ptrs,
+                                       const Value *Base,
+                                       const PointersChainInfo &Info,
+                                       TargetCostKind CostKind) override {
+    return Impl.getPointersChainCost(Ptrs, Base, Info, CostKind);
+  }
   unsigned getInliningThresholdMultiplier() override {
     return Impl.getInliningThresholdMultiplier();
   }
   unsigned adjustInliningThreshold(const CallBase *CB) override {
     return Impl.adjustInliningThreshold(CB);
   }
   int getInlinerVectorBonusPercent() override {
     return Impl.getInlinerVectorBonusPercent();

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

InstructionCost getGEPCost(Type *PointeeType, const Value *Ptr,
     if (static_cast<T *>(this)->isLegalAddressingMode(
             TargetType, const_cast<GlobalValue *>(BaseGV),
             BaseOffset.sextOrTrunc(64).getSExtValue(), HasBaseReg, Scale,
             Ptr->getType()->getPointerAddressSpace()))
       return TTI::TCC_Free;
     return TTI::TCC_Basic;
   }
 
+  InstructionCost getPointersChainCost(ArrayRef<const Value *> Ptrs,
+                                       const Value *Base,
+                                       const TTI::PointersChainInfo &Info,
+                                       TTI::TargetCostKind CostKind) {
+    InstructionCost Cost = TTI::TCC_Free;
+    // In the basic model we take into account GEP instructions only
+    // (although here can come alloca instruction, a value, constants and/or
+    // constant expressions, PHIs, bitcasts ... whatever allowed to be used as a
+    // pointer). Typically, if Base is a not a GEP-instruction and all the
+    // pointers are relative to the same base address, all the rest are
+    // either GEP instructions, PHIs, bitcasts or constants. When we have same
+    // base, we just calculate cost of each non-Base GEP as an ADD operation if
+    // any their index is a non-const.
+    // If no known dependecies between the pointers cost is calculated as a sum
+    // of costs of GEP instructions.
+    for (const Value *V : Ptrs) {
+      const auto *GEP = dyn_cast<GetElementPtrInst>(V);
+      if (!GEP)
+        continue;
+      if (Info.isSameBase() && V != Base) {
+        if (GEP->hasAllConstantIndices())
+          continue;
+        Cost += static_cast<T *>(this)->getArithmeticInstrCost(
+            Instruction::Add, GEP->getType(), CostKind,
+            {TTI::OK_AnyValue, TTI::OP_None}, {TTI::OK_AnyValue, TTI::OP_None},
+            std::nullopt);
+      } else {
+        SmallVector<const Value *> Indices(GEP->indices());
+        Cost += static_cast<T *>(this)->getGEPCost(GEP->getSourceElementType(),
+                                                   GEP->getPointerOperand(),
+                                                   Indices, CostKind);
+      }
+    }
+    return Cost;
+  }
+
   InstructionCost getInstructionCost(const User *U,
                                      ArrayRef<const Value *> Operands,
                                      TTI::TargetCostKind CostKind) {
     using namespace llvm::PatternMatch;
 
     auto *TargetTTI = static_cast<T *>(this);
     // Handle non-intrinsic calls, invokes, and callbr.
     // FIXME: Unlikely to be true for anything but CodeSize.

llvm/lib/Analysis/TargetTransformInfo.cpp

 
 InstructionCost
 TargetTransformInfo::getGEPCost(Type *PointeeType, const Value *Ptr,
                                 ArrayRef<const Value *> Operands,
                                 TTI::TargetCostKind CostKind) const {
   return TTIImpl->getGEPCost(PointeeType, Ptr, Operands, CostKind);
 }
 
+InstructionCost TargetTransformInfo::getPointersChainCost(
+    ArrayRef<const Value *> Ptrs, const Value *Base,
+    const TTI::PointersChainInfo &Info, TTI::TargetCostKind CostKind) const {
+  assert((Base || !Info.isSameBase()) &&
+         "If pointers have same base address it has to be provided.");
+  return TTIImpl->getPointersChainCost(Ptrs, Base, Info, CostKind);
+}
+
 unsigned TargetTransformInfo::getEstimatedNumberOfCaseClusters(
     const SwitchInst &SI, unsigned &JTSize, ProfileSummaryInfo *PSI,
     BlockFrequencyInfo *BFI) const {
   return TTIImpl->getEstimatedNumberOfCaseClusters(SI, JTSize, PSI, BFI);
 }
 
 InstructionCost
 TargetTransformInfo::getInstructionCost(const User *U,

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

LLVM_DUMP_METHOD void dump() const {
         dbgs() << EInfo << ", ";
       dbgs() << "\n";
     }
 #endif
   };
 
 #ifndef NDEBUG
   void dumpTreeCosts(const TreeEntry *E, InstructionCost ReuseShuffleCost,
                      InstructionCost VecCost, InstructionCost ScalarCost,
                      StringRef Banner) const {
     dbgs() << "SLP: " << Banner << ":\n";
     E->dump();
     dbgs() << "SLP: Costs:\n";
     dbgs() << "SLP:     ReuseShuffleCost = " << ReuseShuffleCost << "\n";
     dbgs() << "SLP:     VectorCost = " << VecCost << "\n";
     dbgs() << "SLP:     ScalarCost = " << ScalarCost << "\n";
     dbgs() << "SLP:     ReuseShuffleCost + VecCost - ScalarCost = "
            << ReuseShuffleCost + VecCost - ScalarCost << "\n";
   }
 #endif
 
   /// Create a new VectorizableTree entry.
   TreeEntry *newTreeEntry(ArrayRef<Value *> VL,
                           std::optional<ScheduleData *> Bundle,
                           const InstructionsState &S,
                           const EdgeInfo &UserTreeIdx,
                           ArrayRef<int> ReuseShuffleIndices = std::nullopt,
auto GetCostDiff =
         InstructionCost VecCost = VectorCost(CommonCost);
         LLVM_DEBUG(dumpTreeCosts(E, CommonCost, VecCost - CommonCost,
                                  ScalarCost, "Calculated costs for Tree"));
         return VecCost - ScalarCost;
       };
   // Calculate cost difference from vectorizing set of GEPs.
   // Negative value means vectorizing is profitable.
   auto GetGEPCostDiff = [=](ArrayRef<Value *> Ptrs, Value *BasePtr) {
-    InstructionCost CostSavings = 0;
+    InstructionCost ScalarCost = 0;
+    InstructionCost VecCost = 0;
+    // Here we differentiate two cases: (1) when Ptrs represent a regular
+    // vectorization tree node (as they are pointer arguments of scattered
+    // loads) or (2) when Ptrs are the arguments of loads or stores being
+    // vectorized as plane wide unit-stride load/store since all the
+    // loads/stores are known to be from/to adjacent locations.
+    assert(E->State == TreeEntry::Vectorize &&
+           "Entry state expected to be Vectorize here.");
+    if (isa<LoadInst, StoreInst>(VL0)) {
+      // Case 2: estimate costs for pointer related costs when vectorizing to
+      // a wide load/store.
+      // Scalar cost is estimated as a set of pointers with known relationship
+      // between them.
+      // For vector code we will use BasePtr as argument for the wide load/store
+      // but we also need to account all the instructions which are going to
+      // stay in vectorized code due to uses outside of these scalar
+      // loads/stores.
+      ScalarCost = TTI->getPointersChainCost(
+          Ptrs, BasePtr, TTI::PointersChainInfo::getKnownUniformStrided(),
+          CostKind);
+
+      SmallVector<const Value *> PtrsRetainedInVecCode;
       for (Value *V : Ptrs) {
-      if (V == BasePtr)
+        if (V == BasePtr) {
+          PtrsRetainedInVecCode.push_back(V);
           continue;
+        }
         auto *Ptr = dyn_cast<GetElementPtrInst>(V);
-      // GEPs may contain just addresses without instructions, considered free.
-      // GEPs with all constant indices also considered to have zero cost.
-      if (!Ptr || Ptr->hasAllConstantIndices())
-        continue;
-
-      // Here we differentiate two cases: when GEPs represent a regular
-      // vectorization tree node (and hence vectorized) and when the set is
-      // arguments of a set of loads or stores being vectorized. In the former
-      // case all the scalar GEPs will be removed as a result of vectorization.
+        // For simplicity assume Ptr to stay in vectorized code if it's not a
+        // GEP instruction. We don't care since it's cost considered free.
+        // TODO: We should check for any uses outside of vectorizable tree
+        // rather than just single use.
+        if (!Ptr || !Ptr->hasOneUse())
+          PtrsRetainedInVecCode.push_back(V);
+      }
+
+      if (PtrsRetainedInVecCode.size() == Ptrs.size()) {
+        // If all pointers stay in vectorized code then we don't have
+        // any savings on that.
+        LLVM_DEBUG(dumpTreeCosts(E, 0, ScalarCost, ScalarCost,
+                                 "Calculated GEPs cost for Tree"));
+        return InstructionCost{TTI::TCC_Free};
+      }
+      VecCost = TTI->getPointersChainCost(
+          PtrsRetainedInVecCode, BasePtr,
+          TTI::PointersChainInfo::getKnownNonUniformStrided(), CostKind);
+    } else {
+      // Case 1: Ptrs are the arguments of loads that we are going to transform
+      // into masked gather load intrinsic.
+      // All the scalar GEPs will be removed as a result of vectorization.
       // For any external uses of some lanes extract element instructions will
-      // be generated (which cost is estimated separately). For the latter case
-      // since the set of GEPs itself is not vectorized those used more than
-      // once will remain staying in vectorized code as well. So we should not
-      // count them as savings.
-      if (!Ptr->hasOneUse() && isa<LoadInst, StoreInst>(VL0))
-        continue;
-
-      // TODO: it is target dependent, so need to implement and then use a TTI
-      // interface.
-      CostSavings += TTI->getArithmeticInstrCost(Instruction::Add,
-                                                 Ptr->getType(), CostKind);
-    }
-    LLVM_DEBUG(dbgs() << "SLP: Calculated GEPs cost savings or Tree:\n";
-               E->dump());
-    LLVM_DEBUG(dbgs() << "SLP:     GEP cost saving = " << CostSavings << "\n");
-    return InstructionCost() - CostSavings;
+      // be generated (which cost is estimated separately).
+      TTI::PointersChainInfo PtrsInfo =
+          all_of(Ptrs,
+                 [](const Value *V) {
+                   auto *Ptr = dyn_cast<GetElementPtrInst>(V);
+                   return Ptr && !Ptr->hasAllConstantIndices();
+                 })
+              ? TTI::PointersChainInfo::getNonUniformStrided()
+              : TTI::PointersChainInfo::getKnownNonUniformStrided();
+
+      ScalarCost = TTI->getPointersChainCost(Ptrs, BasePtr, PtrsInfo, CostKind);
+
+      // Remark: it not quite correct to use scalar GEP cost for a vector GEP,
+      // but it's not clear how to do that without having vector GEP arguments
+      // ready.
+      // Perhaps using just TTI::TCC_Free/TTI::TCC_Basic would be better option.
+      if (const auto *Base = dyn_cast<GetElementPtrInst>(BasePtr)) {
+        SmallVector<const Value *> Indices(Base->indices());
+        VecCost = TTI->getGEPCost(Base->getSourceElementType(),
+                                  Base->getPointerOperand(), Indices, CostKind);
+      }
+    }
+
+    LLVM_DEBUG(dumpTreeCosts(E, 0, VecCost, ScalarCost,
+                             "Calculated GEPs cost for Tree"));
+
+    return VecCost - ScalarCost;
   };
 
   switch (ShuffleOrOp) {
   case Instruction::PHI: {
     // Count reused scalars.
     InstructionCost ScalarCost = 0;
     SmallPtrSet<const TreeEntry *, 4> CountedOps;
     for (Value *V : VL) {