[PATCH] D19984: [LV] Preserve order of dependences in interleaved accesses analysis
Adam Nemet via llvm-commits
llvm-commits at lists.llvm.org
Thu May 19 10:21:39 PDT 2016
anemet added a comment.
I am not done reviewing this but I just want to say that I am having a much better feeling about this analysis after your changes. As I said I had major issues with its soundness. Thanks for your patience for working through all this!
I am also sending my initial comments but there may be more coming.
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:947-949
@@ -934,1 +946,5 @@
+
+ /// \brief Returns true if memory accesses \p A and \p B can be reordered. We
+ /// use LoopAccessInfo to determine if any dependences may be violated.
+ bool canReorderMemAccs(Instruction *A, Instruction *B);
};
----------------
Somewhere this should state the criteria for reordering. In theory you can reorder backward dependences but looks like you don't allow any (which is fine).
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5058
@@ -5036,3 +5057,3 @@
StrideAccesses[I] = StrideDescriptor(Stride, Scev, Size, Align);
}
----------------
I am wondering if it's time to change the name of this. After your change it no longer contains only "interesting" strided accesses. How about like AccessStrideInfo?
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5062
@@ -5040,2 +5061,3 @@
-// Analyze interleaved accesses and collect them into interleave groups.
+bool InterleavedAccessInfo::canReorderMemAccs(Instruction *A, Instruction *B) {
+
----------------
I think that we've been using Accesses pretty consistently without any abbreviation.
================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5093-5095
@@ +5092,5 @@
+// location of the last store. This code motion can change the order of load
+// and store instructions and may break dependences. The inserted run-time
+// pointer aliasing checks ensure the absence of some dependences. However, we
+// need to explicitly check that we won't violate the dependences allowed for
+// vectorization.
----------------
I would drop the part about memchecks. This is already pretty long and that part is trivial.
http://reviews.llvm.org/D19984
More information about the llvm-commits
mailing list