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

[Michael Zolotukhin]

Please find more inline comments from me.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3833-3835
@@ +3832,5 @@
+    SmallVector<Instruction *, 4> &Queue, int Stride, bool IsLoad) {
+  unsigned QSize = Queue.size();
+  if (QSize < 2)
+    return;
+
----------------
It'd better be
```
if (Queue.size() < 2)
  return;
```
since `QSize` isn't used anywhere else.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3837-3852
@@ +3836,18 @@
+
+  SetVector<Instruction *> Heads;
+  SetVector<Instruction *> Tails;
+  SmallDenseMap<Instruction *, Instruction *> ConsecutiveChain;
+
+  // Collect all the consecutive pairs.
+  const DataLayout &DL = TheLoop->getHeader()->getModule()->getDataLayout();
+  for (auto &H : Queue) {
+    for (auto &T : Queue) {
+      if (H == T || !isConsecutiveAccess(H, T, SE, DL))
+        continue;
+
+      Heads.insert(H);
+      Tails.insert(T);
+      ConsecutiveChain[H] = T;
+    }
+  }
+
----------------
This is very similar to the code from SLP. Any chance to reuse it?

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3868
@@ +3867,3 @@
+    unsigned Size = ConsecutiveList.size();
+    unsigned Num = std::abs(Stride);
+    if (Size < Num)
----------------
This name is misleading.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3890-3891
@@ +3889,4 @@
+      }
+      // The alignment of the interleaved access is the max member alignment.
+      IA->Align = MaxAlign;
+
----------------
Shouldn't it be minimal alignment instead?

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3921-3922
@@ +3920,4 @@
+  }
+
+  return;
+}
----------------
This is redundant.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3951
@@ +3950,3 @@
+    SmallVector<Instruction *, 4> AccessQueue;
+    bool TryMerge = false;
+    Value *CurBase = nullptr;
----------------
When I first read this code, this name arose a question "Merge with what?" to me.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3955-3963
@@ +3954,11 @@
+    bool IsLoad = false;
+    for (BasicBlock::iterator it = (*bb)->begin(), e = (*bb)->end(); it != e;
+         ++it) {
+      if (TryMerge) {
+        collectInterleavedAccess(AccessQueue, CurStride, IsLoad);
+        AccessQueue.clear();
+        CurBase = nullptr;
+        TryMerge = false;
+      }
+
+      if (!it->mayReadOrWriteMemory())
----------------
mzolotukhin wrote:
> Same here.
This loop is much more complicated than it should be I think.

If we can combine ` if (!StridedAccesses.count(it))` with ` if (CurBase != Base || CurStride != Stride || IsLoad != Read)`, we'll be able to get rid of `TryMerge` and of all these jumps to beginning of the loop.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3988
@@ +3987,3 @@
+
+      // This instructure doesn't fit the current queue. Try to merge current
+      // queue and insert it again.
----------------
s/instructure/instruction/

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4004-4006
@@ +4003,5 @@
+      collectInterleavedAccess(AccessQueue, CurStride, IsLoad);
+      AccessQueue.clear();
+      CurBase = nullptr;
+      TryMerge = false;
+    }
----------------
This seems to be redundant.

http://reviews.llvm.org/D9368

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