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

[Jakub Kuderski via Phabricator via llvm-commits]

kuhar added a comment.

I think it would be good to send the DomTree documentation changes separately as they are a clear improvement regardless of whether this patch goes in or not.



================
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});
----------------
lebedev.ri wrote:
> kuhar wrote:
> > 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(...);
> >                             
> > ```
> > 
> I tried doing that in D99444, but reviewers weren't convinced and i lacked motivation to follow-up.
Then I think it's fine as is, and we could clean it up separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110292