[SLP][RISCV] Account for offset folding in getPointersChainCost

[luke]

Summary

For a GEP in a pointer chain, if:

1. a pointer chain is unit-strided
2. the base pointer wasn't folded and is sitting in a register somewhere
3. the distance between the GEP and the base pointer is small enough and can be folded into the addressing mode of the using load/store

Then we can exclude that GEP from the total cost of the pointer chain,
as it will likely be folded away.

In order to check if 3) holds, we need to know the type of memory access
being made by the users of the pointer chain. For that, we need to pass
along a new argument to getPointersChainCost. (Using the source pointer
type of the GEP isn't accurate, see https://reviews.llvm.org/D149889 for
more details).

Also note that 2) is currently an assumption, and could be modelled more
accurately.

This prevents some unprofitable cases from being SLP vectorized on
RISC-V by making the scalar costs cheaper and closer to the actual
codegen.

For now the getPointersChainCost hook is duplicated for RISC-V to prevent
disturbing other targets, but could be merged back in and shared with
other targets in a following patch.

[luke]

llvm/lib/Target/X86/X86TargetTransformInfo.cpp

4960

I removed this because it seems to be subsumed by the extra check in the base implementation. It's 100% equivalent as it will now cost for GEPs that don't fit into the addressing mode, but that should be more accurate right?

[luke]

Remove fixme

[ABataev]

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

1057

I

1069

Add a comment with the param name for the true argument

[luke]

Address comments

llvm/lib/Target/X86/X86TargetTransformInfo.cpp

4960

Typo, it's *not* 100% equivalent

[ABataev]

I think this patch requires some cost analysis tests.

[reames]

llvm/include/llvm/Analysis/TargetTransformInfo.h

323

Rephrase: is the type of the memory access.

327

Rename: AccessTy

(This matches the naming convention we use elsewhere for this concept.)

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

1066

I don't see anything here which requires the stride to be equal to the type size. What prevents the set of pointers from e.g. advancing by 64 bytes where the access type is 8 bytes? (i.e. what prevents this from being a strided access with non-unit stride in RISCV terms?)

llvm/lib/Target/X86/X86TargetTransformInfo.cpp

4950

KnownStride, and KnownUniform are not the same condition. I don't think your code change in the general model actually matches what you removed here.

I'd suggest by starting with a RISCV specific hook on your heuristic, and then we can merge in a post commit. I think the RISCV version is going to be more restrictive.

[luke]

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

1066

From what I understand, PointersChainInfo::isUniformStride() implies unit stride pointers. The only user of getPointersChainCost is in SLP's getGEPCostDiff, and the only time getKnownUniformStrided() is created is in case 2) here, which is a wide vector load:

// 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(),
      ScalarTy, CostKind);

I agree this is non-obvious, I was considering renaming isUniformStride() to isUnitStride() or something more strict but deferred to keep the patch small. I can do this is a parent patch or alternatively leave a comment explaining this.

[luke]

In D149654#4312732, @ABataev wrote:

I think this patch requires some cost analysis tests.

I agree, although getPointersChainInfo doesn't get called anywhere useful for opt -passes="print<cost-model>", so the only way I can think of verifying the cost is via checking the debug output from SLP. Is there a better way to go about this?

[luke]

Restore x86 hook

[luke]

Rebase ontop of D150662 and add cost test case

[luke]

Fix rebase

[luke]

Doesn't seem to be any changes in x86 code on the llvm-test-suite:

Tests: 2432
Metric: size..text

Program                                        size..text                      
                                               results.head results.patch diff 
 Bitcode/Be...ral_grid/halide_bilateral_grid   59937.00     59937.00       0.0%
 SingleSour.../execute/GCC-C-execute-loop-2d     593.00       593.00       0.0%
 SingleSour.../execute/GCC-C-execute-loop-11    1185.00      1185.00       0.0%
 SingleSour.../execute/GCC-C-execute-loop-12     465.00       465.00       0.0%
 SingleSour.../execute/GCC-C-execute-loop-13     497.00       497.00       0.0%
 SingleSour.../execute/GCC-C-execute-loop-14     385.00       385.00       0.0%
 SingleSour.../execute/GCC-C-execute-loop-15    1633.00      1633.00       0.0%
 SingleSour...e/execute/GCC-C-execute-loop-2     561.00       561.00       0.0%
 SingleSour.../execute/GCC-C-execute-loop-2b     561.00       561.00       0.0%
 SingleSour.../execute/GCC-C-execute-loop-2e     769.00       769.00       0.0%
 Bitcode/Be...hmarks/Halide/blur/halide_blur   35809.00     35809.00       0.0%
 SingleSour.../execute/GCC-C-execute-loop-2f     417.00       417.00       0.0%
 SingleSour.../execute/GCC-C-execute-loop-2g     417.00       417.00       0.0%
 SingleSour...e/execute/GCC-C-execute-loop-3     417.00       417.00       0.0%
 SingleSour.../execute/GCC-C-execute-loop-3b     449.00       449.00       0.0%
                            Geomean difference      nan          nan       0.0%
         size..text                      
run    results.head results.patch    diff
count  2.432000e+03  2.432000e+03  2432.0
mean   1.692725e+04  1.692725e+04  0.0   
std    1.559107e+05  1.559107e+05  0.0   
min    3.530000e+02  3.530000e+02  0.0   
25%    3.850000e+02  3.850000e+02  0.0   
50%    5.130000e+02  5.130000e+02  0.0   
75%    4.765000e+03  4.765000e+03  0.0   
max    7.177889e+06  7.177889e+06  0.0

[luke]

On sqlite3, -O2, there are 12 fewer VF=2 sequences vectorized on RISC-V:

$ grep -E "vsetivli\s+zero, 2, e" sqlite.head.s | wc -l
378
$ grep -E "vsetivli\s+zero, 2, e" sqlite.patch.s | wc -l
366

Most of them are in the form of something like:

	sb	a0, 5(s4)
	sb	s6, 6(s4)
	addi	s4, s4, 1
	vsetivli	zero, 2, e8, mf8, ta, ma
	vmv.v.i	v8, 0
	vse8.v	v8, (s4)

Which are now being emitted as scalar:

	sb	a0, 5(s3)
	sb	s6, 6(s3)
	sb	zero, 1(s3)
	sb	zero, 2(s3)

There are no differences in the code generated on x86. I've attached the RISC-V outputs if anyone wants to see the changes.

sqlite.head.s4 MBDownload

sqlite.patch.s4 MBDownload

[luke]

Isolate changes to RISCVTargetTransformInfo

[luke]

llvm/test/Transforms/SLPVectorizer/RISCV/getpointerschaincost.ll

1

Apologies in advance for testing with the debug output, I can't think of another way to get access to getPointersChainCost.

[luke]

llvm/test/Transforms/SLPVectorizer/RISCV/getpointerschaincost.ll

56

Also worth noting, I tried to come up with a test case where only some of the pointers were folded and some weren't, but couldn't find a sane way to do so.
Namely for RISC-V, we need a chain of pointers that are unit-strided, but is also somehow long enough that the offset overflows 2^12.

[luke]

Rebase

[ABataev]

llvm/test/Transforms/SLPVectorizer/RISCV/getpointerschaincost.ll

1

-pass-remarks-output= inbstead, check some of the remarks_... tests in SLPVectorizer tests directory as an example. Also, better to precommit new tests separately

[reames]

The RISCV and API extension bits now look good to me. Once Alexey's happy with the SLP bits, you're good to go.

Looking at the structure of the patch - thanks for the scope reduction - I realized this is a special case of pointer difference. The general form of this would be to compute the offset between each GEP and the base. If that difference fits in the scalar addressing range, that GEP has zero cost. Otherwise, it has cost.

This does raise the point that considering a constant offset GEP as zero cost is actually wrong. If the offsets are 0, and UINT_MAX, that's not a zero cost GEP on RISCV.

The only machinery I know for this in LLVM is BasicAAResult::DecomposedGEP . We could potentially reuse some of that.

There's also an interaction with the ptradd proposal here. With simpler GEPs, the subtraction becomes trivial.

This is definite definitely future work through. And probably not future work actually worth doing, at least for the general case. :)

[luke]

In D149654#4356406, @reames wrote:

This does raise the point that considering a constant offset GEP as zero cost is actually wrong. If the offsets are 0, and UINT_MAX, that's not a zero cost GEP on RISCV.

I guess with cost modelling there's not a strict definition of "wrong", but whether it could be more accurate. Whilst working on this locally I changed that assumption to check if it's foldable, but intentionally left it out to simplify things. It seems like an edge case where the approximation gets it right most of the time.

[luke]

Convert test to remarks test

[ABataev]

LG

llvm/include/llvm/Analysis/TargetTransformInfo.h

static PointersChainInfo getUnknownStride() {
       return {/*IsSameBaseAddress=*/1, /*IsUnitStride=*/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.
+  /// \p AccessTy is the type of the loads/stores that will ultimately use the
+  /// \p Ptrs.
   InstructionCost
   getPointersChainCost(ArrayRef<const Value *> Ptrs, const Value *Base,
-                       const PointersChainInfo &Info,
+                       const PointersChainInfo &Info, Type *AccessTy,
                        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.
   ///

 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,
+                       const TTI::PointersChainInfo &Info, Type *AccessTy,
                        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,
public:
   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,
+                                       Type *AccessTy,
                                        TargetCostKind CostKind) override {
-    return Impl.getPointersChainCost(Ptrs, Base, Info, CostKind);
+    return Impl.getPointersChainCost(Ptrs, Base, Info, AccessTy, CostKind);
   }
   unsigned getInliningThresholdMultiplier() override {
     return Impl.getInliningThresholdMultiplier();
   }
   unsigned adjustInliningThreshold(const CallBase *CB) override {
     return Impl.adjustInliningThreshold(CB);
   }
   int getInlinerVectorBonusPercent() override {

llvm/include/llvm/Analysis/TargetTransformInfoImpl.h

if (static_cast<T *>(this)->isLegalAddressingMode(
             Ptr->getType()->getPointerAddressSpace()))
       return TTI::TCC_Free;
     return TTI::TCC_Basic;
   }
 
   InstructionCost getPointersChainCost(ArrayRef<const Value *> Ptrs,
                                        const Value *Base,
                                        const TTI::PointersChainInfo &Info,
+                                       Type *AccessTy,
                                        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;
   }
 

llvm/lib/Analysis/TargetTransformInfo.cpp

 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 {
+    const TTI::PointersChainInfo &Info, Type *AccessTy,
+    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);
+  return TTIImpl->getPointersChainCost(Ptrs, Base, Info, AccessTy, CostKind);
 }
 
 unsigned TargetTransformInfo::getEstimatedNumberOfCaseClusters(
     const SwitchInst &SI, unsigned &JTSize, ProfileSummaryInfo *PSI,
     BlockFrequencyInfo *BFI) const {
   return TTIImpl->getEstimatedNumberOfCaseClusters(SI, JTSize, PSI, BFI);
 }
 

llvm/lib/Target/RISCV/RISCVTargetTransformInfo.h

bool preferEpilogueVectorization() const {
     // should re-examine this once vectorization is better tuned.
     return false;
   }
 
   InstructionCost getMaskedMemoryOpCost(unsigned Opcode, Type *Src,
                                         Align Alignment, unsigned AddressSpace,
                                         TTI::TargetCostKind CostKind);
 
+  InstructionCost getPointersChainCost(ArrayRef<const Value *> Ptrs,
+                                       const Value *Base,
+                                       const TTI::PointersChainInfo &Info,
+                                       Type *AccessTy,
+                                       TTI::TargetCostKind CostKind);
+
   void getUnrollingPreferences(Loop *L, ScalarEvolution &SE,
                                TTI::UnrollingPreferences &UP,
                                OptimizationRemarkEmitter *ORE);
 
   void getPeelingPreferences(Loop *L, ScalarEvolution &SE,
                              TTI::PeelingPreferences &PP);
 
   unsigned getMinVectorRegisterBitWidth() const {

llvm/lib/Target/RISCV/RISCVTargetTransformInfo.cpp

InstructionCost RISCVTTIImpl::getArithmeticInstrCost(
   }
   default:
     return ConstantMatCost +
            BaseT::getArithmeticInstrCost(Opcode, Ty, CostKind, Op1Info, Op2Info,
                                          Args, CxtI);
   }
 }
 
+// TODO: Deduplicate from TargetTransformInfoImplCRTPBase.
+InstructionCost RISCVTTIImpl::getPointersChainCost(
+    ArrayRef<const Value *> Ptrs, const Value *Base,
+    const TTI::PointersChainInfo &Info, Type *AccessTy,
+    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 (auto [I, V] : enumerate(Ptrs)) {
+    const auto *GEP = dyn_cast<GetElementPtrInst>(V);
+    if (!GEP)
+      continue;
+    if (Info.isSameBase() && V != Base) {
+      if (GEP->hasAllConstantIndices())
+        continue;
+      // If the chain is unit-stride and BaseReg + stride*i is a legal
+      // addressing mode, then presume the base GEP is sitting around in a
+      // register somewhere and check if we can fold the offset relative to
+      // it.
+      unsigned Stride = DL.getTypeStoreSize(AccessTy);
+      if (Info.isUnitStride() &&
+          isLegalAddressingMode(AccessTy,
+                                /* BaseGV */ nullptr,
+                                /* BaseOffset */ Stride * I,
+                                /* HasBaseReg */ true,
+                                /* Scale */ 0,
+                                GEP->getType()->getPointerAddressSpace()))
+        continue;
+      Cost += 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 += getGEPCost(GEP->getSourceElementType(), GEP->getPointerOperand(),
+                         Indices, CostKind);
+    }
+  }
+  return Cost;
+}
+
 void RISCVTTIImpl::getUnrollingPreferences(Loop *L, ScalarEvolution &SE,
                                            TTI::UnrollingPreferences &UP,
                                            OptimizationRemarkEmitter *ORE) {
   // TODO: More tuning on benchmarks and metrics with changes as needed
   //       would apply to all settings below to enable performance.
 
 
   if (ST->enableDefaultUnroll())

llvm/lib/Target/X86/X86TargetTransformInfo.h

public:
   InstructionCost getGatherScatterOpCost(unsigned Opcode, Type *DataTy,
                                          const Value *Ptr, bool VariableMask,
                                          Align Alignment,
                                          TTI::TargetCostKind CostKind,
                                          const Instruction *I);
   InstructionCost getPointersChainCost(ArrayRef<const Value *> Ptrs,
                                        const Value *Base,
                                        const TTI::PointersChainInfo &Info,
+                                       Type *AccessTy,
                                        TTI::TargetCostKind CostKind);
   InstructionCost getAddressComputationCost(Type *PtrTy, ScalarEvolution *SE,
                                             const SCEV *Ptr);
 
   std::optional<Instruction *> instCombineIntrinsic(InstCombiner &IC,
                                                     IntrinsicInst &II) const;
   std::optional<Value *>
   simplifyDemandedUseBitsIntrinsic(InstCombiner &IC, IntrinsicInst &II,

llvm/lib/Target/X86/X86TargetTransformInfo.cpp

X86TTIImpl::getMaskedMemoryOpCost(unsigned Opcode, Type *SrcTy, Align Alignment,
   // Pre-AVX512 - each maskmov load costs 2 + store costs ~8.
   if (!ST->hasAVX512())
     return Cost + LT.first * (IsLoad ? 2 : 8);
 
   // AVX-512 masked load/store is cheaper
   return Cost + LT.first;
 }
 
-InstructionCost X86TTIImpl::getPointersChainCost(
-    ArrayRef<const Value *> Ptrs, const Value *Base,
-    const TTI::PointersChainInfo &Info, TTI::TargetCostKind CostKind) {
+InstructionCost
+X86TTIImpl::getPointersChainCost(ArrayRef<const Value *> Ptrs,
+                                 const Value *Base,
+                                 const TTI::PointersChainInfo &Info,
+                                 Type *AccessTy, TTI::TargetCostKind CostKind) {
   if (Info.isSameBase() && Info.isKnownStride()) {
     // If all the pointers have known stride all the differences are translated
     // into constants. X86 memory addressing allows encoding it into
     // displacement. So we just need to take the base GEP cost.
     if (const auto *BaseGEP = dyn_cast<GetElementPtrInst>(Base)) {
       SmallVector<const Value *> Indices(BaseGEP->indices());
       return getGEPCost(BaseGEP->getSourceElementType(),
                         BaseGEP->getPointerOperand(), Indices, CostKind);
     }
     return TTI::TCC_Free;
   }
-  return BaseT::getPointersChainCost(Ptrs, Base, Info, CostKind);
+  return BaseT::getPointersChainCost(Ptrs, Base, Info, AccessTy, CostKind);
 }
 
 InstructionCost X86TTIImpl::getAddressComputationCost(Type *Ty,
                                                       ScalarEvolution *SE,
                                                       const SCEV *Ptr) {
   // Address computations in vectorized code with non-consecutive addresses will
   // likely result in more instructions compared to scalar code where the
   // computation can more often be merged into the index mode. The resulting

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp

if (isa<LoadInst, StoreInst>(VL0)) {
       // 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::getUnitStride(), CostKind);
+          Ptrs, BasePtr, TTI::PointersChainInfo::getUnitStride(), ScalarTy,
+          CostKind);
 
       SmallVector<const Value *> PtrsRetainedInVecCode;
       for (Value *V : Ptrs) {
         if (V == BasePtr) {
           PtrsRetainedInVecCode.push_back(V);
           continue;
         }
         auto *Ptr = dyn_cast<GetElementPtrInst>(V);
if (isa<LoadInst, StoreInst>(VL0)) {
         // 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::getKnownStride(), CostKind);
+          TTI::PointersChainInfo::getKnownStride(), VecTy, 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).
       TTI::PointersChainInfo PtrsInfo =
           all_of(Ptrs,
                  [](const Value *V) {
                    auto *Ptr = dyn_cast<GetElementPtrInst>(V);
                    return Ptr && !Ptr->hasAllConstantIndices();
                  })
               ? TTI::PointersChainInfo::getUnknownStride()
               : TTI::PointersChainInfo::getKnownStride();
 
-      ScalarCost = TTI->getPointersChainCost(Ptrs, BasePtr, PtrsInfo, CostKind);
+      ScalarCost = TTI->getPointersChainCost(Ptrs, BasePtr, PtrsInfo, ScalarTy,
+                                             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(),

llvm/test/Transforms/SLPVectorizer/RISCV/getpointerschaincost.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 2
 ; RUN: opt -S -mtriple=riscv64 -mattr=+v -riscv-v-slp-max-vf=0 -passes=slp-vectorizer -pass-remarks-output=%t < %s | FileCheck %s
 ; RUN: FileCheck --input-file=%t --check-prefix=YAML %s
 
 ; Because all of these addresses are foldable, the scalar cost should be 0 when
 ; computing the pointers chain cost.
 ;
 ; TODO: These are currently costed as free the indices are all constants, but we
 ; should check if the constants are actually foldable

 ; CHECK-NEXT:    [[P1:%.*]] = getelementptr i32, ptr [[DEST]], i32 2048
 ; CHECK-NEXT:    store <4 x i32> <i32 1, i32 1, i32 1, i32 1>, ptr [[P1]], align 4
 ; CHECK-NEXT:    ret void
 ;
 entry:
 ; YAML:      Pass:            slp-vectorizer
 ; YAML-NEXT: Name:            StoresVectorized
 ; YAML-NEXT: Function:        g
 ; YAML-NEXT: Args:
 ; YAML-NEXT:   - String:          'Stores SLP vectorized with cost '
 ; YAML-NEXT:   - Cost:            '-2'
 ; YAML-NEXT:   - String:          ' and with tree size '
 ; YAML-NEXT:   - TreeSize:        '2'
   %p1 = getelementptr i32, ptr %dest, i32 2048
   store i32 1, ptr %p1
   %p2 = getelementptr i32, ptr %dest, i32 2049
   store i32 1, ptr %p2
   %p3 = getelementptr i32, ptr %dest, i32 2050
   store i32 1, ptr %p3
   %p4 = getelementptr i32, ptr %dest, i32 2051
   store i32 1, ptr %p4
   ret void
 }
 
-; FIXME: When computing the scalar pointers chain cost here, there is a cost of
+; When computing the scalar pointers chain cost here, there is a cost of
 ; 1 for the base pointer, and the rest can be folded in, so the scalar cost
 ; should be 1.
 define void @h(ptr %dest, i32 %i) {
 ; CHECK-LABEL: define void @h
 ; CHECK-SAME: (ptr [[DEST:%.*]], i32 [[I:%.*]]) #[[ATTR0]] {
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[P1:%.*]] = getelementptr [4 x i32], ptr [[DEST]], i32 [[I]], i32 0
 ; CHECK-NEXT:    store <4 x i32> <i32 1, i32 1, i32 1, i32 1>, ptr [[P1]], align 4
 ; CHECK-NEXT:    ret void
 ;
 entry:
 ; YAML:      Pass:            slp-vectorizer
 ; YAML-NEXT: Name:            StoresVectorized
 ; YAML-NEXT: Function:        h
 ; YAML-NEXT: Args:
 ; YAML-NEXT:   - String:          'Stores SLP vectorized with cost '
-; YAML-NEXT:   - Cost:            '-5'
+; YAML-NEXT:   - Cost:            '-2'
 ; YAML-NEXT:   - String:          ' and with tree size '
 ; YAML-NEXT:   - TreeSize:        '2'
   %p1 = getelementptr [4 x i32], ptr %dest, i32 %i, i32 0
   store i32 1, ptr %p1
   %p2 = getelementptr [4 x i32], ptr %dest, i32 %i, i32 1
   store i32 1, ptr %p2
   %p3 = getelementptr [4 x i32], ptr %dest, i32 %i, i32 2
   store i32 1, ptr %p3
   %p4 = getelementptr [4 x i32], ptr %dest, i32 %i, i32 3
   store i32 1, ptr %p4
   ret void
 }

llvm/test/Transforms/SLPVectorizer/RISCV/struct-gep.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -passes=slp-vectorizer -mtriple=riscv64 -mattr=+v \
 ; RUN: -riscv-v-slp-max-vf=0 -S | FileCheck %s
 
-; FIXME: This should not be vectorized
+; This shouldn't be vectorized as the extra address computation required for the
+; vector store make it unprofitable (vle/vse don't have an offset in their
+; addressing modes)
 
 %struct.2i32 = type { i32, i32 }
 
 define void @splat_store_v2i32(ptr %dest, i64 %i) {
 ; CHECK-LABEL: @splat_store_v2i32(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[P1:%.*]] = getelementptr [[STRUCT_2I32:%.*]], ptr [[DEST:%.*]], i64 [[I:%.*]], i32 0
-; CHECK-NEXT:    store <2 x i32> <i32 1, i32 1>, ptr [[P1]], align 4
+; CHECK-NEXT:    store i32 1, ptr [[P1]], align 4
+; CHECK-NEXT:    [[P2:%.*]] = getelementptr [[STRUCT_2I32]], ptr [[DEST]], i64 [[I]], i32 1
+; CHECK-NEXT:    store i32 1, ptr [[P2]], align 4
 ; CHECK-NEXT:    ret void
 ;
 entry:
   %p1 = getelementptr %struct.2i32, ptr %dest, i64 %i, i32 0
   store i32 1, ptr %p1
   %p2 = getelementptr %struct.2i32, ptr %dest, i64 %i, i32 1
   store i32 1, ptr %p2
   ret void
 }