[PATCH] D16197: [LV] Vectorize first-order recurrences

[Adam Nemet via llvm-commits]

anemet added a comment.

Hi Matt,

I've started reviewing this but I found that I could use an example of this transformation fully spelled out.  Particularly, I am not sure I understand why the initial vector value is a splat vector.

Thanks,
Adam


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:367
@@ -366,1 +366,3 @@
 
+  /// Vectorize first-order recurrences.
+  void fixFirstOrderRecurrence(PHINode *Phi);
----------------
I guess a definition of first-order recurrence should be added here.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3323-3324
@@ +3322,4 @@
+    //
+    // FIXME: We need to refactor this function since the recurrences in
+    //        RdxPHIsToFix are not necessarily reductions.
+    if (Legal->isFirstOrderRecurrence(RdxPhi)) {
----------------
I actually think we need to this before this patch ;).  This is a 200-line loop and we shouldn't pile on more strangeness.

I think that all we need to do is to rename RdxPHIsToFix to PHIsToFix in a prequel NFC patch.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3326
@@ +3325,3 @@
+    if (Legal->isFirstOrderRecurrence(RdxPhi)) {
+      fixFirstOrderRecurrence(RdxPhi);
+      continue;
----------------
I know that we use fix... in other places but that's pretty general.  How about vectorize... or something like that.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:3564-3567
@@ +3563,6 @@
+  // recurrences are of the form "current = phi(initial, previous)".
+  auto *Initial = Phi->getIncomingValue(0);
+  auto *Previous = Phi->getIncomingValue(1);
+  if (!OrigLoop->isLoopInvariant(Initial))
+    std::swap(Initial, Previous);
+
----------------
assert that Previous is loop-invariant at this point?


http://reviews.llvm.org/D16197