[PATCH] D19984: [LV] Preserve order of dependences in interleaved accesses analysis

[silviu.baranga@arm.com via llvm-commits]

sbaranga added a comment.

I have just a few quick remarks (and most of them were inherited from the existing code).

Cheers,
Silviu


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5071
@@ +5070,3 @@
+  if (!A->mayWriteToMemory() && !B->mayWriteToMemory())
+    return true;
+
----------------
We should improve this at some point to check for NoDep (which is what we're really looking for here) and is implied by this test.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5077
@@ +5076,3 @@
+  // the memory must be disjoint if the checks succeed, it's safe to reorder.
+  auto *Distance =
+      dyn_cast<SCEVConstant>(PSE.getSE()->getMinusSCEV(DesA.Scev, DesB.Scev));
----------------
This makes assumptions on how LAA works. It might be technically true at the moment, but if the dependence analysis would be improved it would possibly invalidate this.

I think the case where we memcheck each pointer is an edge case? I'm not sure it's worth doing the interleaved access vectorization in this case.

The more common case could be if two pointers are in different alias sets or have different underlying objects? But that should be expressed as a NoDep.

================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5093
@@ +5092,3 @@
+  // are dependent. First, we get the interleave factor of the accesses. If
+  // they aren't interleaved or they have different strides, don't reorder.
+  auto FactorA = std::abs(DesA.Stride);
----------------
Same here. Ideally we would know we have NoDep from LAA.

Some of the checks here seem to be specifically related to forming the interleaved groups, and less about reordering accesses.


http://reviews.llvm.org/D19984