[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:49:56 PDT 2017


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

Actually, it should read D35851. Sorry!

Btw, thank you for all the nice verifiers. While they don't catch
everything, they make the intentions of the code a lot more clear.

Best,
Tobias

> 
> 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
> >
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list