[PATCH] D110292: Use a deterministic order when updating the DominatorTree
Jakub Kuderski via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 26 07:00:30 PST 2021
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
More information about the llvm-commits
mailing list