[PATCH] D19669: [RewriteStatepointsForGC] Use SetVector/MapVector instead of DenseSet/DenseMap to guarantee stable ordering

[Sanjoy Das via llvm-commits]

sanjoy accepted this revision.
sanjoy added a comment.
This revision is now accepted and ready to land.

This lgtm with some inline comments on the SetOperations bit.


================
Comment at: include/llvm/ADT/SetOperations.h:34
@@ -31,2 +33,3 @@
   return Changed;
 }
+/// set_union(A, B) - Compute A := A u B, return whether A changed.
----------------
Given that these functions only work with `SetVector`, why not just add these as member functions on `SetVector` (with the same TODO there as you have here)?

================
Comment at: include/llvm/ADT/SetOperations.h:35
@@ -32,1 +34,3 @@
 }
+/// set_union(A, B) - Compute A := A u B, return whether A changed.
+/// Special case for SetVector.
----------------
Add a blank line here, same for `set_subtract`

================
Comment at: include/llvm/ADT/SetOperations.h:39
@@ +38,3 @@
+template <class S1Ty, class S2Ty>
+bool set_union(SetVector<S1Ty> &S1, const S2Ty &S2) {
+  bool Changed = false;
----------------
The first template args should probably be named `V1Ty` or `El1Ty`, since it is no longer a "set" type.  Same for `set_subtract`.


http://reviews.llvm.org/D19669