[PATCH] D149654: [SLP] Don't cost pointers that can be folded in getPointersChainCost

[Luke Lau via Phabricator via llvm-commits]

luke added inline comments.


================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfoImpl.h:1066
+        // addressing mode, then we don't need an extra ADD operation
+        unsigned Stride = DL.getTypeStoreSize(MemOpTy);
+        if (Info.isUniformStride() &&
----------------
reames wrote:
> 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?)
>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.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149654/new/

https://reviews.llvm.org/D149654