[PATCH] D36113: [Loop Vectorize] Vectorize Loops with Backward Dependence

[Daniel Berlin via Phabricator via llvm-commits]

dberlin added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorizePred.cpp:111
+
+bool LoopInstructionsReordering::checkDepAndReorder(
+    BasicBlock::iterator StoreInstIter, BasicBlock::iterator LoadInstIter) {
----------------
hfinkel wrote:
> Please add a comment explaining what this function does.
This funciton looks like it tries to figure out if the operands can be moved upward.  You don't need to iterate through the instructions at all to do this.
You can just use the def-use chains and stop when your chain is above the other instruction.
If you need local-to-block ordering, use OrderedInstructions->dominates to tell when you are above the 


================
Comment at: lib/Transforms/Vectorize/LoopVectorizePred.cpp:179
+      MemoryAccess *D ;
+      if (MemoryUseOrDef *NewMemAcc =
+                      dyn_cast_or_null<MemoryUseOrDef>(MemAcc))
----------------
You are guaranteed this is a MemoryUseOrDef, since oyu asked about a load.
There is no need for a dyn_cast_or_null, a simple cast will suffice.


================
Comment at: lib/Transforms/Vectorize/LoopVectorizePred.cpp:184
+        return reordered;
+      if (MSSA->isLiveOnEntryDef(D)) {
+
----------------
This check does not make much sense.
The right check to see if it's not dependent in the loop would be to see if the defining access is above the loop. As hal says, that makes it loop invariant.
The right check to see if it not dependent on the store would be to see if the defining access dominates (in the global sense, so if it's part of the same block, it must come before) the store, and is not part of an SCC of the memoryssa use-def graph that contains the store's memory access.

(If it is not part of an scc, it cannot be changing in the loop. If the store's memory access does not appear in the SCC, it cannot be part of whatever cycle affects this load)

IE even in the case that the store is below the load:

a = memoryphi(foo, bar)
baz = memorydef(a)
memoryuse(baz)
bar = memorydef(baz)

This will give the right answer. 




https://reviews.llvm.org/D36113