[PATCH] D110292: Use a deterministic order when updating the DominatorTree

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar added inline comments.


================
Comment at: llvm/include/llvm/Support/GenericDomTree.h:552
+  /// and the reverse of these updates provides the pre-view of the CFG.
   /// \param PostViewUpdates An unordered sequence of update to perform in order
   /// to obtain a post-view of the CFG. The DT will be updated assuming the
----------------
unordered -> ordered also here?


================
Comment at: llvm/lib/CodeGen/IndirectBrExpandPass.cpp:265-267
+    for (BasicBlock *BB : BBs) {
+      if (UniqueSuccessors.insert(BB).second)
+        Updates.push_back({DominatorTree::Insert, SwitchBB, BB});
----------------
This pattern appears many time in the patch. I wonder if we could have a helper function that helps with that. Perhaps something like `insert_range(InsertTo, NewElements) -> RangeOfInserted`:

```
for (BasicBlock* Inserted : insert_range(UniqueSuccessors, BBs))
  Updates.push_back(...);
                            
```



================
Comment at: llvm/test/Transforms/JumpThreading/domtree-updates.ll:11
+; happened to trigger a non-determinism quite often when being executed
+; multipe times (I could typically see varying results when running the test
+; less that 10 times in a row).
----------------
nit: I -> We or `it was possible to see varrying'


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110292/new/

https://reviews.llvm.org/D110292