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

Daniel Berlin via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 31 13:44:05 PDT 2017


FYI, I think you wrote  D36017 a few times when you meant  D36107, which
caused it to link a completely unrelated review :)


On Mon, Jul 31, 2017 at 1:41 PM, Tobias Grosser <tobias.grosser at inf.ethz.ch>
wrote:

> On Mon, Jul 31, 2017, at 22:39, Tobias Grosser via llvm-commits wrote:
> > 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.
>
> Ah, the reason why https://reviews.llvm.org/D36017 is related is that it
> provides a possible solution to consistently handle unreachables and
> infinite loops.
>
> Best,
> Tobias
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170731/72f18f7b/attachment.html>


More information about the llvm-commits mailing list