[PATCH] D19501: Add LoadStoreVectorizer pass

[Alina Sbirlea via llvm-commits]

asbirlea added a comment.

Example in the previous comment should produce correct code with these changes.


================
Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:322
@@ +321,3 @@
+    } else if (NumFound == Chain.size()) {
+      LastInstr = I.getIterator();
+      break;
----------------
This should be: LastInstr = ++I; (using updated patch)
It's always valid: either uses the next valid instruction or BB.end().

Fixes issue that isVectorizable misses testing the last instruction in the chain.

================
Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:376
@@ +375,3 @@
+  }
+
+  for (auto EntryMem : MemoryInstrs) {
----------------
Consider asserting Chain.size() == ChainInstrs.size()?

================
Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:390
@@ +389,3 @@
+      // first load in the chain).
+      if (isa<StoreInst>(V) && isa<LoadInst>(VV) && VVIdx < VIdx)
+        continue;
----------------
As loads are inserted at the location of the last load, either invert sign (VVIdx > VIdx) and update comment, or fix load insert location.
The former misses vectorization opportunities, but it avoids (at least some of the) incorrect code generation.

================
Comment at: lib/Transforms/Vectorize/LoadStoreVectorizer.cpp:394
@@ +393,3 @@
+      // Same case, but in reverse.
+      if (isa<LoadInst>(V) && isa<StoreInst>(VV) && VVIdx > VIdx)
+        continue;
----------------
Same as above.


http://reviews.llvm.org/D19501