[PATCH] D29866: [PDSE] Add PDSE.

Tobias Grosser via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 20 04:42:06 PDT 2017


On Tue, Jun 20, 2017, at 01:03 PM, Daniel Berlin via llvm-commits wrote:
> On Tue, Jun 20, 2017 at 12:26 AM, Tobias Grosser <tobias at grosser.es>
> wrote:
> 
> > I think the PDT brokenness is something all of us want to see resolved.
> > I just had an interesting discussion with Hal how currently the PDT is
> > not usable to test the validity of liferange metadata.
> >
> > I think it would be great if we collect the use case we have for
> > dominator trees to clearly see where they are "broken" today and which
> > solution could address this brokenness.
> 
> 
> I'm not sure why you put broken in quotes?

Because we can only reason about broken, if we know what is correct. In
the presence of unreachables and infinite loops this is not a
straightforward extension. I know you claim it is trivial, but that's
not clear.

Anyway, let's wait for the verifier.

> > With Jakub Kuderski pushing
> > non-trivial improvements to dominance relations in LLVM we seem to have
> > a unique chance to get this fixed and well documented in a way that
> > either satisfies all of us -- or at least clearly documents the
> > tradeoffs that have been taken in the final result.
> >
> 
> Jakub has a verifier he's about to post that will verify the structural
> properties of the trees are valid.
> If you can make your version/tradeoffs verify, have at it.

I think a verifier is a good step, especially if formalizes the
invariants you would like.

Best,
Tobias

> 
> 
> >
> > I already started once a document discussing post-dominance literature,
> > but less so use cases. Maybe Jakub would like to collect the different
> > use cases if/when he is touching post-dominance.
> >
> > Best,
> > Tobias
> >
> > On Mon, Jun 19, 2017, at 09:00 AM, Davide Italiano via llvm-commits
> > wrote:
> > > On Sun, Jun 18, 2017 at 11:56 PM, Daniel Berlin <dberlin at dberlin.org>
> > > wrote:
> > > >>
> > > >>
> > > >> ================
> > > >> Comment at: lib/Transforms/Scalar/PDSE.cpp:920
> > > >> +    if (!PDT.getRootNode()) {
> > > >> +      DEBUG(dbgs() << "FIXME: ran into the PDT bug. nothing we can
> > > >> do.\n");
> > > >> +      return false;
> > > >> ----------------
> > > >> Can you please describe the `PDT` bug? I'm not really familiar with
> > it (or
> > > >> I call it with another name :)
> > > >>
> > > >
> > > > This is because the postdominator tree is *badly* broken in a myriad of
> > > > ways.
> > > >
> > > > It's possible to hit this particular issue in a number of ways.
> > > >
> > > > In practice, we are going to have to fix post-dominators before we
> > ever turn
> > > > on this pass.
> > > >
> > >
> > > Oh. I'm aware of the PDT brokeness (I also have some examples) but I
> > > thought you're referring to something specifically :)
> > >
> > > Thanks!
> > >
> > > --
> > > Davide
> > > _______________________________________________
> > > llvm-commits mailing list
> > > llvm-commits at lists.llvm.org
> > > http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> >
> _______________________________________________
> 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