[PATCH] D36107: [PostDom] document the current handling of infinite loops and unreachables

Tobias Grosser via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 31 13:39:53 PDT 2017


On Mon, Jul 31, 2017, at 22:29, Jakub Kuderski via Phabricator via
llvm-commits wrote:
> kuhar added a comment.
> 
> In https://reviews.llvm.org/D36107#826573, @grosser wrote:
> 
> > we have little tests that verify the update behavior of PDT after changes that introduce/remove virtual nodes. This is shown by the fact, that the first test case (construction of unreachables) illustrates a bug where we forget to add a new virtual exit node.
> >
> > I would like to commit this change, to have some more test coverage for https://reviews.llvm.org/D36107 and especially to make the impact of https://reviews.llvm.org/D36017 more clear.
> 
> 
> Just to clarify a bit: the current behavior posdominator updates is just
> broken when it comes to root handling -- it just happens that the
> verifier is broken in an analogous way and blindly trusts the set of
> roots inside a dominator tree. If for some reason we wanted keep not
> including infinite loops in postdominators, then it would be still
> possible to fix the root-finding behavior on updates. I mean, this issue
> is a bit orthogonal to what https://reviews.llvm.org/D36017 tries to do.

Sure. The goal of this patch was nothing else but to show the general
brokenness we see here. While looking through your patch closely and
trying to explore the current and future behavior some of this
brokenness popped up. I thought I better post these test cases as they
might help to fix future bugs and as they also will document well the
direction we are going.

I fully agree that the incorrect update behavior for unreachable
instructions is mostly independent of the goal of connecting infinite
loops to virtual exits.

Best,
Tobias


More information about the llvm-commits mailing list