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

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope marked 2 inline comments as done.
bjope added inline comments.


================
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});
----------------
kuhar wrote:
> 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.
Ok, great. Thanks for all the feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110292