[PATCH] [LoopVectorize]Teach Loop Vectorizer about interleaved memory access

Michael Zolotukhin mzolotukhin at apple.com
Wed May 6 14:06:54 PDT 2015


Hi Hao,

First of all, thanks for working on this!
Below is the first round of comments from me, I'll get back to it again to dig into more interesting parts.

BTW, what are the problems that prevents us from getting performance benefits?


================
Comment at: lib/Analysis/LoopAccessAnalysis.cpp:776-779
@@ -779,8 +775,6 @@
                            VectorizerParams::VectorizationInterleave : 1);
-
-  // The distance must be bigger than the size needed for a vectorized version
-  // of the operation and the size of the vectorized operation must not be
-  // bigger than the currrent maximum size.
-  if (Distance < 2*TypeByteSize ||
-      2*TypeByteSize > MaxSafeDepDistBytes ||
-      Distance < TypeByteSize * ForcedUnroll * ForcedFactor) {
+  unsigned NumIterations = ForcedFactor * ForcedUnroll;
+  // The minimum number of vectorized & interleaved iterations is 2.
+  if (NumIterations < 2)
+    NumIterations = 2;
+
----------------
>From your and Renato's comments I think it's more appropriate to have an assert here. Otherwise, this is very misleading.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:581
@@ +580,3 @@
+    SmallSet<InterleavedAccess *, 4> DelSet;
+    // Collecting all the pointers to avoid delete a pointer twice,
+    for (auto &I : InterleavedAccessMap)
----------------
Nitpick: s/twice,/twice./

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:1972
@@ +1971,3 @@
+  }
+  return;
+}
----------------
This looks redundant.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3763-3764
@@ +3762,4 @@
+
+static bool isConsecutiveAccess(Instruction *A, Instruction *B,
+                                ScalarEvolution *SE, const DataLayout &DL) {
+  Value *PtrA = getPointerOperand(A);
----------------
This code is a dupe of the one from SLP vectorizer. Could we do anything about it?

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3943-3945
@@ +3942,5 @@
+  //        load A[i], store A[i], load A[i+1], store A[i+1].
+  for (Loop::block_iterator bb = TheLoop->block_begin(),
+                            be = TheLoop->block_end();
+       bb != be; ++bb) {
+    if (blockNeedsPredication(*bb))
----------------
Auto + range loops could be used here.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3955-3956
@@ +3954,4 @@
+    bool IsLoad = false;
+    for (BasicBlock::iterator it = (*bb)->begin(), e = (*bb)->end(); it != e;
+         ++it) {
+      if (TryMerge) {
----------------
Same here.

http://reviews.llvm.org/D9368

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list