[PATCH] D11064: Don't rely on the DepCands iteration order when constructing checking pointer groups

[Adam Nemet]

anemet added inline comments.

================
Comment at: llvm/trunk/lib/Analysis/LoopAccessAnalysis.cpp:245-253
@@ -232,4 +244,11 @@
 
-    for (auto MI = DepCands.member_begin(DI), ME = DepCands.member_end();
+    // Get all indeces of the members of this equivalence class and sort them.
+    // This will allow us to process all accesses in the order in which they
+    // were added to the RuntimePointerCheck.
+    for (auto MI = DepCands.member_begin(LeaderI), ME = DepCands.member_end();
          MI != ME; ++MI) {
       unsigned Pointer = PositionMap[MI->getPointer()];
+      MemberIndices.push_back(Pointer);
+    }
+    std::sort(MemberIndices.begin(), MemberIndices.end());
+
----------------
sbaranga wrote:
> anemet wrote:
> > anemet wrote:
> > > s/indeces/indices
> > > 
> > Are you positive we need to sort the members too?
> > 
> > I think these are hooked up to the equivalence class in program order and it's effectively a linked list, so they should preserve the chaining order.
> > 
> > Either way it needs a comment.
> I think that's correct, they don't actually need to be sorted - at least with the current implementation of equivalence classes. I think the question is if this is a guarantee or an internal implementation detail. I can't find any place that explicitly makes this guarantee.
Sounds like something that is worth documenting (it removes redundant work in our case).

People can object if there is a reason to keep this unspecified but I don't see why that would be useful.


Repository:
  rL LLVM

http://reviews.llvm.org/D11064