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

[silviu.baranga at arm.com]

sbaranga 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());
+
----------------
anemet wrote:
> 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.
Thanks! I've removed the call to std::sort in r242033, I'll update the documentation in another change.


Repository:
  rL LLVM

http://reviews.llvm.org/D11064