[PATCH] [PlaceSafepoints] Remove dependence on LoopSimplify

[Sanjoy Das]

================
Comment at: lib/Transforms/Scalar/PlaceSafepoints.cpp:571
@@ +570,3 @@
+    auto &PollLocations = PBS->PollLocations;
+    PollLocations.erase(std::unique(PollLocations.begin(),
+                                    PollLocations.end()),
----------------
`std::unique` only removes consecutive elements so I don't think it would work with an unsorted array.  I think it is sufficient to just move this after the `std::sort` on `PollLocations` below.

================
Comment at: lib/Transforms/Scalar/PlaceSafepoints.cpp:575
@@ +574,3 @@
+
+    auto order_by_bb_name = [](Instruction *a, Instruction *b) {
+      return -1 == a->getParent()->getName().compare(b->getParent()->getName());
----------------
LLVM naming conventions say:

```
auto OrderByBBName = [](Instruction *A, Instruction *A) {
```

================
Comment at: lib/Transforms/Scalar/PlaceSafepoints.cpp:576
@@ +575,3 @@
+    auto order_by_bb_name = [](Instruction *a, Instruction *b) {
+      return -1 == a->getParent()->getName().compare(b->getParent()->getName());
+    };
----------------
`StringRef` has an `operator<`.

================
Comment at: lib/Transforms/Scalar/PlaceSafepoints.cpp:604
@@ -598,3 +603,3 @@
         for (unsigned i = 0; i < Term->getNumSuccessors(); i++) {
           BasicBlock *Succ = Term->getSuccessor(i);
           if (DT.dominates(Succ, Term->getParent())) {
----------------
Side comment, unrelated to this change:  this nested loop uses `i`, the same induction variable as the outer loop which can be confusing.  It might be clearer to change both these loops to be range for loops.

================
Comment at: lib/Transforms/Scalar/PlaceSafepoints.cpp:616
@@ -610,3 +615,3 @@
         for (BasicBlock *Header : Headers) {
           BasicBlock *NewBB = SplitEdge(Term->getParent(), Header, nullptr);
           SplitBackedges.insert(NewBB);
----------------
Side comment that has nothing to do with this change:  `SplitEdge` optionally takes a `DominatorTree` as input -- if the reason for doing that is that `SplitEdge` updates the dom tree, then we can just pass in the DT to `SplitEdge` not recalculate it below.

http://reviews.llvm.org/D9585

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/