[PATCH] D19984: [LV] Handle RAW dependences in interleaved access analysis

[Matthew Simpson via llvm-commits]

mssimpso added a comment.

Hi Silviu, thanks for the comments.

> I think non-zero dependences would have already been rejected by LAA. Would this this be the reason why it is correct to only look at the zero distance ones?


Yes, that should be the case!


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:932
@@ +931,3 @@
+    if (!Distance && R->mayReadFromMemory() && W->mayWriteToMemory() &&
+        !DT->dominates(R, W))
+      return true;
----------------
sbaranga wrote:
> I guess we can check this with DT because the loads/stores are not predicated? (same in a bunch of other places).
> 
> 
I intended the dominance check to work for the predicated accesses as well. The check says that if the read does not dominate the write, then we have to conservatively assume the write may happen first, leading to a read-after-write dependence.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5162
@@ +5161,3 @@
+          }
+        LoopIndependentRAWStores.push_back(B);
+      }
----------------
sbaranga wrote:
> Wouldn't we need to add B to LoopIndependentRAWStores even if we add it to StoresToRemove?
StoresToRemove holds stores whose groups will definitely be removed. LoopIndependentRAWStores holds stores that we don't yet have enough information about to determine if they need to be removed or not.

In this case, if the current store (B) is already in a group, it means that it will be re-ordered by sinking it to the insert location of the group, violating the dependence. If it's not yet in a group (and thus, won't be re-ordered), we don't yet know that the load (A) won't be hoisted, which would also violate the dependence. Once we know that another load has been added to A's group, we know that A will be re-ordered. When this happens, we move all the stores in LoopIndependentRAWStores to StoresToRemove, to mark them for definite removal.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5180
@@ +5179,3 @@
+      // the store for removal, and prevent further loads from being added to
+      // the group by breaking.
+      if (A->mayReadFromMemory() && !LoopIndependentRAWStores.empty()) {
----------------
sbaranga wrote:
> This sentence seems to be unfinished?
Thanks for catching that. I'll submit an update


http://reviews.llvm.org/D19984