[PATCH] D145163: Add support for vectorization of interleaved memory accesses for scalable VF

[mgabka via Phabricator via llvm-commits]

mgabka added inline comments.


================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:835-839
+  bool hasInterleavedLoad(VectorType *VecTy, Value *Addr, uint32_t Factor,
+                          bool IsMasked) const;
+
+  bool hasInterleavedStore(SmallVectorImpl<Value *> &StoredVecs, Value *Addr,
+                           uint32_t Factor, bool IsMasked) const;
----------------
I think these functions could actually be joined into 1, something like:

 supportsInterleaving(VectorType *VecTy, uint32_t Factor, bool IsMasked)

I think it is going to be unlikely that target supports store but not load for the same vector type, @paulwalker-arm what do you think?


================
Comment at: llvm/lib/IR/IRBuilder.cpp:587
+  if (Factor != 2)
+    return nullptr;
+  assert(Ty->isVectorTy() && "Type should be vector");
----------------
luke wrote:
> Maybe this should be an assertion
Hi Luke, 
so I could remove the Factor argument entirely and make this function specific for just Factor=2 (and use assertions) what makes sense for now as the experimental interleaving intrinsics are only for factor 2.
 


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2721
+        // Check if we can create target specific interleaving load.
+        if (TTI->hasInterleavedLoad(VecTy, AddrParts[Part], Group->getFactor(),
+                                    false))
----------------
luke wrote:
> Need to check that `Group->getFactor() == 2` here or that the call to CreateMaskedInterleavedLoad succeeds
So my idea was that it would be up to the hasInterleavedLoad function to make sure that it returns true only when Factor is 2, so no extra checks is needed I think.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:2751
+          StridedVec = Builder.CreateShuffleVector(NewLoads[Part], StrideMask,
+                                                   "strided.vec");
 
----------------
luke wrote:
> It's somehow possible to reach here with a scalable vector type if `TII->hasInterleavedLoad` returns false. 
> Can we check somewhere inside the vectorizer cost model that if `hasInterleavedLoad` is false then we rule out any recipe with an interleave group for a scalable VF?
So it is actually connected by the LV cost model, the LoopVectorizationCostModel::getInterleaveGroupCost is calling TTI.getInterleavedMemoryOpCost which should return invalid cost for factors different than 2. 


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

https://reviews.llvm.org/D145163