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

Renato Golin renato.golin at linaro.org
Mon May 11 13:25:36 PDT 2015


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3758
@@ +3757,3 @@
+
+  SetVector<Instruction *> Heads;
+  SetVector<Instruction *> Tails;
----------------
An explanation of what heads and tails are would be good. If I read correctly, it's not just the first and last accesses, but every first/last access in the list of consecutive pairs. So, in your example:

    A[i]
    A[i+1]
    A[i+2]

Heads:

    A[i], A[i+1]

Respective tails:

    A[i+1], A[i+2]

right?

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3776
@@ +3775,3 @@
+      //      A[i] (2)
+      //      A[i]+A[i+1] (1)(2)
+      // Then we get incorrect result.
----------------
You probably mean:

    //      A[i]+A[i+1] (1)(3)

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3787
@@ +3786,3 @@
+
+  for (auto &I : Heads) {
+    if (Tails.count(I))
----------------
I'd call this auto variable "Head", so you can use I for the load/store loops at the end.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3788
@@ +3787,3 @@
+  for (auto &I : Heads) {
+    if (Tails.count(I))
+      continue;
----------------
Haven't you checked for this already up there?

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3793
@@ +3792,3 @@
+    // Collect a chain with consecutive accesses.
+    auto Inst = I;
+    while (Tails.count(Inst) || Heads.count(Inst)) {
----------------
You don't seem to be using I or Inst any more, you could declare Inst inside the loop, like:

    for (auto Inst = I; Tails.count(Inst) || Heads.count(Inst); ) {
      ...
    }

However, I don't understand why you didn't just:

    for (auto Inst = I; ConsecutivePairs.count(Inst); ) {
      ...
    }


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3803
@@ +3802,3 @@
+    unsigned IALen = std::abs(Stride);
+    if (ChainLen < IALen)
+      continue;
----------------
Shouldn't we guarantee that:

    ChainLen % IALen == 0

?

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3826
@@ +3825,3 @@
+          MinAlign = Align;
+        else if (Align && Align < MinAlign)
+          MinAlign = Align;
----------------
std::min()?

http://reviews.llvm.org/D9368

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






More information about the llvm-commits mailing list