[PATCH] D84763: DomTree: Make PostDomTree immune to block successors swap

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added a comment.

In D84763#2179475 <https://reviews.llvm.org/D84763#2179475>, @kuhar wrote:

> Thanks for pushing on this.
>
> While this makes root finding insensitive to successors' order, I think it still may be possible to run into tree instability by changing the order of tree children (also plain domtree), and in turn changing DFS numbers used by other analyses. In the case of a full tree construction, we could populate an ordering like this before tree construction to solve the tree children order issue, but I don't think this will work in case of incremental updates. AFAIK tree updates would still be based on some Cfg/Graph traits, which may give you unstable order.



> Given that you want to fix InstCombine not to swap successors and move this to SimplifyCFG, does this patch improve anything in the current pass pipeline?

I don't believe that is the correct fix, because it literally says "nope, there is no way to update PDT after swapping successors other than full domtree recomputation", that is why i'm pushing for fixing PDT.

> Overall, looks like a nice improvement to me, if there is no noticeable impact on compilation times. Have you benchmarked this patch?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84763