[PATCH] D32740: [PM/Unswitch] Fix a bug in the domtree update logic for the new unswitch pass.

[Chandler Carruth via Phabricator via llvm-commits]

chandlerc added inline comments.


================
Comment at: lib/Transforms/Scalar/SimpleLoopUnswitch.cpp:135
+    // the DFS numbering.
+    for (DomTreeNode *InnerN : DomNodes)
+      DomSet.insert(InnerN->getBlock());
----------------
sanjoy wrote:
> chandlerc wrote:
> > sanjoy wrote:
> > > I'd be tempted to sort and then binary search instead of building an intermediate set.
> > I have a lame reason to not do that: it makes the order of the worklist non-determinist because i have to sort by pointer order. At the point where I have to keep two lists, it seems like a set is just simpler code-wise.
> I don't see why non-determinism in the worklist order matters here.
That changes the order of children in the domtree, which in turn can change the output of anything that walks the domtree.

Typically, the domtree's order is fully determined by the order of the IR in the function. I'm not worried about having any particular order, but I'm worried about non-deterministic order as I suspect other passes transform code in ways influenced by the domtree visit order.

Does this seem unreasonable?


https://reviews.llvm.org/D32740