[PATCH] D48392: [Dominators] Simplify child lists and make them deterministic

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar added a subscriber: NutshellySima.
kuhar added inline comments.


================
Comment at: include/llvm/Support/GenericDomTreeConstruction.h:1221
     // that case.
     SmallDenseMap<std::pair<NodePtr, NodePtr>, int, 4> Operations;
     Operations.reserve(AllUpdates.size());
----------------
Won't this also introduce non-determinism in the case you described?


================
Comment at: include/llvm/Support/GenericDomTreeConstruction.h:1255
 
     llvm::sort(Result.begin(), Result.end(),
                [&Operations](const UpdateT &A, const UpdateT &B) {
----------------
Related to above.


================
Comment at: include/llvm/Support/GenericDomTreeConstruction.h:1272
+           FS.back().getInt() == CurrentUpdate.getKind());
+    FS.pop_back();
     if (FS.empty()) BUI.FutureSuccessors.erase(CurrentUpdate.getFrom());
----------------
I think this deserves a comment -- it's only obvious that the successor/predecessor is at the end once you realize updates are performed in the same order as they happened to be legalized.


Repository:
  rL LLVM

https://reviews.llvm.org/D48392