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

[Michael Zolotukhin]

Hi Hao,

The patch LGTM with some minor notes. Thank you for the efforts, your work is very appreciated!

Michael


================
Comment at: lib/Analysis/LoopAccessAnalysis.cpp:838-839
@@ +837,4 @@
+  // Two accesses in memory (stride is 2):
+  //     | A[0] |      | A[2] |      | A[4] |      | A[6] |      |
+  //                              | B[0] |      | B[2] |      | B[4] |
+  //
----------------
Something went wrong with the indentation here.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:667
@@ +666,3 @@
+private:
+  unsigned Delta; // Interleave Factor.
+  bool Reverse;
----------------
Could we rename `Delta` to `InterleaveFactor` or something like this. When roaming through the code it'd be much easier to understand what it stands for - especially when the code is in-tree and one doesn't see the context of the current patch.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:690
@@ +689,3 @@
+///
+/// Call this class to analyze interleaved accesses only when we can vectorize
+/// a loop. Otherwise it's meaningless to do analysis as the vectorization
----------------
s/Call this class/Use this class/

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4455
@@ +4454,3 @@
+    if (std::abs(Stride) < 2 ||
+        std::abs(Stride) > static_cast<int>(MaxInterleaveStride))
+      continue;
----------------
static_cast isn't needed here.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4552-4556
@@ +4551,7 @@
+
+      // Previous dependence check guarantees no dependence between two accesses
+      // if the distance is not multiple of the size. So just ignore such cases
+      // and won't worry about dependences.
+      if (DistanceToA % static_cast<int>(DesA.Size))
+        continue;
+
----------------
What is the previous check you are referring to? And if it's guaranteed, could this `if` be replaced with an assertion?

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:4560
@@ +4559,3 @@
+      int IndexB =
+          Group->getIndex(A) + DistanceToA / static_cast<int>(DesA.Size);
+
----------------
`static_cast` isn't necessary here (and if we want to cast everything  `getIndex` should also be casted from `unsigned`).

================
Comment at: test/Transforms/LoopVectorize/interleaved-accesses.ll:1
@@ +1,2 @@
+; RUN: opt -S -loop-vectorize -instcombine -force-vector-width=4 -force-vector-interleave=1 -enable-interleaved-mem-accesses=true -runtime-memory-check-threshold=24 < %s | FileCheck %s
+
----------------
Which of the tests needs `-runtime-memory-check-threshold=24`? Could we get rid of this flag by adding `noalias` attribute, or aliasing metadata?

================
Comment at: test/Transforms/LoopVectorize/interleaved-accesses.ll:39
@@ +38,3 @@
+  %arrayidx0 = getelementptr inbounds [1024 x i32], [1024 x i32]* @AB, i64 0, i64 %indvars.iv
+  %0 = load i32, i32* %arrayidx0, align 4
+  %1 = or i64 %indvars.iv, 1
----------------
Please run the tests through `opt -instnamer` to replace %[0-9]+ names - they are really hard to deal with when one need to modify the test.

http://reviews.llvm.org/D9368

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