[PATCH] D28117: [NewGVN] Merge unconditional branches before value numbering

Daniel Berlin via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 26 10:13:56 PST 2016


SGTM.
I noticed, BTW, that a few of our xfailed tests actually work fine to test
what they are mostly trying to test, the outputjust doesn't perfectly match
yet due to lack of predicate handling.

For example, in calls-nonlocal, we eliminate all the strlen's, etc, which
is what it claims to test.
We just don't propagate some of the equalities yet.



On Mon, Dec 26, 2016 at 9:52 AM, Davide Italiano <dccitaliano at gmail.com>
wrote:

> On Mon, Dec 26, 2016 at 9:47 AM, Daniel Berlin <dberlin at dberlin.org>
> wrote:
> > I don't think NewGVN should do this, instead of a cleanup pass, unless it
> > actually produces better analysis results.
> >
> > I'm already tempted to have us stop wasting time checking
> > isInstructionTriviallyDead.
> >
>
> I have that in my todolist. In my very non-systematic tests it doesn't
> actually improve things substantially.
>
> > I know it matches what GVN does, but that doesn't seem a good reason to
> me,
> > and i don't see how it would improve a PRE that properly does dataflow
> > analysis.
> > In fact, you can prove it can't :)
> >
>
> I agree, see my comment in the bug https://llvm.org/bugs/show_
> bug.cgi?id=31468
>
> > (if the branch is unconditional, and can be merged, it's existence should
> > provably not affect the solution to PRE's dataflow equations).
> >
> > Does it actually improve elimination?
> >
>
> I don't think it does, in fact I think it makes sense to just change
> the test. I'm all for changing behaviour between old and newGVN when
> it's needed.
>
> Thanks!
>
> --
> Davide
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20161226/12ea2f7c/attachment.html>


More information about the llvm-commits mailing list