[PATCH] D26905: [SLP] Vectorize loads of consecutive memory accesses, accessed in non-consecutive (jumbled) way.

Matthew Simpson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Dec 15 08:55:07 PST 2016

mssimpso added inline comments.

Comment at: include/llvm/Analysis/LoopAccessAnalysis.h:695
+/// access.
+SmallVector<Value *, 8> sortMemAccess(ArrayRef<Value *> VL, const DataLayout &DL,
+                                      ScalarEvolution &SE);
Should this be renamed to sortMemAccesses? If so, the comment above should also be updated: "jumbled memory accesses".

Also, should we be returning a SmallVector here? We could also pass a SmallVectorImpl<Value *> &Sorted to the function and place the sorted values there.

Comment at: lib/Analysis/LoopAccessAnalysis.cpp:1068
+    // Compute the constant offset from the base pointer of each memory access
+    // and insert into the multimap to ensure it is sorted.
+    Value *Ptr = getPointerOperand(Val);
Please update comment since you're no longer using a multimap.

Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:2565
+      // must be followed by a 'shuffle' with the required jumbled mask.
+      if (E->NeedToShuffle && (VL.size() == E->Scalars.size())) {
+        SmallVector<Constant *, 8> Mask;
ashahid wrote:
> mssimpso wrote:
> > I probably missed this, but why are we checking the sizes? Does this mean there will be cases where E->NeedToShuffle is true but we don't generate the shuffle?
> No, I want to ensure that resulting vector type is not differing due to the length of the vector value.
I don't think I fully understand this yet. Can you please make the comment more detailed. In particular, when does VL.size() not equal Scalars.size()? Is this the case when a bundle gets split up into smaller chunks? And then if this is true, what does it imply for the jumbled accesses. It looks like we will end up with a vector load still, but then when are they placed in the right order? Sorry if this should all be obvious!

Comment at: lib/Transforms/Vectorize/SLPVectorizer.cpp:413
-  /// Vectorize a single entry in the tree.
-  Value *vectorizeTree(TreeEntry *E);
+  /// Vectorize a single entry in the tree.VL is the scalars in program order.
+  Value *vectorizeTree(ArrayRef<Value *> VL, TreeEntry *E);
Can we be a bit more explicit about VL. Are VL the scalar roots of the vectorizable tree?


More information about the llvm-commits mailing list