[PATCH] D81089: Mark InstCombine as not preserving PostDomTree

[Yevgeny Rouban via Phabricator via llvm-commits]

yrouban added a comment.

In D81089#2086290 <https://reviews.llvm.org/D81089#2086290>, @efriedma wrote:

> Even if we agree the way the PostDominatorTree computes roots in cases involving infinite loops shouldn't depend on the ordering of successors, I'm not sure it's correct to say that instcombine "preserves the CFG".  Even for the regular DominatorTree, I think reordering the successors invalidates DFS numbering (in the sense that it changes the result of getDFSNumIn()).
>
> In terms of how the PostDominatorTree should decide what a "root" is, I'm not sure there's any good way to make that decision. Given an SCC which doesn't have any natural parent, you can pick any node in that SCC as the "fake root", and there isn't any natural reason to favor one node over another.
>
> Really, the natural choice is to just follow the standard dominator algorithm and say the nodes are unreachable.  Then for passes which want it, we can add a transform to mutate the CFG to introduce fake roots.


Good point! In other words, if DFSNum was a separate CFG base analysis (and it is as a part of DomTree), it must be invalidated.
@lebedev.ri This shows that making the PostDomTree better will not help.
I will stick to invalidating all CFG analyses in InstCombine only if successors are reordered. I believe we should do this to fix the bug. A better solution can be implemented later on.


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

https://reviews.llvm.org/D81089